Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix passing build jobs number (-j) to Autotools build helper #12505

Merged

Conversation

czoido
Copy link
Contributor

@czoido czoido commented Nov 10, 2022

Changelog: Fix: Give priority to -j argument set by user in recipe over the default set by conan in Autotools build helper.
Docs: omit

Closes: #12494
Closes: #12296

@czoido czoido added this to the 1.55 milestone Nov 10, 2022
@czoido czoido changed the title fix jobs handling Fix passing build jobs number (-j) to Autotools build helper Nov 10, 2022
@@ -48,16 +49,19 @@ def make(self, target=None, args=None):
str_args = self._make_args
str_extra_args = " ".join(args) if args is not None else ""
jobs = ""
if "-j" not in str_args and "nmake" not in make_program.lower():
jobs_already_passed = re.search(r"-j(\d+)", join_arguments([str_args, str_extra_args]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this should be expected. The moment Conan has a standard, defined mechanism to customize the paralelism, this shouldn't be taken into account:

  • The profile njobs should have priority to what the recipe says. In general, anything defined in the profiles should have priorities
  • Why would a recipe hardcoded the number of cores it is using in a custom arg in the helper?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Install target doesn't always support multiprocessing in Makefiles (there are examples in conan-center), so recipe must ensure robust installation regardless of njobs in profile.

@czoido czoido marked this pull request as ready for review November 14, 2022 13:54
@franramirez688 franramirez688 merged commit 857c3d1 into conan-io:develop Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants