Bug 137982 – prepare-changelog does not read paths containing spaces properly

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Remy Demarest]

I run prepare-changelog on a repository that contains changes inside folders whose name contains spaces and I get multiple errors like this:
fatal: Path 'Safari/AppKit' does not exist in 'origin/master'
We have two folder named "AppKit Extra Classes" and "AppKit Extra Methods" for which this could apply.

[Blaze Burg]

Ugh, just ran into this too.

[Blaze Burg]

Relevant trace from using `perl -d:Trace prepare-Changelog`:

>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:335:                 my @deleted_function_ranges = get_function_line_ranges(\*SOURCE, $file);
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:688:     my ($file_handle, $file_name) = @_;
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:692:     return get_function_line_ranges_for_cpp($file_handle, $file_name) if $file_name =~ /\.(c|cpp|m|mm|h)$/;
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:693:     return get_function_line_ranges_for_java($file_handle, $file_name) if $file_name =~ /\.java$/;
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:694:     return get_function_line_ranges_for_javascript($file_handle, $file_name) if $file_name =~ /\.js$/;
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:695:     return get_selector_line_ranges_for_css($file_handle, $file_name) if $file_name =~ /\.css$/;
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:696:     return get_function_line_ranges_for_perl($file_handle, $file_name) if $file_name =~ /\.p[lm]$/;
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:697:     return get_function_line_ranges_for_python($file_handle, $file_name) if $file_name =~ /\.py$/ or $file_name =~ /master\.cfg$/;
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:698:     return get_function_line_ranges_for_swift($file_handle, $file_name) if $file_name =~ /\.swift$/;
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:702:     my $first_line = <$file_handle>;
fatal: Path 'Safari.xcworkspace/xcshareddata/xcschemes/Only' does not exist in 'HEAD'
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:703:     seek($file_handle, 0, 0);
>> /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog:705:     return () unless $first_line =~ m|^#!(?:/usr/bin/env\s+)?(\S+)|;
Use of uninitialized value $first_line in pattern match (m//) at /Users/bburg/repos/webkit-tot/OpenSource/Tools/Scripts/prepare-ChangeLog line 705.

[Blaze Burg]

Created attachment 269012 [details]
Proposed Fix

[Joseph Pecoraro]

Comment on attachment 269012 [details]
Proposed Fix

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

r=me

> Tools/Scripts/prepare-ChangeLog:319
> +        $file =~ s/ /\\ /;

I think this needs the trailing /g in order to change all occurrences.

[Blaze Burg]

Committed r195080: <http://trac.webkit.org/changeset/195080>

[Daniel Bates]

Comment on attachment 269012 [details]
Proposed Fix

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

>> Tools/Scripts/prepare-ChangeLog:319
>> +        $file =~ s/ /\\ /;
> 
> I think this needs the trailing /g in order to change all occurrences.

This change, including the chomp(), is not necessary. The actual fix for this bug was below with using the open() call that takes a mode. In fact, this change breaks function list generation for file paths that have a space in them because the code above ^^^ built up a dictionaries of line ranges keyed off the **unescaped** filename. However, after this substitution the code below will query these dictionaries for the **escaped** filename and hence **never** find a match.

> Tools/Scripts/prepare-ChangeLog:323
> +            open(SOURCE, "<", $file) or next;

This is the actual fix!