[GStreamer][GTK][WPE] update-webkit-libs-jhbuild fails to detect changes in included moduleset files. For instance, it cannot detect modification of gstreamer/jhbuild.modules when updating dependencies of WebKitGtk.
This regressed when we split GStreamer stuff into a separate moduleset file (that update-webkit-libs-jhbuild does not know about).
Created attachment 323529 [details] Patch Tentative patch. Note that my Perl-fu is close to non-existant. (But at least this did not seem to break anything locally for me.)
I discussed with Michael on this by email and he told me that we don't want to have to modify the perl script every time we add a new moduleset file. So maybe we should find another fix. In the meantime I prepared a patch that parses jhbuild.modules to determine if other jhbuild.modules are included. If any, it computes MD5 sum for each included jhbuild.modules file. If you think it is a good way to fix this bug, I can upload the patch.
(In reply to Nael Ouedraogo from comment #3) > I discussed with Michael on this by email and he told me that we don't want > to have to modify the perl script every time we add a new moduleset file. So > maybe we should find another fix. Well, not having to touch the Perl is certainly nicer. > In the meantime I prepared a patch that parses jhbuild.modules to determine > if other jhbuild.modules are included. If any, it computes MD5 sum for each > included jhbuild.modules file. If you think it is a good way to fix this > bug, I can upload the patch. That sounds certainly *much better* than the patch I have posted earlier. Thanks a lot for working on it!
Comment on attachment 323529 [details] Patch Marking my patch as obsolete, as Nael's solution will be much better.
Created attachment 323535 [details] Patch
I'd prefer somebody with better Perl-fu could review this but the main ideas lgtm.
Comment on attachment 323535 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323535&action=review > Tools/Scripts/webkitdirs.pm:49 > +use XML::LibXML; This is a new runtime dependency. It should be added to the scripts Tools/gtk/install-dependencies and Tools/wpe/install-dependencies. The name of the package in Debian is libxml-libxml-perl I will be installing it on the bots so the EWS don't fail anymore due to this
(In reply to Carlos Alberto Lopez Perez from comment #8) > Comment on attachment 323535 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=323535&action=review > > > Tools/Scripts/webkitdirs.pm:49 > > +use XML::LibXML; > > This is a new runtime dependency. It should be added to the scripts > Tools/gtk/install-dependencies and Tools/wpe/install-dependencies. > The name of the package in Debian is libxml-libxml-perl > I will be installing it on the bots so the EWS don't fail anymore due to this For Arch Linux the package is “perl-xml-libxml”: https://www.archlinux.org/packages/extra/x86_64/perl-xml-libxml/
Comment on attachment 323535 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323535&action=review r=me if you also update the two install-dependencies scripts. Thanks! >>> Tools/Scripts/webkitdirs.pm:49 >>> +use XML::LibXML; >> >> This is a new runtime dependency. It should be added to the scripts Tools/gtk/install-dependencies and Tools/wpe/install-dependencies. >> The name of the package in Debian is libxml-libxml-perl >> I will be installing it on the bots so the EWS don't fail anymore due to this > > For Arch Linux the package is “perl-xml-libxml”: > > https://www.archlinux.org/packages/extra/x86_64/perl-xml-libxml/ The package name variations for this on the different distros are incredible... I believe the correct Fedora package is "perl-libxml-perl". > Tools/Scripts/webkitdirs.pm:1888 > +sub getJhbuildIncludedFilePath I think this should be called "getJhbuildIncludedFilePaths" since it returns multiple paths.
(In reply to Michael Catanzaro from comment #10) > The package name variations for this on the different distros are > incredible... I believe the correct Fedora package is "perl-libxml-perl". Not to be confused with Debian's libxml-libxml-perl and Arch's perl-xml-libxml....
Created attachment 324009 [details] Patch
Thanks for all reviews and for helping me to find the good lib names for each distribution. In uploaded patch, I moved getJhbuildIncludedFilePaths sub from webkitdirs.pm to update-webkit-libs-jhbuild script to avoid issues when other ports (win port) use webkitdir.pm and perl-libXML is not installed. I also updated install-dependencies.
Comment on attachment 324009 [details] Patch Clearing flags on attachment: 324009 Committed r223536: <https://trac.webkit.org/changeset/223536>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35028586>
(In reply to Michael Catanzaro from comment #10) > The package name variations for this on the different distros are > incredible... I believe the correct Fedora package is "perl-libxml-perl". Wrong... it is perl-XML-LibXML. I don't pretend to understand
Committed r223943: <https://trac.webkit.org/changeset/223943>