GNOME Bugzilla – Bug 782320
No "ninjaargs" or equivalent
Last modified: 2018-04-21 04:30:13 UTC
You can change makeargs and add a -j (which is useful when used with IceCC) but there is no way to do that when things are built with ninja.
It would be useful to me too to build WebKit with -j1, since CMake modules now use ninja by default and there is no way to change that in the jhbuildrc AFAIK, see: https://bugzilla.gnome.org/show_bug.cgi?id=778784#c11
Both the Meson and CMake modtypes collect makeargs and module_makeargs, but neither of them passes them to Ninja. I'm not strongly sold on adding yet another variable, especially if you can use module_makeargs.
Created attachment 364278 [details] [review] Allow passing makeargs to Ninja-based modules
It doesn't really matter either way, but I think it would be less confusing to add a ninjaargs variable than to reuse makeargs. Just be sure to update index.docbook, moduleset.dtd, and moduleset.rnc. Reusing makeargs doesn't make (*heh*) as much sense to me, because make is not used at all. But I'm OK with that if that's your preference, Emmanuele.
Review of attachment 364278 [details] [review]: (accepted with the hesitation I mention above)
The issue about adding a new `ninjaargs` configuration option (both global and per-module) means adding a whole lot of code to handle that new option and propagate it everywhere. I'd rather not touch jhbuild *that* much, especially now that we're trying to move away from it.
(In reply to Michael Catanzaro from comment #4) > It doesn't really matter either way, but I think it would be less confusing > to add a ninjaargs variable than to reuse makeargs. Just be sure to update > index.docbook, moduleset.dtd, and moduleset.rnc. The moduleset DTDs already specify the attributes. The documentation is also missing a bunch of things for Meson and CMake. > Reusing makeargs doesn't make (*heh*) as much sense to me, because make is > not used at all. But I'm OK with that if that's your preference, Emmanuele. Meson and CMake can both use different backends; CMake defaults to Ninja, but it's entirely possible to run things without Ninja installed, and it'll fall back to make — which will then use the `makeargs` or `module_makeargs` configuration. Having an option being considered depending on whether you have a tool installed looks more like a spooky action at a distance to me, and would definitely be more confusing. This whole thing makes more sense if you define `makeargs` as "the configuration for the tool that performs the build" — which usually is `make` but could also be `ninja`. After all, internally, both the MesonModule and CMakeModule classes inherit from MakeModule. Renaming the option to `buildargs` would make more sense than adding `ninjaargs`, but it would also be a massive change.
OK, makeargs it is!
The patch works fine for me, I'm able to pass -j1 to ninja.
Attachment 364278 [details] pushed as f8c0519 - Allow passing makeargs to Ninja-based modules
This breaks use of default makeargs. For example, if I have this in my .jhbuildrc: # Verbose build output makeargs = 'V=1' It will cause all ninja-based builds to fail with: *** Building jsonrpc-glib *** [1/1] ninja V=1 -j 2 ninja: error: unknown target 'V=1', did you mean 'all'? I don’t think we should be using the same variable for make and ninja here. Example log: https://jenkins.freedesktop.org/view/GNOME%20Coverity/job/coverity/job/coverity-scan-jhbuild/2991/console
We'll have to use a separate ninjaargs variable, and require that CMake always uses the Ninja generator.
Please feel free to revert this. Sorry we didn't consider that it could break this way.
Emmanuele reverted it as: commit f54479328091b70e81b02f5ea8bd1becc614955b (HEAD -> master, origin/master, origin/HEAD) Author: Emmanuele Bassi <ebassi@gnome.org> Date: Mon Dec 18 16:45:57 2017 +0000 Revert "Allow passing makeargs to Ninja-based modules" This reverts commit f8c05196933f8c1010c5b9d163567c10c990dda7. This commit broke valid use cases of makeargs, so we're going to need a different option for Ninja. Thanks, Emmanuele. Sorry for being a spanner in the works here.
Created attachment 370554 [details] [review] Allow expressing arguments for Ninja Ninja-based modules, like MesonModule and CMakeModule, need a way to control the arguments passed to Ninja — mostly, the ability to set the `-j 1` argument to disable parallel builds. We tried with commit f8c0519693 to overload makeargs for that, but it had to be reverted because, unsurprisingly, make and ninja have different ideas about what arguments are valid, and jhbuild has global options for Make arguments. Let's try again, this time introducing a NinjaModule base class, in parallel to the existing MakeModule one; the NinjaModule class can do all the Ninja discovery and execution, and control the configuration options — both global and per module. The MesonModule class can inherit straight from NinjaModule, thus removing all the ad hoc code that used MakeModule for the set up, but then ignored its `make()` method. The CMakeModule is a bit of a nasty case, as CMake can have both a Make and a Ninja backend, which means we need to inherit from MakeModule and NinjaModule; luckily, both MakeModule and NinjaModule inherit from the shared Package class, so we don't have conflicting initializations.
A bit late, but this works for me when building Meson and CMake modules.
Meson has `makeargs` in the DTD; is that unwanted?
(In reply to Daniel Boles from comment #17) > Meson has `makeargs` in the DTD; is that unwanted? I haven't touched the various schemas.
The `makeargs` was added in commit 28d866fdbedef9633c32da799abd642f34f43b3a, as a copy-paste from cmake.
Oh, yeah, I didn't mean to blame you for that, was just wondering if it might be worth removing, since it seems not to be useful. I was thinking about this the other day and forgot until I got a notification here.
Created attachment 370556 [details] [review] Allow expressing arguments for Ninja v2: - update the DTD and RNC schemas
I think you can just push the commit, before conflicts start to appear.
Attachment 370556 [details] pushed as 57d00a9 - Allow expressing arguments for Ninja Reviewed-by: nobody Considering the lack of reviewers for Jhbuild, I went ahead and pushed the change. If somebody has any issue, feel free to re-open the bug and/or revert the commit.
I just pushed commit a8c7dcd to fix DTD and RNC schemas.