Bug 135161

Summary: Correct auto-version.pl handling of __VERSION_TEXT__, __BUILD_NUMBER__, and __BUILD_NUMBER_SHORT__ for 4+-tuple versions
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Tools / TestsAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, dbates, ddkilzer, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ddkilzer: review+

Description Brent Fulgham 2014-07-22 09:44:14 PDT
While playing with some additional test cases Dave Kilzer suggested, I realized that the auto-version.pl script does not match the behavior of the original auto-version.sh script.

This patch fixes the problem for the following test cases:

10530.1.1.1
10530.30.20.10
10530.300.200.100
10530.3000.2000.1000

7530.30.20.10
7530.300.200.100
7530.3000.2000.1000
Comment 1 Radar WebKit Bug Importer 2014-07-22 09:46:05 PDT
<rdar://problem/17763546>
Comment 2 Brent Fulgham 2014-07-22 09:55:59 PDT
Created attachment 235294 [details]
Patch
Comment 3 David Kilzer (:ddkilzer) 2014-07-22 10:29:09 PDT
Comment on attachment 235294 [details]
Patch

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

r=me

> WebKitLibraries/win/tools/scripts/auto-version.pl:126
> +    $PROPOSED_VERSION =~ s/^\s*(.*)\s*$/$1/; # Get rid of any leading/trailing whitespace
> +

Please add one test with trailing and leading whitespace.  (I didn't see any added to this patch.)

> WebKitLibraries/win/tools/scripts/auto-version.pl:127
>      # Split out the three components of the dotted version number.  We pad

Nit:  "three components" isn't accurate; maybe just remove the word "three".

> WebKitLibraries/win/tools/scripts/auto-version.pl:153
> +    $PROPOSED_VERSION = substr $PROPOSED_VERSION, $charactersToRemove;

Nit:  Kind of weird to omit parens here for substr() when they're used everywhere else.
Comment 4 Brent Fulgham 2014-07-22 11:12:05 PDT
Comment on attachment 235294 [details]
Patch

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

>> WebKitLibraries/win/tools/scripts/auto-version.pl:126
>> +
> 
> Please add one test with trailing and leading whitespace.  (I didn't see any added to this patch.)

Will do.

>> WebKitLibraries/win/tools/scripts/auto-version.pl:127
>>      # Split out the three components of the dotted version number.  We pad
> 
> Nit:  "three components" isn't accurate; maybe just remove the word "three".

Done.

>> WebKitLibraries/win/tools/scripts/auto-version.pl:153
>> +    $PROPOSED_VERSION = substr $PROPOSED_VERSION, $charactersToRemove;
> 
> Nit:  Kind of weird to omit parens here for substr() when they're used everywhere else.

Agreed! Fixed.
Comment 5 Brent Fulgham 2014-07-22 11:36:09 PDT
Committed r171351: <http://trac.webkit.org/changeset/171351>