Bug 135124 - [Win] Extend auto-version.pl to support 5-tuple versions
Summary: [Win] Extend auto-version.pl to support 5-tuple versions
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-21 11:47 PDT by Brent Fulgham
Modified: 2014-07-22 09:01 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.77 KB, patch)
2014-07-21 12:07 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (3.95 KB, patch)
2014-07-21 13:05 PDT, Brent Fulgham
ddkilzer: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 (588.59 KB, application/zip)
2014-07-21 15:38 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-07-21 11:47:14 PDT
The existing regular expressions parse 1-3-tuple versions. However, we want to start using 5-tuple version values.
Comment 1 Radar WebKit Bug Importer 2014-07-21 11:47:41 PDT
<rdar://problem/17750334>
Comment 2 Brent Fulgham 2014-07-21 12:07:27 PDT
Created attachment 235235 [details]
Patch
Comment 3 Brent Fulgham 2014-07-21 13:05:22 PDT
Created attachment 235241 [details]
Patch
Comment 4 Brent Fulgham 2014-07-21 13:05:44 PDT
Comparison of results between old shell script and Perl script:

"auto-version.sh":
5300.4.3.2.1 -> 3, 00, 4, 3
530.4.3.2.1 -> 5, 30, 4, 3
53.4.3.2.1 -> 5, 3, 4, 3
5.4.3.2.1 -> 5, ERROR, 4, 3
5300.4.3.2 -> 3, 00, 4, 3
530.4.3.2 -> 5, 30, 4, 3
53.4.3.2 -> 5, 3, 4, 3
5.4.3.2 -> 5, ERROR, 4, 3
5300.4.3 -> 3, 00, 4, 3
530.4.3 -> 5, 30, 4, 3
53.4.3 -> 5, 3, 4, 3
5.4.3 -> 5, ERROR, 4, 3
5300.4 -> 3, 00, 4, 0
530.4 -> 5, 30, 4, 0
53.4 -> 5, 3, 4, 0
5.4 -> 5, ERROR, 4, 0
5300 -> 3, 00, 0, 0
530 -> 5, 30, 0, 0
53 -> 5, 3, 0, 0
5 -> 5, ERROR, 0, 0

"auto-versoin.pl":
5300.4.3.2.1 -> 3, 00, 4, 3
530.4.3.2.1 -> 5, 30, 4, 3
53.4.3.2.1 -> 5, 3, 4, 3
5.4.3.2.1 -> 5, ERROR, 4, 3
5300.4.3.2 -> 3, 00, 4, 3
530.4.3.2 -> 5, 30, 4, 3
53.4.3.2 -> 5, 3, 4, 3
5.4.3.2 -> 5, ERROR, 4, 3
5300.4.3 -> 3, 00, 4, 3
530.4.3 -> 5, 30, 4, 3
53.4.3 -> 5, 3, 4, 3
5.4.3 -> 5, ERROR, 4, 3
5300.4.3 -> 3, 00, 4, 0
530.4 -> 5, 30, 4, 0
53.4 -> 5, 3, 4, 0
5.4 -> 5, ERROR, 4, 0
5300 -> 3, 00, 0, 0
530 -> 5, 30, 0, 0
53 -> 5, 3, 0, 0
5 -> 5, ERROR, 0, 0
Comment 5 David Kilzer (:ddkilzer) 2014-07-21 13:10:27 PDT
Would like to see test results for these 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 6 David Kilzer (:ddkilzer) 2014-07-21 13:13:24 PDT
Comment on attachment 235241 [details]
Patch

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

