GNOME Bugzilla – Bug 709746
pango.trigger issues for win32 builds
Last modified: 2021-05-17 15:58:51 UTC
jhbuild 3.5.91, linux mint 15 olivia. Build target is i586-mingw32msvc-g++ These run natively using wine, so the build is not a cross build, just a non-default target. /usr/share/jhbuild/triggers/pango.trigger contains essentially these three lines. I have issues with each one. # IfExecutable: pango-querymodules # REMatch: /lib.*/pango/.*/modules/.*\.so pango-querymodules > $JHBUILD_PREFIX/etc/pango/pango.modules Issues: 1. Testing for pango-querymodules is wrong because that is not what pango's build process creates. It should be something like pango-querymodules$(EXEEXT) Why do the check? Immediately before the check, jhbuild has already checked that pango has been installed. Not a big deal: one symlink later and I'm in business. 2. The REMatch looks for ".so". Again, that's not what the build process creates. In my case it creates .dll files. This one I don't really see what to do about, other than perhaps dropping the check. 3. The command -- missing $(EXEEXT) again -- dumps its output in "etc". That is a strange location given that the input is in "lib". "etc" is really "share"'s older cousin and should be architecture independent. Note, that "pango-querymodules --update-cache" uses "lib". I am guessing this all boils down to this: I would like to be able to override jhbuild's triggers or perhaps trigger directory somehow. I have similar problems with gdk-pixbuf.trigger. Interestingly enough gtk+'s trigger actually works for me -- it runs the command for the wrong platform, but its input and output are architecture independent.
I have no experience myself with using jhbuild for non-native builds; I didn't even consider it when adding the triggers mechanism. Open to patches here, but I have no offhand suggestions for what they would be. As for issue 1) - the rationale here was that we don't have a strong association between what triggers are run and what modules are installed; instead they should only run if they need to. This covers cases like "jhbuild buildone someapp" where someapp installs GSettings schemas, but you don't have glib built in your jhbuild root. We could probably change it to do just do what the gnome-continuous triggers do: https://git.gnome.org/browse/gnome-continuous/tree/src/triggers/0070pango.trigger#n22 i.e. use $(which).
I've seen this as well -- just helped a coworker who had a GTK install with completely broken fonts and warnings about a missing pango cache. 'jhbuild postinst' fixed it. The pango trigger clearly needs to be run more often...
Created attachment 296636 [details] [review] [PATCH] triggers: patch regexp matches A couple of triggers (most noticeably pango) had their triggers defined as a filename containing "/lib/" appearing in the manifest. Now that the manifest contains relative paths, this was failing to match the "lib/" that is now at the start of the line. Change the expression to use ^ instead of /.
Created attachment 296649 [details] [review] [PATCH] triggers: patch regexp matches Fix the share/ ones too.
Review of attachment 296649 [details] [review]: 09:40 < fredp> desrt: oh, I thought I said yes already. 09:40 < fredp> desrt: so, well, I say it now.
Pushed as ceaa031.
This doesn't look fixed. All three issues seem unaddressed.
Morten: if you're going to reopen a bug, it helps to state the evidence that you have that convinces you that both the person writing the patch and the maintainer of the module were wrong...
Ryan: eh? Let's see. Item 1: we still have "# IfExecutable: pango-querymodules" which fails to check for the right executable. That would involve using $(EXEEXT). Item 2: the patch still has # REMatch: ^lib.*/pango/.*/modules/.*\.so which clearly still looks for ".so" and not for whatever shared libraries are called on the system in question. Item 3: that does indeed appear to have been addressed by someone else in the meantime. My bad. I'm sure the regexp patches solve a problem -- it just isn't the one I filed this bug about.
Indeed -- I failed to notice that the original report was clearly about Windows build issues. Thanks for pointing that out. Morten: Can you provide a bit more information about your setup here? I guess your wine setup is binfmt-based, but how does that work with shellscripts, precisely? For example, if pango.trigger runs: pango-querymodules --update-cache is it your intention that that runs the cross-built pango-querymodules or that it will run the system version? I guess it wouldn't find the cross-build version unless we added .exe to that line in the script, right? fwiw, the IfExecutable part alone is correct, since it works via cmd.has_command() which contains this: # also check for cmd.exe on Windows if sys.platform.startswith('win') and os.path.exists(prog + ".exe"): return True but clearly this won't work for cross-builds, since 'sys.platform' will be 'linux'. Probably we need a fix there for identifying cross-builds as well (or just always check '.exe'). As for the .so check, indeed, let's just drop the extension. It's no big deal if we end up running the trigger too often, but by looking in that dir, I don't think we're exactly getting a lot of non-.so files in there anyway... The real problem here is definitely the pango-querymodules system vs. jhbuilt issue, though. Running the system version is not likely to do very much with .dll files, so I assume that we need to run the jhbuilt one. Your "just symlink" approach fixed the real problem here by accident. Unless we always want to make that symlink, we're going to need to figure out how to solve it properly.
Created attachment 297499 [details] [review] triggers: drop '.so' from regexps for triggers Don't hardcode the assumption that we're going to be on a system that uses *.so as library names. This will fix running of triggers on Mac OS and Windows builds.
Created attachment 297514 [details] [review] jhbuild postinst: don't run uninstalled triggers 'jhbuild postinst' will currently blindly run all triggers, even if the IfExecutable condition is not met. Make sure the condition is met before running each trigger. This patch is also important because a future patch will add the full path of the located 'IfExecutable' executable as an argument to the command to run, and this is impossible to do if we can't find that executable.
Created attachment 297515 [details] [review] triggers: drop '.so' from regexps for triggers Don't hardcode the assumption that we're going to be on a system that uses *.so as library names. This will fix running of triggers on Mac OS and Windows builds.
Created attachment 297516 [details] [review] cmds: add find_command() helper Add a utility function to find the full path of a program in $PATH, including the possibility that it ends with '.exe', based on the code of the old has_command() function. Reimplement has_command() as a wrapper around the new command.
Created attachment 297517 [details] [review] cmds: always search for .exe files We may be doing cross-builds to Windows on non-Windows machines, so remove the check for Windows before looking for ".exe" files in cmd.find_command(). This allows for binfmt-based execution of triggers. We still follow the $PATH order, and we still prefer the version without ".exe" within each $PATH directory. This means that we will find a .exe file only if it is earlier in the PATH than a non-.exe version. This will be exactly the case when crossbuilding. It's also highly unlikely that we would find any .exe files at all in the PATH, on a Unix system, except during crossbuilds.
Created attachment 297519 [details] [review] triggers: pass executable path as first argument If a trigger has an IfExecutable condition, then pass the full path of the executable that we found as the first argument to the trigger, when running it. Our searching for IfExecutable results includes support for appending ".exe" to the end of a filename when searching in $PATH. This will allow triggers to use "$1" to run the correct executable.
Created attachment 297520 [details] [review] triggers: use "$1" for all triggers Take advantage of the new executable argument to trigger scripts.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/jhbuild/-/issues/185.