Bug 79509 – Bash scripts should support LF endings only

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Ashod Nakashian]

Some bash scripts have svn:eol-style property set to LF while others have it set to Native, which on Windows is CRLF. This is causing issues on bash versions that find CR unexpected.

This bug is to set svn:eol-style to LF on all bash scripts (and possibly consider python and perl scripts as well). This is the most common denominator and should avoid potential issues.

[Ashod Nakashian]

Created attachment 128849 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 128849 [details]
Patch

Please create the patch using ./Tools/Scripts/svn-create-patch so that the property changes are included in the patch.

[Ashod Nakashian]

Created attachment 129406 [details]
Patch to convert all bash scripts to LF eol-style and mark them as executable.

Created the patch using svn-create-patch as suggested David Kilzer. In addition to fixing eol-style to LF, I've marked all bash scripts (with extension sh) as executable as some had the property while others didn't.

[David Kilzer (:ddkilzer)]

Comment on attachment 129406 [details]
Patch to convert all bash scripts to LF eol-style and mark them as executable.

r=me

[WebKit Review Bot]

Comment on attachment 129406 [details]
Patch to convert all bash scripts to LF eol-style and mark them as executable.

Rejecting attachment 129406 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'David Kilz..." exit_code: 9 cwd: /mnt/git/webkit-commit-queue/

Failed to find the property value for the SVN property "svn:eol-style": "## -0,0 +1 ##
". at /mnt/git/webkit-commit-queue/Tools/Scripts/VCSUtils.pm line 1184, <ARGV> line 28.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'David Kilz..." exit_code: 9 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/11821005

[Eric Seidel (no email)]

It looks like you may be hitting the same issue described in bug 80104.

I think svn-apply regressed at some point.  Sadly our most recent svn-apply maintainer has gone off to other things.

Who uses SVN anymore anyway? ;p

[Ashod Nakashian]

