add option to limit nprocs in build

Add option -P NPROC to Projects/Athena/build.sh to set the number of concurrent make/ninja processes when building a release

ping @akraszna

This merge request affects 1 package:

• Projects/Athena

Adding @akraszna ,@rbianchi as watchers

added Build master review-pending-level-1 labels

CI Result SUCCESS (hash befe827a)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22267]

cc @akraszna. This should probably be then ported to all build scripts? And why not name the option -j ?

hmm - I answered Attila's questions in the thread yesterday...

Hey is this new "add comment now" feature of gitlab when I reply to a thread? I added the comment, why do I need to tell it to really add it a second time? argh....

Happy to change the option to -j NPROC. If people are ok with the concept (I'm sensing some resistance ;-) ), I'll fix all the other scripts too.

Following the presentation that we got about CMake in a recent HSF meeting (https://indico.cern.ch/event/956862/) I was thinking about a somewhat larger re-write of these scripts. Since by now one can avoid calling the underlying build tool directly completely. Like:

cmake --build . --parallel <N>

See: https://cmake.org/cmake/help/v3.12/manual/cmake.1.html#build-tool-mode

(It's been possible since at least version 3.12...)

It's also possible to perform the installation of the project without relying on the underlying build tool at all. Like:

DESTDIR=FOO cmake --install .

See: https://cmake.org/cmake/help/v3.15/manual/cmake.1.html#install-a-project

So we could introduce a -j option for the build scripts. But:

• We absolutely need to modify all the build.sh script in sync;
• I myself would much prefer if we would upgrade to performing the build and installation steps like this.

This way, instead of having a -N argument for using Ninja, we could just have a -G argument for the build.sh scripts, which would be passed directly to the -G argument of CMake for choosing a generator. (To point out: The whole reason for this CMake layer on top of GNU Make / Ninja is that the same abstraction can be used to perform build/installation steps even when using Xcode or Visual Studio. Which can be pretty neat.)

This sounds like more of a long term solution (though a good one). Would be nice to have something quickly.

However, if folks really don't like the idea, I can just patch the build.sh script in my build wrapper.

ping?

After some rounds of updates in atlas/atlasexternals (to make it more friendly towards builds with Ninja or any other build tool), I now started a big re-write of the build scripts in !39481 (merged). Which I hope to finish sometime tomorrow...

Once ready, I'll very much count on some testing from you Charles.

I'm always happy to try to break things!

added 1 commit

• 662a8dcd - use -j instead of -P

Compare with previous version

This merge request affects 1 package:

• Projects/Athena

Adding @akraszna ,@rbianchi as watchers

CI Result SUCCESS (hash 662a8dcd)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 22329]

added review-pending-expert label and removed review-pending-level-1 label

Hi @akraszna and @leggett ,

this MR is around for a long time already. Could you please check, and decide what to do with it ?

Thanks, Pavol [as L2 MR shifter]

mentioned in merge request !39481 (merged)

I'm pretty sure that this can be closed now that !38481 (merged) is in. @leggett can you confirm? Tom - L1 shifter

It's been 3 weeks since we pinged @leggett , so I'm re-trying -- L1

sure. closing.

closed