Bug 82073 – [GTK] libgcrypt and p11-kit should not be in jhbuild modules

• Component: New Bugs
• Status: RESOLVED FIXED

[Gustavo Noronha (kov)]

[GTK] libgcrypt and p11-kit should not be in jhbuild modules

[Gustavo Noronha (kov)]

Created attachment 133522 [details]
Patch

[Gustavo Noronha (kov)]

For completeness sake: these libraries ended up in our jhbuild.modules file because during the hackfest we were in a hurry to make the jhbuild system go live and fix the build. There is no reason why these libraries should be included, really, they are easily installable as system packages.

[Philippe Normand]

Comment on attachment 133522 [details]
Patch

Attachment 133522 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12113768

[Gustavo Noronha (kov)]

Created attachment 133542 [details]
Patch

[Martin Robinson]

Comment on attachment 133542 [details]
Patch

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

Looks good, though if possible it'd would be nice to remove Tools/gtk/clean-jhbuild and replace it with a couple inline Perl system() calls.

> Tools/Scripts/webkitdirs.pm:1947
> +        # If the configuration changed, dependencies may have been removed.
> +        # Since we lack a granular way of uninstalling those we wipe out the
> +        # jhbuild root and start from scratch.
> +        if (system("$sourceDir/Tools/gtk/clean-jhbuild") ne 0) {
> +            die "Cleaning jhbuild failed!";
> +        }

Is it possible to do this in Perl rather than creating another script?

> Tools/Scripts/webkitdirs.pm:1953
> +    if (checkForArgumentAndRemoveFromArrayRef("--update-gtk", \@buildArgs)) {
> +        $needUpdate = 1;
> +    }

This argument is unfamiliar to me. Where is it used?

[Gustavo Noronha (kov)]

(In reply to comment #5)
> (From update of attachment 133542 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133542&action=review
> 
> Looks good, though if possible it'd would be nice to remove Tools/gtk/clean-jhbuild and replace it with a couple inline Perl system() calls.

Will do!

> > Tools/Scripts/webkitdirs.pm:1953
> > +    if (checkForArgumentAndRemoveFromArrayRef("--update-gtk", \@buildArgs)) {
> > +        $needUpdate = 1;
> > +    }
> 
> This argument is unfamiliar to me. Where is it used?

That was added for the EWS bots to use, it was suggested by abarth, it's similar to how the chromium port does it.

[Gustavo Noronha (kov)]

Created attachment 133578 [details]
Patch

[Gustavo Noronha (kov)]

Committed r111929: <http://trac.webkit.org/changeset/111929>

[Philippe Normand]

This patch badly broke setups where the build directory is the git branch name, at least.

Here my build directory is /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/branchName/Release

jhbuild clean is triggered... and fails:


/bin/sh /home/phil/gst/jhbuild/build/WebKit/WebKitBuild/Dependencies/Root/share/jhbuild/triggers/glib.trigger
*** the following modules were not built *** [16/16]
fonts
Cleaning jhbuild modules failed! at /home/phil/gst/jhbuild/build/WebKit/Tools/Scripts/webkitdirs.pm line 1951.

It seems the jhbuild.modules md5sum test always fails in that setup... investigating... :(

[Philippe Normand]

As a side note I don't think storing the jhbuildrc.md5sum and jhbuild.modules.md5sum in the build directory is a very good idea... Why not storing it in Dependencies or in Tools/ somewhere and ignore it from the VCS POV?

[Philippe Normand]

(In reply to comment #10)
> As a side note I don't think storing the jhbuildrc.md5sum and jhbuild.modules.md5sum in the build directory is a very good idea... Why not storing it in Dependencies or in Tools/ somewhere and ignore it from the VCS POV?

Ok let's close this, I have a follow-up patch...

[Gustavo Noronha (kov)]

(In reply to comment #10)
> As a side note I don't think storing the jhbuildrc.md5sum and jhbuild.modules.md5sum in the build directory is a very good idea... Why not storing it in Dependencies or in Tools/ somewhere and ignore it from the VCS POV?

It's been stored in the build directory because in a way whether a rebuild is required or not for that particular build type depends on this. However, I can see how that might not be the case.