Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Support parallel builds (enabled by default). #357
Conversation
elopio
reviewed
Mar 1, 2016
| + -d --debug print debug information while executing (including | ||
| + backtraces) | ||
| + --no-parallel-build use only a single build job per part (defaults to | ||
| + number of CPUs) |
elopio
Mar 1, 2016
Member
This default is confusing. A little. I think I need more words, but I can't find a good way to say it.
Something like: defaults to use the number of CPUs to build parts in parallel.
The problem with this one is that it sounds like you will be building many parts at the same time, instead of building one part at a time.
kyrofa
Mar 1, 2016
Member
Hmm, how about "the default number of jobs per part is equal to the number of CPUs"?
elopio
reviewed
Mar 1, 2016
| @@ -110,6 +113,8 @@ def main(): | ||
| log.configure(log_level=log_level) | ||
| + common.set_enable_parallel_builds(not args['--no-parallel-build']) | ||
| + | ||
| try: | ||
| commands.load(cmd).main(argv=args['ARGS']) |
elopio
Mar 1, 2016
Member
I hate this common and its global variables. Could you delay using it? In here you can pass --no-parallel-build to the build main in argv, which is the only who cares about it. Would that make sense, or just make it worse?
kyrofa
Mar 1, 2016
Member
I agree with you, and I thought about that... but it honestly seemed inconsistent with how we were doing other things. I'm worried that doing that here would fragment the codebase's consistency, which may be worse than disliking how we solve an overall problem. Thoughts?
elopio
Mar 1, 2016
Member
Sure, that's a great point. Let's just keep thinking about how to get rid of all the globals.
sergiusens
Mar 1, 2016
Collaborator
It would need a rename, but this should live in the yaml module which loads the project (there, that's a new name) and keeps the state of it (state which does not correspond to an individual plugin).
kyrofa
Mar 1, 2016
Member
Yeah that sounds promising. Would we then pass around the project to each command?
sergiusens
Mar 1, 2016
Collaborator
so <command>.main(project) would work in most cases.
I correct myself; the yaml module already handle the plugins as it is part of the whole tree anyways.
This would potentially allow for parallelizing unit test run.
|
lgtm. I just hate the common and globals, but I don't have a great solution for that. What about adding some docs? It would me nice to mention that not all the plugins will use multiple CPUs. |
|
@elopio I'm always on board with more docs, though in this case I'm not sure where to put them. This is such a transparent change I don't think it'll really change anyone's life other than making things faster (for some plugins). What did you have in mind? |
|
I thought about docs/snapcraft-usage. That file IMO needs some more content to explain some of the flags. |
kyrofa
Kyle Fazzari
|
Perfect. Thanks. |
|
retest this please |
added a commit
that referenced
this pull request
Mar 1, 2016
kyrofa
merged commit 1a62686
into
snapcore:master
Mar 1, 2016
kyrofa
deleted the
kyrofa:feature/1551417/parallel_builds
branch
Mar 1, 2016
pushed a commit
to smoser/snapcraft
that referenced
this pull request
Sep 14, 2016
pushed a commit
to kalikiana/snapcraft
that referenced
this pull request
Apr 6, 2017
kyrofa commentedMar 1, 2016
Several plugins within Snapcraft support parallel builds, but Snapcraft currently doesn't take advantage of this and builds them with a single job. This PR resolves LP: #1551417 by adding support for parallel builds, and enabling them by default in the autotools, cmake, and make plugins. It also adds an option to disable parallel builds via the
--no-parallel-buildoption for the case of running in a RAM-limited environment.