r=me, but consider adding well-named subroutines to do the work (and bdash's feedback on IRC about splitting on '.').

> WebKitLibraries/win/tools/scripts/auto-version.pl:94
> +my ($BUILD_MAJOR_VERSION, $BUILD_MINOR_VERSION, $BUILD_TINY_VERSION);
> +{
> +    $PROPOSED_VERSION =~ m/^(?:(\d+)\.)?(?:(\d+)\.)?(?:(\d+)\.)?(?:(\d+)\.)?(\*|\d+)$/ or die "Couldn't parse $PROPOSED_VERSION";
> +    $BUILD_MAJOR_VERSION = $1;
> +    $BUILD_MINOR_VERSION = $2;
> +    $BUILD_TINY_VERSION = $3;
> +    if (!defined $3) {
> +        $BUILD_TINY_VERSION = $5;
> +    }
> +
> +    # The default version (with no decimals) will be matched by the regexp
> +    # to $BUILD_TINY_VERSION. If that happens, we need to move it to
> +    # $BUILD_MAJOR_VERSION.
> +    if (!defined $BUILD_MAJOR_VERSION && !defined $BUILD_MINOR_VERSION) {
> +        $BUILD_MAJOR_VERSION = $BUILD_TINY_VERSION;
> +        $BUILD_TINY_VERSION = 0;
> +    } elsif (!defined $BUILD_MINOR_VERSION && !defined $3) {
> +        $BUILD_MINOR_VERSION = $BUILD_TINY_VERSION;
> +        $BUILD_TINY_VERSION = 0;
> +    }
>  }

Why not make this a subroutine?

> WebKitLibraries/win/tools/scripts/auto-version.pl:101
> +{
> +    $BUILD_MAJOR_VERSION =~ s/^.*(\d\d\d)$/$1/;
> +}

Ditto.

> WebKitLibraries/win/tools/scripts/auto-version.pl:126
> +my ($MAJOR_VERSION, $MINOR_VERSION);
> +{
> +    if ($BUILD_MAJOR_VERSION =~ m/^[^\d]*(\d)(\d{1,})/) {
> +        $MAJOR_VERSION = $1;
> +        $MINOR_VERSION = $2;
> +    } else {
> +        $MAJOR_VERSION = $BUILD_MAJOR_VERSION;
> +    }
> +    print "From $BUILD_MAJOR_VERSION got $MAJOR_VERSION and $MINOR_VERSION\n";
> +}

Ditto.
Comment 7 Build Bot 2014-07-21 15:38:01 PDT
Comment on attachment 235241 [details]
Patch

Attachment 235241 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5137600729841664

New failing tests:
media/W3C/video/src/src_reflects_attribute_not_source_elements.html
Comment 8 Build Bot 2014-07-21 15:38:03 PDT
Created attachment 235250 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 9 Brent Fulgham 2014-07-21 16:02:14 PDT
(In reply to comment #7)
> (From update of attachment 235241 [details])
> Attachment 235241 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.appspot.com/results/5137600729841664
> 
> New failing tests:
> media/W3C/video/src/src_reflects_attribute_not_source_elements.html

Ummm. I really doubt that. :-)
Comment 10 Brent Fulgham 2014-07-21 16:11:21 PDT
Committed r171319: <http://trac.webkit.org/changeset/171319>
Comment 13 Carlos Alberto Lopez Perez 2014-07-21 18:42:55 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > Committed r171319: <http://trac.webkit.org/changeset/171319>
> > 
> > This broke test-webkitperl on both Linux and Mac:
> > 
> > GTK port log: http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/1580/steps/webkitperl-test/logs/stdio
> > Mac port log: http://build.webkit.org/builders/Apple%20Mavericks%20Release%20WK1%20%28Tests%29/builds/7501/steps/webkitperl-test/logs/stdio
> 
> Speculative fix checked in under r171324 <http://trac.webkit.org/changeset/171324>.

Worked :)

Thanks
Comment 14 Brent Fulgham 2014-07-22 09:01:09 PDT
Two follow-up patches were needed:

1. Handle the 'cygwin' case as well as 'MSWin32' for Windows builds:
<http://trac.webkit.org/changeset/171324>

2. Revert to older Perl 5.8 syntax to support Mountain Lion build bots.
<http://trac.webkit.org/changeset/171335>