Bug 93208 – [jhbuild] move md5sum checking to update-webkit-libs-jhbuild

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Gustavo Noronha (kov)]

[jhbuild] move md5sum checking to update-webkit-libs-jhbuild

[Gustavo Noronha (kov)]

Created attachment 156570 [details]
Patch

[Martin Robinson]

Comment on attachment 156570 [details]
Patch

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

Looks good, but I have a couple concerns below...

> Tools/Scripts/update-webkit-libs-jhbuild:50
> +    # Read the whole file.

You can probably omit this comment.

> Tools/Scripts/update-webkit-libs-jhbuild:63
> +    foreach my $file (qw(jhbuildrc.md5sum jhbuild.modules.md5sum)) {

It seems a bit less expensive and logical to start with the base names and then append ".md5sum" for the frozen version of the md5sum, like in saveJhbuildMd5.

> Tools/Scripts/webkitdirs.pm:1924
> +    unshift(@buildArgs, jhbuildWrapperPrefixIfNeeded());

Will this do the right thing if the WebKit jhbuild isn't used? It adds an empty string to the front of the array.

[Gustavo Noronha (kov)]

(In reply to comment #2)
> > Tools/Scripts/webkitdirs.pm:1924
> > +    unshift(@buildArgs, jhbuildWrapperPrefixIfNeeded());
> 
> Will this do the right thing if the WebKit jhbuild isn't used? It adds an empty string to the front of the array.

Actually, it'll prepend 'env':


+    } elsif (isGtk()) {
+        if (-e getJhbuildPath()) {
+            return File::Spec->catfile(sourceDir(), "Tools", "gtk", "run-with-jhbuild");
+        }
+        return "env";

[Gustavo Noronha (kov)]

Comment on attachment 156570 [details]
Patch

http://trac.webkit.org/changeset/125674

[Chris Dumez]

After this patch, the behavior on EFL port seems unexpected:
It triggers jhbuild before EVERY build. This is quite annoying:
"""
Updating EFL port dependencies using jhbuild...
*** Checking out pixman *** [1/22]
*** Skipping pixman (not updated) *** [1/22]
*** Checking out cairo *** [2/22]
*** Skipping cairo (not updated) *** [2/22]
*** Checking out fonts *** [3/22]
*** Skipping fonts (not updated) *** [3/22]
*** Checking out fontconfig *** [4/22]
*** Skipping fontconfig (not updated) *** [4/22]
*** Checking out freetype6 *** [5/22]
*** Skipping freetype6 (not updated) *** [5/22]
*** Checking out harfbuzz *** [6/22]
*** Skipping harfbuzz (not updated) *** [6/22]
*** Checking out libffi *** [7/22]
*** Skipping libffi (not updated) *** [7/22]
*** Checking out glib *** [8/22]
...
"""

[Philippe Normand]

Comment on attachment 156570 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:2105
>      if (isEfl()) {
> -        saveJhbuildMd5();
> +        system("perl", "$sourceDir/Tools/Scripts/update-webkitefl-libs") == 0 or die $!;

Due to this change I presume?