Open huonw opened 9 years ago
This is largely by design as-written, but I think we should print the error of the build script ASAP instead of waiting for the rest of the parallel compilations to run.
As discussed on IRC, we would still want to give some indication the build script failed and possibly reprint the message (since someone doing a "fire and forget" compilation in a background terminal would miss the error if there are a lot of dependencies).
That said, is the loss in clarity of error messages worth parallelism with a build script? I imagine many build scripts (e.g. those doing code generation) would be so quick to compile and run that not much is gained, but I guess there are heavier-weight scripts, e.g. compiling a large C/C++ library, which do take a while to run... but these normally have internal parallelism (at least, Cargo passes in a parallelism number for them to use with make -jN
), so running things in parallel with a build script may actually be overloading the computer beyond what the user desired?
We could in theory add timings to produce different error messages if the build script took longer than, say, 500ms. I do think that the parallelism can indeed get overloaded quickly (N packages and J parallelism can lead to N x J parallel tasks), but I also don't think it's so bad because the quickly-finishing jobs will get weeded out quite quickly and then your huge C++ library you really want to be building in parallel.
A sketch to solve this and things like #835 in a more general sense might be to allow Cargo to be forced to execute all custom build scripts before starting to compile all (otherwise free-to-run) dependencies. Imagine a setting build_script_runs_first = true
in Cargo.toml
where we inject all .is_build()
into all StageLibraries
(obviously without causing havoc).
This would "bubble up" all StageRunCustomBuild
in the job queue, which would be guaranteed to look something like
The crate owner is free to choose this option. On the plus-side, failing build scripts stop the whole job queue immediately (with no extra work having been started). This might be a plus if failures in the build script haul dirty-ing the dependencies anyway or excessive parallelism due to the build script executing in parallel with the job queue is to be avoided. On the downside, the user looses a bit of parallelism, as everything has to wait for the darn build script.
I can't say I really waded through job_queue
so the implementation sketch might be off :-)
Hm so issues like #835 I think have been fixed otherwise by avoiding hiding errors, and I'm somewhat hesitant to fix this as it's sort of intended behavior to max out parallelism as much as possible and avoid terminating builds early.
It's true yeah we could have a manifest flag for this, but I'd imagine that it'd basically almost never get set? If it's just a debugging tool (maybe?) then a manifest flag could also be quite annoying to work with.
A manifest flag is probably too heavy-handed, especially as this would be rev-controlled and be a "pay-frequently-benefit-seldomly"-option.
User interface aside, I think all-build-scripts-finish-first
is a legitimate use case by OP.
Perhaps yeah! Argubally literally all interleavings of any one build is a legitimate use case as well, though.
```console
achala@laptop:~/GIT/mir2wasm$ cargo build
Compiling mir2wasm v0.1.0 (file:///home/achala/GIT/mir2wasm)
error: failed to run custom build command for mir2wasm v0.1.0 (file:///home/achala/GIT/mir2wasm)
process didn't exit successfully: /home/achala/GIT/mir2wasm/target/debug/build/mir2wasm-4f880614b2dd408e/build-script-build (exit code: 101)
--- stdout
running: "cmake" "/home/achala/GIT/mir2wasm/binaryen" "-DBUILD_STATIC_LIB=ON" "-DCMAKE_INSTALL_PREFIX=/home/achala/GIT/mir2wasm/target/debug/build/mir2wasm-cadbd25f2f277410/out" "-DCMAKE_C_FLAGS= -ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_C_COMPILER=/usr/bin/cc" "-DCMAKE_CXX_FLAGS= -ffunction-sections -fdata-sections -fPIC-m64" "-DCMAKE_CXX_COMPILER=/usr/bin/c++" "-DCMAKE_BUILD_TYPE=Debug"
-- Building with -std=c++11
-- Building with -msse2
-- Building with -mfpmath=sse
-- Building with -Wall
-- Building with -Werror
-- Building with -Wextra
-- Building with -Wno-unused-parameter
-- Building with -fno-omit-frame-pointer
-- Building with -fPIC
-- Building with -O0
-- Building with -g3
-- Configuring done
-- Generating done
-- Build files have been written to: /home/achala/GIT/mir2wasm/target/debug/build/mir2wasm-cadbd25f2f277410/out/build
running: "cmake" "--build" "." "--target" "install" "--config" "Debug" "--"
[ 7%] Building CXX object src/emscripten-optimizer/CMakeFiles/emscripten-optimizer.dir/optimizer-shared.cpp.o
[ 7%] Building CXX object src/asmjs/CMakeFiles/asmjs.dir/shared-constants.cpp.o
[ 14%] Built target wasm
[ 14%] Built target ast
[ 15%] Building CXX object src/emscripten-optimizer/CMakeFiles/emscripten-optimizer.dir/parser.cpp.o
[ 56%] Built target passes
[ 57%] Building CXX object src/support/CMakeFiles/support.dir/threads.cpp.o
src/asmjs/CMakeFiles/asmjs.dir/build.make:86: recipe for target 'src/asmjs/CMakeFiles/asmjs.dir/shared-constants.cpp.o' failed
CMakeFiles/Makefile2:430: recipe for target 'src/asmjs/CMakeFiles/asmjs.dir/all' failed
src/support/CMakeFiles/support.dir/build.make:206: recipe for target 'src/support/CMakeFiles/support.dir/threads.cpp.o' failed
CMakeFiles/Makefile2:595: recipe for target 'src/support/CMakeFiles/support.dir/all' failed
src/emscripten-optimizer/CMakeFiles/emscripten-optimizer.dir/build.make:86: recipe for target 'src/emscripten-optimizer/CMakeFiles/emscripten-optimizer.dir/parser.cpp.o' failed
src/emscripten-optimizer/CMakeFiles/emscripten-optimizer.dir/build.make:62: recipe for target 'src/emscripten-optimizer/CMakeFiles/emscripten-optimizer.dir/optimizer-shared.cpp.o' failed
CMakeFiles/Makefile2:485: recipe for target 'src/emscripten-optimizer/CMakeFiles/emscripten-optimizer.dir/all' failed
Makefile:129: recipe for target 'all' failed
--- stderr
In file included from /home/achala/GIT/mir2wasm/binaryen/src/emscripten-optimizer/istring.h:32:0,
from /home/achala/GIT/mir2wasm/binaryen/src/asmjs/shared-constants.h:20,
from /home/achala/GIT/mir2wasm/binaryen/src/asmjs/shared-constants.cpp:17:
/home/achala/GIT/mir2wasm/binaryen/src/support/threads.h:51:8: error: ‘function’ in namespace ‘std’ does not name a template type
std::function
@huonw Indeed build "scripts" compile and run before anything else, so it's true that it would make sense for their failure to cause global failure immediately. There are cases however when the error occurs and is shown during compilation of dependencies and not strictly after, for example when rebuilding. So do you think it still has a considerable impact today?
Note: I've not verified the behavior specified here.
Since this was discussed, we've added a --keep-going
flag (#10496), I could see it being reasonable that we don't start new rustc invocations on build script errors if we aren't already, reserving starting new invocations to --keep-going
.
E.g.
Dependencies could take a long time to compile (e.g. in servo), and fixing the problem raised by the build script may require adjustments that force many dependencies to be rebuild anyway, so it seems reasonable to at least give some indication that the build script failed immediately when it fails.