Consider supporting a SAMUFLAGS environment
Closed this issue · 20 comments
This is a feature that ninja itself is not willing to support [0] [1] [2].
Much like MAKEFLAGS these kinds of environments can greatly ease development and maintainence projects. Though the exposure of such things introduce complication and should generally be avoided it may have benefits outweighing the disadvantages in this case.
I also appreciate that samurai itself is trying to maintain feature pairty with ninja so this may be rejected on those grounds alone.
Nevertheless, do you have any thoughts about adding such functionality?
Edit: The title should probably be SAMUFLAGS to maintain consistency with the binary name.
I skimmed those discussions, and I think it's unfortunate that so many are frustrated with the outcome.
Here are my current thoughts:
In general my goal with samurai are to remain as simple as possible with supporting most of the essential features of ninja (so it is a drop-in replacement in most cases). I am hesitant to add any features unique to samurai that increase complexity in any way, and also to avoid conflicts in case they are added to a future ninja version with slightly different semantics. Personally, I would have liked to see explicit scoping, but I have not implemented it for the above reasons.
However, if this were added as a samurai-specific feature (through SAMUFLAGS), there is little chance with this conflicting with any future ninja feature. I believe implementation would also be pretty easy. That said, even after reading the arguments in those threads, I still don't really understand why a wrapper script is unacceptable. I don't want to turn this into a rehash of those discussions, but maybe you could provide a concrete example with the system and tools you are using such that a wrapper script does not work?
Regarding the feature itself, I don't see any issue if it is limited to the flags -v and -j. So, I think it just comes down to whether deviating from ninja's feature set is worth it.
Let me think on this some more.
Yes I agree, I don't wish to rehash any of the arguments here since they're still continuing elsewhere. Thanks for considering it, all of your points are reasonable and expected. Although there's always a balance to consider; providing a very common convenience can result in less (potentially broken) total code in the world by adding a degree of complexity to the source (in this case, FLAGS environments).
If this was implemented it would be a huge plus in the favor of samurai over ninja... A wrapper script is a crude hack at best.
With programs that use make a common use case is to export MAKEFLAGS in the local environment to aid debugging and then use a script to wrap the build process. This way I can just change the MAKEFLAGS in my environment instead of constantly editing wrapper scripts and my PATH accordingly.
Adding -j# directly to the build script is not acceptable because different users with different computers may use the same scripts and making a wrapper script for samu itself would require root or modifying the PATH environment variable which kind of defeats the point of avoiding environment variables in the first place.
I think I'm okay with adding this, but I have a few questions to make sure I understand a few things.
With programs that use make a common use case is to export MAKEFLAGS in the local environment to aid debugging and then use a script to wrap the build process. This way I can just change the MAKEFLAGS in my environment instead of constantly editing wrapper scripts and my PATH accordingly.
I agree this would be more convenient, but if your alias was alias samu='samu $SAMUFLAGS', or your wrapper script was
#!/bin/sh
exec /usr/bin/samu $SAMUFLAGS "$@"you wouldn't have to constantly edit your wrapper scripts and PATH.
Adding -j# directly to the build script is not acceptable because different users with different computers may use the same scripts and making a wrapper script for samu itself would require root or modifying the PATH environment variable which kind of defeats the point of avoiding environment variables in the first place.
Most build systems I've seen work by first generating the .ninja files, and then you are able to build by running ninja, samu, or whatever implementation/alias/wrapper you want (i.e. it is the top-level process of the build). What type of build scripts are you referring to that are calling ninja/samu inside?
Personally, I think it's a mistake that cmake and meson have a hardcoded list of known ninja command names. I shouldn't have had to modify meson to get it to work with samu, and I shouldn't have to pass -D CMAKE_MAKE_PROGRAM=samu to get it to generate the build scripts on a system without ninja. They should just write the .ninja files containing a ninja_required_version line, and let the user decide how to execute the build.
I did not consider adding $SAMUFLAGS directly to the wrapper script, I suppose that would work even if it seems silly as it share any issues that directly using the environment variable would have.
An alias would not be acceptable because its entirely possible the alias would be dropped by a build script or unalias -a before being used and in extreme cases even does not even exist for some old and limited shells.
What type of build scripts are you referring to that are calling ninja/samu inside?
Specifically I mean shell based scripts that wrap the build and packaging process as used by Slackware. These will build the package from source using autotools, cmake, meson or etc. and are often run by regular users. For programs that use make its standard to export MAKEFLAGS somewhere in the user's environment to control things like the amount of make jobs.
Currently for ninja/samu it seems the only good solution would be to document how to create a wrapper script in the users environment, any aliases in the user's environment won't exist in the script's environment. Also note these build scripts can be run manually or by various different packaging tools.
Personally, I think it's a mistake that cmake and meson have a hardcoded list of known ninja command names. I shouldn't have had to modify meson to get it to work with samu, and I shouldn't have to pass
-D CMAKE_MAKE_PROGRAM=samuto get it to generate the build scripts on a system without ninja. They should just write the .ninja files containing aninja_required_versionline, and let the user decide how to execute the build.
I completely agree, for meson I have noticed that when I have both ninja and samu installed configure will test for ninja and then I will manually proceed to use samu...
If samurai added a $SAMUFLAGS variable to set this, I would be willing to provide first-class integration into Arch Linux's "makepkg" packaging tool. This would have the advantage of providing a single, unified way to support configuring build parallelism (like the existing $MAKEFLAGS support), and could be automatically configured by system administrators and/or forwarded into build containers (such as the "makechrootpkg" wrapper for makepkg). Unfortunately, it does not look like ninja will ever be invited to integrate into my distro packaging, as per the above-linked discussions.
As such I would really appreciate if this feature would be added (hopefully soon).
At least for makechrootpkg, using a wrapper script is completely unacceptable -- the environment variable is configured on the user's system, and in theory a (slow, runs extra process and script interpreter) wrapper script could be installed to /usr/local/bin despite being a rather inelegant design... but that wrapper script cannot be installed to the chroot without adding a large amount of samurai-specific code (proportional to what is there already), including code to check whether samurai is already present in the chroot or is scheduled to be installed -- the alternative would be that is_program_in_path samu would report "yes", while samu would report "/usr/local/bin/samu: 2: exec: /usr/bin/samu: not found". This would be ironically terrible if there is any build system out there which, say, checks for samu and ninja, discovers both commands exist, and prefers a broken samu wrapper script instead of a working ninja executable.
I'm open to adding it, but I don't really have the time right now to do it myself, and I don't have any personal use for the feature (I always use the default value or -j1).
If you want the feature added soon, the best way would be to send a pull request.
Update for those watching the issue: @andreasbaumann agreed to implement this, and there is now an initial implementation at #21
Thank you!
What is your timeline for a new stable release with this feature?
I'd like to get #18 fixed, and I can make a release after that. Hopefully I can find time for this in the next week or so.
Sounds great. :)
@eli-schwartz I'm about ready to make a release. If you are using samurai to build Arch Linux packages, it'd be helpful if you could try the current master and check for any issues. I always do my best to check several popular ninja generators, but the more packages we check the better :)
I've tested libinput, pkgconf, and weston and all but weston succeed.
In the case of weston I'm seeing these errors:
meson --prefix=/usr --libexecdir=/usr/lib/weston -Dsimple-dmabuf-drm=intel build
SAMUFLAGS=-vj2 samu -C build
...
samu: job failed: cc -Iclients/8f4d908@@weston-presentation-shm@exe -Iclients -I../clients -Iclients/.. -I../clients/.. -Iclients/../shared -I../clients/../shared -Iinclude -I../include -Iprotocol -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/lib/libffi-3.2.1/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/uuid -I/usr/include/pango-1.0 -I/usr/include/fribidi -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Wpedantic -std=gnu99 -g -Wno-unused-parameter -Wno-shift-negative-value -Wno-missing-field-initializers -Wno-pedantic -fvisibility=hidden -march=x86-64 -mtune=generic -O2 -fstack-protector --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2 -MD -MQ 'clients/8f4d908@@weston-presentation-shm@exe/presentation-shm.c.o' -MF 'clients/8f4d908@@weston-presentation-shm@exe/presentation-shm.c.o.d' -o 'clients/8f4d908@@weston-presentation-shm@exe/presentation-shm.c.o' -c ../clients/presentation-shm.c
../clients/presentation-shm.c:46:10: fatal error: xdg-shell-client-protocol.h: No such file or directory
46 | #include "xdg-shell-client-protocol.h"
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
samu: subcommand failed
With ninja -j2 -C build this seems to progress without issue.
Here's the build.ninja file: https://0x0.st/zLyk.txt
To be fair, this happens with samu without using -j flags or the environment so this may just be a general issue.
I tried building a bunch of projects, using samu 0.6, one of them failed: xreader. The issue looks like this:
[19/167] Compiling C object 'shell/ce635c4@@shell@sta/ev-sidebar-attachments.c.o'.
samu: job failed: ccache cc -Ithumbnailer/1663afd@@xreader-thumbnailer@exe -Ithumbnailer -I../thumbnailer -I. -I../ -Ilibdocument -I../libdocument -Ilibview -I../libview -Ilibmisc -I../libmisc -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/lib/libffi-3.2.1/include -I/usr/include/fribidi -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/uuid -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gio-unix-2.0 -I/usr/include/libdrm -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/dbus-1.0/include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -g '-DDATADIR="/usr/share"' '-DLIBDIR="/usr/lib"' '-DBINDIR="/usr/bin"' '-DEV_BACKENDSDIR="/usr/lib/xreader/3/backends"' '-DXREADERDATADIR="/usr/share/xreader"' -DENABLE_EPUB -Wno-deprecated-declarations -Wno-deprecated -Wno-declaration-after-statement '-DXREADER_LOCALE_DIR="/usr/share/locale"' -DXREADER_COMPILATION -DHAVE_CONFIG_H -pthread -MD -MQ 'thumbnailer/1663afd@@xreader-thumbnailer@exe/xreader-thumbnailer.c.o' -MF 'thumbnailer/1663afd@@xreader-thumbnailer@exe/xreader-thumbnailer.c.o.d' -o 'thumbnailer/1663afd@@xreader-thumbnailer@exe/xreader-thumbnailer.c.o' -c ../thumbnailer/xreader-thumbnailer.c
In file included from ../thumbnailer/xreader-thumbnailer.c:21:
../xreader-document.h:43:10: fatal error: libdocument/ev-document-type-builtins.h: No such file or directory
43 | #include <libdocument/ev-document-type-builtins.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
The funny thing is that if I run samu libdocument/ev-document-type-builtins.h then this header is generated:
[1/1] Generating ev-document-type-builtins.h with a meson_exe.py custom command.
and compilation continues. It's created via meson's gnome.mkenums(), and both the enums and other files checked into git together form the meson library('foo', sources: [generated_enums, 'a', 'bunch', 'of', 'files']).
And the kicker is that if I try to execute ninja shell/ce635c4@@shell@sta/ev-sidebar-attachments.c.o it will also fail the same way. There's no dependency relationship between those two files, but it seems ninja somehow sorts the same build.ninja to make those autogenerated files come first. Building it once and then cleaning the builddir will, however, cause .ninja_deps to incorporate the necessary dependencies and build properly the next time around.
This is an issue for the current stable 0.6 release of samurai. Although perhaps it is actually an issue for meson.
Your error seems to be in the same class as the one I encountered.
So I submitted a meson issue for this. I think it's probably meson's responsibility to get this right. Unsure what, if anything, samurai should do about this.
"It's a documentation issue."
Okay, so samurai is good at finding projects with broken meson.build definitions. I'm liking this more and more. :D
Thanks for testing. Yeah, samurai doesn't schedule jobs in the same order as ninja, I've hit issues like this before, but they are always bugs in the dependency graph. I opened a bug that I will likely close as won't fix so I have something to refer people too. I should probably add something to the README as well.
So I just finished testing all my [community] packages which currently use meson:
cinnamon-desktop cinnamon-menus cinnamon-session ksh nemo-extensions nemo xapps xed xreader
With the above noted exception of xreader, all these projects build successfully with commit 854f648, with the right maxjobs and verbose mode enabled, producing byte-identical content as compared to building with ninja or with samurai 0.6
Great! Thanks so much for testing. I have one small change I want to make, but let's move discussion over to #23.