Added support for environment variable NINJA_FLAGS

[ocroquette]

The value of the variable is parsed like command line arguments. You can for
instance use less cores than available (-j), or limit the parallelism based
on the system load (-l). For instance, on a build server used by multiple
users, you may want to set NINJA_FLAGS=-l 1 on the system level, so that
the server never gets overloaded by parallel ninja builds. Command line
parameters override the flags set with NINJA_FLAGS.

[ocroquette]

Not sure why the CI build is failing. Can somebody help me out?

[nico]

Previously #797.

[nico]

travis is failing because

src/ninja.cc:1141:16: warning: deprecated conversion from string constant to ‘char*’ [-Wwrite-strings]
   argv[argc++] = ""; // fake argv[0] (command) for getopt
                ^

(click the red x to get to that: https://travis-ci.org/ninja-build/ninja/builds/354768286?utm_source=github_status&utm_medium=notification)

[nico]

I'm generally wary of making env vars affect your build. If you have the power to set env vars for everyone, you also have the power to run alias ninja='ninja -l 1', which should address your use case as well.

[ocroquette]

"alias" is not an option on Windows, nor does it work when Ninja is called from another tool, for instance cmake. In those cases, how can you enforce custom default settings for -j and -l ?

Would it be acceptable to have dedicated environment variables like NINJA_MAX_JOBS and NINJA_MAX_LOAD? That would affect only build performance and not the build output.

Another option would be a configuration file, but it would have a bigger impact on the design.

[nico]

I'm somewhat skeptical of that too :-/ I've debugged so many build issues where in the end the resolution was "oh I had this one env var set that I set months ago and forgot about".

[ocroquette]

Like I said, the two specific environment settings can not impact the build result, so the risk of "build issues" is limited to the performance, which is very easy to troubleshoot with the documentation or FAQ.

Other build tools have something similar, for instance make has MAKEFLAGS, gradle has ~/.gradle/settings.gradle (for instance to enable/disable the daemon), Maven has ~/.m2/settings.xml, cmake has environment variables. All these tools live pretty well with the risk you mentioned, because it's more than compensated by the added value.

I am obviously not the only one who needs that, there are several valid and common use cases, so what is the alternative to give to the user the possibility to set more sensible default values?

[nico]

I agree the risk is limited to performance, but that seems like a pretty big risk in a project whose purpose is performance.

The alternative depends on your exact use case. For the one mentioned in comment 0, on linux I'd probably set the default nice level in limits.conf for each user since it sounds like you don't want this just for ninja builds. For just ninja at an admin level, I'd set the alias. For personal use, I'd manually use the flags. I'm less familiar with Windows, but similar things are probably possible there (if ninja is system-provided, put ninja -l 1 in a bat file and put that in %path%, say).

[ocroquette]

Using an alias or a BAT script has the same issues, with additional problems and limitations mentioned. So for the users who really needs this, it's better to provide a clean way.

[ocroquette]

Hi @nico. If the main concern is the troubleshooting, how about displaying a message on the console when the environment variables are used? Something like:

Setting flags from environment variable NINJA_FLAGS: -j 4 -v

Or if we want to limit this to the performance options:

Setting maximum parallel jobs from environment variable NINJA_MAX_JOBS: 4

People not using this feature would not be impacted.
People using this feature by mistake or troubleshooting someone's build would be clearly notified.
People using this feature on purpose could find the message annoying, but it's better than having nothing at all.

[Hello71]

I mean, it is extremely common to install software to /usr/local then forget about it. will you also disable installation to /usr/local?

[make-github-pseudonymous-again]

I build software from the AUR (Arch User Repository) using ninja. I am searching for a solution to always use -j1 because I still want to be able to use my system while the build runs, and the packager of the software used ninja's default which I guess is the number of cores of the cpu. Is there another way to assign this parameter by default aside the suggestion made in this pull request?

[jhasse]

Instead of an alias, you can use this simple Python script:

#!/usr/bin/env python3

import sys
import subprocess

try:
	subprocess.check_call(['/usr/bin/ninja', '-j1'] + sys.argv[1:])
except subprocess.CalledProcessError as e:
	exit(e.returncode)

Name it ninja, make it executable with chmod +x and put it somewhere in your PATH (e.g. at /usr/local/bin).

Not sure if this will work on Windows, too?

@ocroquette Can you name some limitations with this script approach? It should work with CMake for example.

[jhasse]

It takes 50 ms on my system. Is that really noticeable in your builds? If I understand it correctly the use-case for this seems to be full rebuilds anyway.

[ocroquette]

Thanks for the idea, but I can only repeat my comment above:

Using an alias or a BAT [or python or whatever] script has the same issues, with additional problems and limitations mentioned. So for the users who really needs this, it's better to provide a clean way.

It looks like the design decision here is not to support any user settings. While I can understand the concern anything that could modify the build result, it doesn't apply for the parallelism factor. It just forces the user to resort to cumbersome solutions which doesn't need to be.

[jhasse]

has the same issues, with additional problems and limitations mentioned.

Correct me if I'm wrong here, but I only see three issues mentioned here:

1. "is not an option on Windows"

Python should work on Windows.

2. "nor does it work when Ninja is called from another tool, for instance cmake."

If it's on the PATH, it should work with CMake.

3. "In those cases, how can you enforce custom default settings for -j and -l ?"

The Python script can remove those from sys.argv.

So no, it does not have the same issues as an alias has.

[ocroquette]

Python doesn't solve anything. If I wanted to solve this with an additional script, I would use a language supported natively by the platform anyway (for instance CMD on Windows and SH on Unix-likes), so that I don't have to deploy Python on every developer PC and build server of the company just to pass an integer to Ninja.

Relying on a wrapper script is in general not a good option, our whole build system is complex enough by nature that we don't need a hack that relies on external files and the order of directories in PATH.

The issues I was referring to are the one mentioned by nico, the main one being that it is possible to forget about the custom settings values. Refusing to do this in Ninja just moves the problem elsewhere.

[jhasse]

If I wanted to solve this with an additional script, I would use a language supported natively by the platform anyway (for instance CMD on Windows and SH on Unix-likes)

Of course that's also possible, I've just used Python, because I know it better.

we don't need a hack that relies on external files

It's only one external file.

and the order of directories in PATH.

Because you don't like reliance on env variables? That's exactly the reason why we are against NINJA_FLAGS.

The issues I was referring to are the one mentioned by nico, the main one being that it is possible to forget about the custom settings values. Refusing to do this in Ninja just moves the problem elsewhere.

Yes, but I think this way it's a bit more explicit. Also moving it out of Ninja is exactly the point for a feature that won't be used by most of Ninja's users.

[make-github-pseudonymous-again]

A working solution then is to create a directory /etc/mock/ninja/j1, put the ninja python or sh or whatever script in there, then when one needs to set the option -j1 by default while running another executable some_executable, use env PATH=/etc/mock/ninja/j1:$PATH some_executable ....

That would not prevent some_executable to request a specific path for ninja (for instance /usr/bin/ninja). Hence, it might not work in some situations. However, this is good enough for my use case as the calling executable relies on $PATH.

Am I missing anything?

[fbbdev]

@jonesmz I agree with you that filtering NINJA_FLAGS would be inconvenient (and counterintuitive too). If it gets implemented, it should come with full power.

But I'm really concerned with a particular and IMO concretely possible instance of your third case, i.e. a sysadmin with arcane intentions, or some idiot build tool, setting NINJA_FLAGS to a perverse default, which an unknowing user will then be forced to debug. And in my debugging experience a lot of time will often be wasted before one remembers to check for things that are completely out of context.

So, this is not an instance of not knowing what you are doing, actually your very knowledge becomes an hindrance. Also the problem is more about collaboration than about single users.

On the other hand it's true that solving such kind of problems is not in any way ninja's responsibility, yet I strongly believe that some protection, like a warning, should be included, as @ocroquette himself suggested. Which is not convenient either, but still better than nothing.

E.g.:

ninja: warning: additional flags parsed from NINJA_FLAGS environment variable, resulting invocation is: ninja -j4 <etc>

(with all overloaded flags omitted).

However this should come in a form that avoids hurting build log parsers.

[jhasse]

@jhasse you are apparently the only maintainer involved in this discussion, so I am afraid the decision is on you :)

