GNOME Bugzilla – Bug 693290
use of <suggests> in modulesets is confused
Last modified: 2021-05-17 15:56:31 UTC
We use <suggests> like so: <autotools id="glib"> ... <suggests> <!-- these provide gnome implementations of glib extension points --> <dep package="gvfs"/> <dep package="glib-networking"/> </suggests> </autotools> as if it somehow means "these things should be built after building glib". It doesn't mean that at all, though. 'jhbuild build --build-optional-modules glib' just builds glib and its dependencies (and --build-optional-modules is the only argument listed in --help that says anything about suggests or 'soft depends'). Meanwhile, another module uses it in a way that seems to mean "this would be nice to have to enable optional features during my build": <autotools id="libchamplain"> ... <suggests> <dep package="libsoup"/> <dep package="clutter-gtk"/> </suggests> </autotools> When building that module, --build-optional-modules has no effect on the number of packages built (vs. giving no arguments), but --ignore-suggests results in the suggests packages being ignored. I guess this probably means that glib is the example of invalid usage (and that I don't understand what --build-optional-modules is supposed to do, unless it overrides a config file saying the opposite). Outside of glib, it appears that the only module using <suggests> in a way that introduces cyclic dependencies is gnome-js-common suggesting seed.
*** Bug 693288 has been marked as a duplicate of this bug. ***
*** Bug 693289 has been marked as a duplicate of this bug. ***
My take on suggests has been that it's often used where one would have runtime dependencies. The glib one is pretty much like that - we can't make glib <depends> on gvfs since it'd create a cicular dep. But the recommended glib configuration definitely includes gvfs. Soo...dunno. Personally I think the right think is to make the current use of suggests explicitly runtime dependencies. The actual thing one might think <suggests> is for - optional dependencies, like Recommends: in Debian...well, it's just a huge new layer of complication. Maybe it'd make sense to look at after jhbuild actually understands build vs runtime.
Currently <suggests> is the same thing as <depends> with two differences: 1) you can turn it off with --ignore-suggests 2) if a module in suggests can't be built, it doesn't poison the other module
After a bit more reading it seems that what we really want to be using here is <after>. The code for dealing with these two things is slightly difficult to understand, however, and it produces strange results. It seems not to be working as documented... Assuming we can untangle it all, there is an open question here. Do we really want to install dconf/gvfs/glib-networking every time I type 'jhbuild install glib'? I'd probably prefer to have gvfs/dconf/glib-networking brought in as part of some gnome-desktop meta-module. Consider on a case-by-case basis: gvfs: totally not required for a reasonably-well-functioning GLib assuming people only want to use local files (which has more to do with the user's expectations than the program's). In short: it is very unlikely that any particular program will "notice" that this is missing. dconf: probably required for reasonably-well-functioning GLib if any program wants to actually save its settings, but not in all situations. On Mac OS, for example, we don't want dconf. Perhaps a good case for conditions. glib-networking: we could probably could make the strongest case for always installing this one since it's required for working TLS. Right now the dependency resolution algorithm seems to be bringing in gvfs/glib-networking/dconf in some combinations based on semi-random chance depending on how many passes of the dependency algorithm gets run, depending on the list of modules. If we did make this work properly with 'after' and always installed them, I think that would be an improvement, but then we would definitely want a --no-after switch to disable that in case I really did only want glib.
Created attachment 293505 [details] [review] resolver: warn on circular dependencies Enable the warning for circular dependencies in module sets. This warning being disabled by default has almost certainly led to incorrect use of <suggests> in our modulesets.
With the above patch we can see some of the problems that <suggests> is causing: W: Circular dependencies detected: gtk+ -> atk -> glib -> gvfs -> glib W: Circular dependencies detected: gtk+ -> atk -> glib -> glib-networking -> glib W: Circular dependencies detected: gtk+ -> atk -> glib -> dconf -> glib W: Circular dependencies detected: gtk+ -> glib -> dconf -> gtk+ W: Circular dependencies detected: gtk+ -> adwaita-icon-theme -> gtk+
Comment on attachment 293505 [details] [review] resolver: warn on circular dependencies Attachment 293505 [details] pushed as bfa36c2 - resolver: warn on circular dependencies
Created attachment 294259 [details] [review] 3.16: change glib <suggests> to <after> <suggests> is for packages that would ideally be available as a build dependency for this package -- not for packages that should be installed after in order to supplement functionality. That's <after>. That's the case for glib-networking, dconf and gvfs for GLib and adwaita-icon-theme for Gtk+.
Created attachment 294260 [details] [review] 3.16: adwaita-icon-theme does not depend on gtk+ Resolve a warning about a cyclic dependency (since gtk+ suggests the icon theme).
Created attachment 294261 [details] [review] core: rewrite the dependency solver, again Although this was already done a couple of years ago (bug 669554), there is room for more simplicity in the algorithm, with only small and desirable functionality changes. The new algorithm offers much more consistent handling of 'after' depends (if they are enabled): it simply installs them after the module in question, as if the user had requested that. It also removes warnings about cyclic dependencies when 'A' depends on 'B' and 'B' is after 'A'. This is a perfectly reasonable situation. The change in handling of 'after' depends will produce some surprising results when we enable 'after' by default. For example, 'dconf' is an 'after' of 'glib', but depends on 'gtk+' (for the editor) so building 'glib' will pull in 'gtk+'. That's perfectly logical -- we'll just need to figure out a way to sort it all out. The new algorithm will also abort in case a dependency cycle is detected while attempting to satisfy the hard depends of a module requested for building which is the only reasonable thing to do, in my opinion.
Created attachment 294262 [details] [review] core: stop ignoring 'after' modules The --build-optional-modules flag on the 'build' command was the only way to get jhbuild to actually consider 'after' modules. Get rid of it and turn that functionality on by default. Add a new config flag to turn it off. That makes it the same as 'suggests' now (which has a similar config flag).
Created attachment 294263 [details] [review] docs: clarify 'suggests' and 'after' Clarify the documentation for what <suggests/> and <after/> do, as well as adding documentation for the new 'ignore_after' config option.
(In reply to comment #12) > Created an attachment (id=294262) [details] [review] > core: stop ignoring 'after' modules > > The --build-optional-modules flag on the 'build' command was the only > way to get jhbuild to actually consider 'after' modules. Get rid of it > and turn that functionality on by default. Not exactly. If an <after> dependency is being built for some other reason then jhbuild will use <after> dependencies for ordering the build. <suggests> dependencies works the same way except that they are added to the build unless --ignore-suggests is passed on the command line. IOW same behavior with opposite defaults.
Created attachment 294839 [details] [review] fixup! core: rewrite the dependency solver, again
The dependency solver is one of the few jhbuild pieces with unit tests, this breaks them :) There's an easy one:
+ Trace 234600
raise DependencyCycleError(_("Could not resolve dependencies for module '%s'.") % module.name)
but there's three other ones where a more attentive look would be useful.
-- 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/163.