Bug 48527 – commit-log-editor uses full paths for section headers when using msysgit's Perl and multiple ChangeLogs have been edited

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

If you use commit-log-editor with msysgit's Perl, and you've edited more than one ChangeLog, you'll get section headers like this:

h:/cyghome/dev/WebKit/OpenSource/LayoutTests:
h:/cyghome/dev/WebKit/OpenSource/WebKit2:

instead of like this:

WebKit2:
LayoutTests:

The badness seems to occur when the ChangeLog path is run through File::Spec->rel2abs.

[Adam Roben (:aroben)]

I cannot reproduce the abs2rel badness when doing this:

perl -e 'use File::Spec; print File::Spec->abs2rel("WebKit2/ChangeLog"), "\n";'

Very strange!

[David Kilzer (:ddkilzer)]

(In reply to comment #0)
> The badness seems to occur when the ChangeLog path is run through File::Spec->rel2abs.

(In reply to comment #1)
> I cannot reproduce the abs2rel badness when doing this:
> 
> perl -e 'use File::Spec; print File::Spec->abs2rel("WebKit2/ChangeLog"), "\n";'

Did you mean abs2rel() or rel2abs()?

[Adam Roben (:aroben)]

(In reply to comment #2)
> (In reply to comment #0)
> > The badness seems to occur when the ChangeLog path is run through File::Spec->rel2abs.
> 
> (In reply to comment #1)
> > I cannot reproduce the abs2rel badness when doing this:
> > 
> > perl -e 'use File::Spec; print File::Spec->abs2rel("WebKit2/ChangeLog"), "\n";'
> 
> Did you mean abs2rel() or rel2abs()?

Whoops, I meant rel2abs.

When I run this script:

perl -e 'use File::Spec; print File::Spec->rel2abs("WebKit2/ChangeLog"), "\n";'

I get:

/h/cyghome/dev/WebKit/OpenSource/WebKit2/ChangeLog

which is an msys-style path, not a Windows-style-with-forward-slashes path like is appearing in the commit log. The same thing happens if I tell perl to print that to a file instead of to stdout.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> When I run this script:
> 
> perl -e 'use File::Spec; print File::Spec->rel2abs("WebKit2/ChangeLog"), "\n";'
> 
> I get:
> 
> /h/cyghome/dev/WebKit/OpenSource/WebKit2/ChangeLog
> 
> which is an msys-style path, not a Windows-style-with-forward-slashes path like is appearing in the commit log. The same thing happens if I tell perl to print that to a file instead of to stdout.

What are $changeLog and $topLevel set to when this line of code is run?

    $changeLog = File::Spec->abs2rel(File::Spec->rel2abs($changeLog), $topLevel);

[Adam Roben (:aroben)]

(In reply to comment #4)
> What are $changeLog and $topLevel set to when this line of code is run?
> 
>     $changeLog = File::Spec->abs2rel(File::Spec->rel2abs($changeLog), $topLevel);

$changeLog is "WebKit2/ChangeLog"
$topLevel is "."
File::Spec->rel2abs($changeLog) is "h:/cyghome/dev/WebKit/OpenSource/WebKit2/ChangeLog"
File::Spec->abs2rel(File::Spec->rel2abs($changeLog), $topLevel) is "h:/cyghome/dev/WebKit/OpenSource/WebKit2/ChangeLog"

It's rel2abs that's going screwy. Like I said, I can't reproduce this result with the simple script from comment 3.

[Adam Roben (:aroben)]

(In reply to comment #3)
> When I run this script:
> 
> perl -e 'use File::Spec; print File::Spec->rel2abs("WebKit2/ChangeLog"), "\n";'
> 
> I get:
> 
> /h/cyghome/dev/WebKit/OpenSource/WebKit2/ChangeLog
> 
> which is an msys-style path, not a Windows-style-with-forward-slashes path like is appearing in the commit log. The same thing happens if I tell perl to print that to a file instead of to stdout.

I also tried running this script from a normal command prompt instead of from msys's bash prompt. I got the same result: an msys-style path. I can't figure out what's causing the Windows-style path to be printed!

[Adam Roben (:aroben)]

I did another experiment. I modified commit-log-editor to print the output of rel2abs and then immediately exit. Then I did:

git status > log.txt
commit-log-editor log.txt

And I got an msys-style path! So something different is happening when git is invoking commit-log-editor than when I invoke it directly.

[Adam Roben (:aroben)]

I modified commit-log-editor to also dump %ENV. When run from the bash prompt, paths in environment variables are all msys-style. When run from git commit, many paths (but not all) are Windows-style, including $PWD. I'll bet this is the cause of the difference.

[Adam Roben (:aroben)]

Created attachment 79420 [details]
data dumped when running commit-log-editor from bash prompt

[Adam Roben (:aroben)]

Created attachment 79421 [details]
data dumped when running commit-log-editor from inside git commit

[Adam Roben (:aroben)]

Created attachment 79423 [details]
Convert paths in environment variables back to msys-style inside commit-log-editor

[WebKit Review Bot]

Attachment 79423 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2

perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LANG = "en_US.US-ASCII"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
Updating OpenSource
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LANG = "en_US.US-ASCII"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
RA layer request failed: OPTIONS of 'http://svn.webkit.org/repository/webkit': could not connect to server (http://svn.webkit.org) at /usr/lib/git-core/git-svn line 2295

Died at Tools/Scripts/update-webkit line 132.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Roben (:aroben)]

(In reply to comment #12)
> Attachment 79423 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2
> 
> perl: warning: Setting locale failed.
> perl: warning: Please check that your locale settings:
>     LANGUAGE = (unset),
>     LC_ALL = (unset),
>     LANG = "en_US.US-ASCII"
>     are supported and installed on your system.
> perl: warning: Falling back to the standard locale ("C").
> Updating OpenSource
> perl: warning: Setting locale failed.
> perl: warning: Please check that your locale settings:
>     LANGUAGE = (unset),
>     LC_ALL = (unset),
>     LANG = "en_US.US-ASCII"
>     are supported and installed on your system.
> perl: warning: Falling back to the standard locale ("C").
> RA layer request failed: OPTIONS of 'http://svn.webkit.org/repository/webkit': could not connect to server (http://svn.webkit.org) at /usr/lib/git-core/git-svn line 2295
> 
> Died at Tools/Scripts/update-webkit line 132.

Maybe this was caused by the server upgrade that happened this morning?

[David Kilzer (:ddkilzer)]

Comment on attachment 79423 [details]
Convert paths in environment variables back to msys-style inside commit-log-editor

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

r=me, but please fix the regex to only match one alphabetic character.

> Tools/Scripts/commit-log-editor:299
> +        $ENV{$key} =~ s/^(\w):\//\/$1\//g;

Consider using a character other than "/" for the s/// operator here (like s###) so that you don't have to escape the '/' characters in the path:

    $ENV{$key} =~ s#^(\w):/#/$1/#g;

I don't think you want "\w" here since that will match any *word*.  If it's only one character and only a-z (or A-Z), I would use:

    $ENV{$key} =~ s#^([[:alpha:]]):/#/$1/#g;

Or this:

    $ENV{$key} =~ s#^([a-zA-Z]):/#/$1/#g;

[Adam Roben (:aroben)]

Comment on attachment 79423 [details]
Convert paths in environment variables back to msys-style inside commit-log-editor

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

>> Tools/Scripts/commit-log-editor:299
>> +        $ENV{$key} =~ s/^(\w):\//\/$1\//g;
> 
> Consider using a character other than "/" for the s/// operator here (like s###) so that you don't have to escape the '/' characters in the path:
> 
>     $ENV{$key} =~ s#^(\w):/#/$1/#g;
> 
> I don't think you want "\w" here since that will match any *word*.  If it's only one character and only a-z (or A-Z), I would use:
> 
>     $ENV{$key} =~ s#^([[:alpha:]]):/#/$1/#g;
> 
> Or this:
> 
>     $ENV{$key} =~ s#^([a-zA-Z]):/#/$1/#g;

\w only matches a single character. It matches a "word" character (alphanumeric or underscore). But you made me realize we shouldn't match numbers or underscore, so I'll go with [[:alpha:]].

I'll also use #.

[Adam Roben (:aroben)]

Committed r76255: <http://trac.webkit.org/changeset/76255>