I'm not the only one and didn't even close this PR. Also I can't make final decisions about Ninja.

[ocroquette]

@jhasse

I'm not the only one and didn't even close this PR. Also I can't make final decisions about Ninja.

I tried to catch the attention of other maintainers, with no luck so far. How do you suggest we should proceed then?

Independently of the decision, your opinion as a maintainer matters, so it would definitely help to hear it. Do you personally think that the NINJA_FLAGS (restricted or not) is an acceptable way to solve our problem, since we didn't find any other good option in the lengthy discussion?

Just for the record, I am thankful for the free time you spend on Ninja, including on this topic. But keep in mind you are not the only one. I have already spent several hours here, time that I cannot expect my employer to account for. I am surely not the only user in this case.

[jhasse]

I tried to catch the attention of other maintainers, with no luck so far. How do you suggest we should proceed then?

Fastest way would be getting it merged in a fork, e.g. https://github.com/jonesmz/splinter
If you want it in upstream Ninja, I would start a thread on the mailing list, which the other maintainers pay more attention to than GitHub.

Independently of the decision, your opinion as a maintainer matters, so it would definitely help to hear it.

I would rather stay out of it for a while, sorry.

[ocroquette]

Fastest way would be getting it merged in a fork, e.g. https://github.com/jonesmz/splinter

