-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider supporting a SAMUFLAGS environment #15
Comments
I skimmed those discussions, and I think it's unfortunate that so many are frustrated with the outcome. Here are my current thoughts: However, if this were added as a samurai-specific feature (through Regarding the feature itself, I don't see any issue if it is limited to the flags 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, |
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 Adding |
I think I'm okay with adding this, but I have a few questions to make sure I understand a few things.
I agree this would be more convenient, but if your alias was #!/bin/sh
exec /usr/bin/samu $SAMUFLAGS "$@" you wouldn't have to constantly edit your wrapper scripts and PATH.
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 |
I did not consider adding An
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 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.
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 |
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:
With Here's the To be fair, this happens with |
I tried building a bunch of projects, using samu 0.6, one of them failed: xreader. The issue looks like this:
The funny thing is that if I run
and compilation continues. It's created via meson's And the kicker is that if I try to execute 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: 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. |
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 withninja
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.The text was updated successfully, but these errors were encountered: