Bug 135161 - Correct auto-version.pl handling of __VERSION_TEXT__, __BUILD_NUMBER__, and __BUILD_NUMBER_SHORT__ for 4+-tuple versions
Summary: Correct auto-version.pl handling of __VERSION_TEXT__, __BUILD_NUMBER__, and _...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-07-22 09:44 PDT by Brent Fulgham
Modified: 2014-07-22 11:36 PDT (History)
5 users (show)

See Also:


Attachments
Patch (16.91 KB, patch)
2014-07-22 09:55 PDT, Brent Fulgham
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>