rust-lang / jobserver-rs

Apache License 2.0
69 stars 40 forks source link

Client::configure can't be used to call e.g. make #24

Closed glandium closed 2 years ago

glandium commented 4 years ago

Client::configure only sets CARGO_MAKEFLAGS, and there is no way to retrieve the value it sets in the Command because there is no API for that. So it's not possible to set MAKEFLAGS yourself to pass cargo's jobserver to GNU make in a build script.

alexcrichton commented 4 years ago

This was changed in https://github.com/alexcrichton/jobserver-rs/commit/6ac9e7ca16fd792865cf16f6d2c8155dfbf299f3 I think for compatibility reasons or maybe avoiding breakage (I forget at this point...), but adding an option to set MAKEFLAGS seems reasonable to me!

est31 commented 3 years ago

The cargo docs added by commit https://github.com/rust-lang/cargo/commit/cbf25a9b0ae5ac6f5b6da96e645b7fa6a75dc245 aren't really accurate because of this issue:

* `NUM_JOBS` - the parallelism specified as the top-level parallelism. This can
               be useful to pass a `-j` parameter to a system like `make`. Note
               that care should be taken when interpreting this environment
               variable. For historical purposes this is still provided but
               recent versions of Cargo, for example, do not need to run `make
               -j` as it'll automatically happen. Cargo implements its own
               [jobserver] and will allow build scripts to inherit this
               information, so programs compatible with GNU make jobservers will
               already have appropriately configured parallelism.

The jobserver implemented/provided by cargo to build scripts is not compatible with make jobservers. So you need to absolutely use NUM_JOBS right now to pass on the -j param, or copy over the "CARGO_MAKEFLAGS" env var to "MAKEFLAGS" so that cargo understands it...

IMO cargo should just set MAKEFLAGS.

Right now it seems that the jobserver crate only provides compatibility with make when make is the master, but not when make is the slave and the tool that uses this crate is the master.

est31 commented 3 years ago

I guess the main issue why @alexcrichton made 6ac9e7ca16fd792865cf16f6d2c8155dfbf299f3 is to allow outside MAKEFLAGS to get to sub-makes, which I agree is an important concern. However, I think some smart appending should be able to do it, e.g. one that parses the present MAKEFLAGS env var, removes any exant params that specify a jobserver, and re-adds the jobserver params of cargo.