(In reply to comment #6)
> It looks like you may be hitting the same issue described in bug 80104.
> 
> I think svn-apply regressed at some point.  Sadly our most recent svn-apply maintainer has gone off to other things.

So are you saying generating the diff with git will fix the issue and will make the style server happy?

> 
> Who uses SVN anymore anyway? ;p

I was under the impression that although many use git, svn was still the de facto repo and use-case. Either that, or we're maintaining two repos and vc's when one is practically all that's used.

[David Barr]

(In reply to comment #5)
This is because svn 1.7 changed to unidiff output for property changes:
  http://subversion.apache.org/docs/release-notes/1.7.html#diff-properties
For property additions, conversion to the 1.6 format is as simple as stripping the header.
Is it possible to simply re-upload the patch with the svn-1.7 property diff headers removed? 
ie. strip all lines '## -0,0 +1 ##' from the patch.
Alternatively, one could regenerate the patch with svn 1.6.

[Ashod Nakashian]

Created attachment 130805 [details]
Convert all bash scripts to LF eol-style and mark them as executable

Per David's suggestion, stripped SVN 1.7 specific lines from the diff to make the style checker happy.

[Ashod Nakashian]

Created attachment 130812 [details]
Convert all bash scripts to LF eol-style and mark them as executable

Another fix to the patch to make it compatible with SVN 1.6 diff.

[David Kilzer (:ddkilzer)]

Comment on attachment 130812 [details]
Convert all bash scripts to LF eol-style and mark them as executable

r=me

Let's try this again!

[WebKit Review Bot]

Comment on attachment 130812 [details]
Convert all bash scripts to LF eol-style and mark them as executable

Rejecting attachment 130812 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
9.
Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'David Kilz..." exit_code: 9 cwd: /mnt/git/webkit-commit-queue/

Failed to find the property value for the SVN property "svn:eol-style": "+LF
". at /mnt/git/webkit-commit-queue/Tools/Scripts/VCSUtils.pm line 1184, <ARGV> line 28.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'David Kilz..." exit_code: 9 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/11903151

[Daniel Bates]

(In reply to comment #12)
> (From update of attachment 130812 [details])
> Rejecting attachment 130812 [details] from commit-queue.
> [...]
> Failed to find the property value for the SVN property "svn:eol-style": "+LF
> ". at /mnt/git/webkit-commit-queue/Tools/Scripts/VCSUtils.pm line 1184, <ARGV> line 28.

This patch will need to be landed by hand since svn-apply, which is used by the commit-queue to apply a patch to a working copy, only supports the svn:executable property. We have a FIXME in VCSUtils to recognize other SVN properties, <http://trac.webkit.org/browser/trunk/Tools/Scripts/VCSUtils.pm?rev=106054#L1102>. We should also look to recognize the git equivalent of svn:eol-style, see gitattribute(5) <http://schacon.github.com/git/gitattributes.html>.

[David Kilzer (:ddkilzer)]

Landing locally on Lion did not work either:

$ ./Tools/Scripts/webkit-patch land-from-bug 79509
Fetching: https://bugs.webkit.org/show_bug.cgi?id=79509&ctype=xml
1 reviewed patch found on bug 79509.
Processing 1 patch from 1 bug.
Updating working directory
Updating OpenSource
U    Source/JavaScriptCore/wtf/url/api/ParsedURL.h
U    Source/JavaScriptCore/ChangeLog
U    Source/WebCore/ChangeLog
U    Source/WebCore/platform/KURLWTFURLImpl.h
Updated to revision 110261.
Updating Internal
At revision 41613.
Processing patch 130812 from bug 79509.
Reading Keychain for bugs.webkit.org account and password.  Click "Allow" to continue...
Logging in as [e-mail]...
Failed to run "[u'/Volumes/Data/OpenSource/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'David Kilzer']" exit_code: 25 cwd: /Volumes/Data/OpenSource

Failed to find the property value for the SVN property "svn:eol-style": "+LF
". at /Volumes/Data/OpenSource/Tools/Scripts/VCSUtils.pm line 1184, <ARGV> line 28.

Failed to run "[u'/Volumes/Data/OpenSource/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'David Kilzer']" exit_code: 25 cwd: /Volumes/Data/OpenSource



$ ./Tools/Scripts/svn-apply ~/Downloads/patch.txt 
Failed to find the property value for the SVN property "svn:eol-style": "+LF
". at /Volumes/Data/OpenSource/Tools/Scripts/VCSUtils.pm line 1184, <ARGV> line 28.

[David Kilzer (:ddkilzer)]

(In reply to comment #13)
> This patch will need to be landed by hand since svn-apply, which is used by the commit-queue to apply a patch to a working copy, only supports the svn:executable property.

Doh, just read the "landed by hand" part.  I won't have time to do this until after next week if anyone else wants to try.

[Daniel Bates]

(In reply to comment #15)
> (In reply to comment #13)
> > This patch will need to be landed by hand since svn-apply, which is used by the commit-queue to apply a patch to a working copy, only supports the svn:executable property.
> 
> Doh, just read the "landed by hand" part.  I won't have time to do this until after next week if anyone else wants to try.

I'll look to land it tomorrow unless someone else does it before then.

[Ashod Nakashian]

(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #13)
> > > This patch will need to be landed by hand since svn-apply, which is used by the commit-queue to apply a patch to a working copy, only supports the svn:executable property.
> > 
> > Doh, just read the "landed by hand" part.  I won't have time to do this until after next week if anyone else wants to try.
> 
> I'll look to land it tomorrow unless someone else does it before then.

Thanks David and Daniel. I'll look into the VCSUtils script over the weekend to see if I can hack the missing property support.

[Daniel Bates]

Comment on attachment 130812 [details]
Convert all bash scripts to LF eol-style and mark them as executable

View in context: https://bugs.webkit.org/attachment.cgi?id=130812&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

Will remove before landing.

[Daniel Bates]

Committed r110306: <http://trac.webkit.org/changeset/110306>

[Daniel Bates]

(In reply to comment #19)
> Committed r110306: <http://trac.webkit.org/changeset/110306>

For some reason webkit-patch land 79509 committed only the changes to the ChangeLog files.

Committed the property changes in <http://trac.webkit.org/changeset/110318>.