It's great that @jonesmz shares his work, but forking just to have -j as a persistent setting isn't worth it, from my perspective.

If you want it in upstream Ninja, I would start a thread on the mailing list, which the other maintainers pay more attention to than GitHub.

I didn't think of that, thanks for the hint. I will do so.

[ocroquette]

This was discussed on the mailing-list, and unfortunately no maintainer thinks our problem deserves a solution.

https://groups.google.com/forum/#!topic/ninja-build/E3s9gE7RiLQ

[kobalicek]

No meson and no ninja is the solution for me in the end. Thanks to everybody involved for trying to convince the maintainers the feature was worth it.

[ocroquette]

Lack response from maintainer doesn't imply that the feature won't be merged, eventually.

We made our point and actually got responses from the 2 active maintainers, both negative.

And of course, the majority of people who are opposed to a feature like this say nothing about why. [...]
This person simply says they are opposed to it, without providing an explanation.

Since it's not possible for an environment variable that is not set to cause any harm, and all of the proposed alternatives have exactly the same downsides (someone might set the wrapper script up wrong, just like the environment variable being set up wrong) there's been no objection to really argue against.

I agree. But unfortunately, it is very easy for anyone (including maintainers) to be against any solution for a problem they are not impacted by.
I would expect more understanding from the maintainers, but if it is not there, I don't see what we can do.

[eli-schwartz]

The independent Ninja build language reimplementation https://github.com/michaelforney/samurai supports this via the environment variable SAMUFLAGS. Simply use samu anywhere you'd otherwise use ninja, or install samu as your system /usr/bin/ninja (some linux distros have options to install this competing implementation as the default, or only, ninja).

[lilydjwg]

@eli-schwartz unfortunately samu doesn't support -l which is what a multi-user build machine needs. I'll going to create a wrapper and hack the ninja command in a post-install hook instead. It's not a good and nice solution but I think we've run out of other options.

[ddevault]

Consider asking samu to implement -l, or sending them a patch, I'm sure they'd be open to it.

[Salamandar]

@lilydjwg Adding support for -l is really not difficult. Please do an issue and mention my name if they say they don't have time for it.

[eli-schwartz]

@lilydjwg there's an open feature request to add it to samurai and the developer has even said he will accept a patch for it.

michaelforney/samurai#41

[lilydjwg]

@ddevault @Salamandar @eli-schwartz thank you but I've taken another suggestion to make those processes compete fairly with cgroups because I don't need to modify the build environment this way so it's much simpler to implement. Hopefully make & ninja will play fair and give enough CPU resources to system services. (make -l & ninja -l doesn't play very well: the process numbers are unbalanced and CPU cores are under-utilized.)

[Salamandar]

I think I agree. They just "dont start jobs if load is too high", that's not a good idea for a multi-user machine. Cgroups are the way to go.

[RJVB]

I think I agree. They just "dont start jobs if load is too high", that's not a good idea for a multi-user machine. Cgroups are the way to go.

Are those available on all modern Unix variants?

[jhasse]

Depends on your definition of "modern Unix".

But I guess that's a rhetorical question anyway and you already know that cgroups are a Linux kernel feature.

[RJVB]

But I guess that's a rhetorical question anyway and you already know that cgroups are a Linux kernel feature.

Actually I didn't, just had a hunch. Here's another not-really rhetorical question: how many such a platform-specific feature can you use in what's supposed to be a lean and mean application, before it isn't anymore? ;) I agree though that the current load-limited option is (AFAICT) mostly interesting/useful if you want to allow ninja to take over the entire host without shooting itself in the foot (too much).

[eli-schwartz]

What does this have to do with anything?

Someone who previously thought samurai wasn't a good replacement for ninja because it doesn't implement -l has now decided it's perfectly fine because they don't need -l, because they'd much rather use linux cgroups before running ninja/samurai.

No one has suggested implementing or reimplementing -l to simply set up cgroups itself, so I don't know what "lean and mean application" you think you are referring to.