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

build: don't compile and test in parallel #3495

Closed

Conversation

jbergstroem
Copy link
Member

While invoking make -j2 test-addons the rules all and test-build (as part of build-addons) would be run simultaneously. Avoid this by telling make to not run build-addons in parallel.

/R=@bnoordhuis ?

@jbergstroem
Copy link
Member Author

@jbergstroem
Copy link
Member Author

CI broken because of npm -- I'll rebase once the dust settles.

@mscdex mscdex added the build Issues and PRs related to build files or the CI. label Oct 23, 2015
@jasnell
Copy link
Member

jasnell commented Oct 23, 2015

Landed the npm fix...

While invoking `make -j2 test-addons` the rules `all` and `test-build`
(as part of `build-addons`) would be run simultaneously. Avoid this by
telling `make` to not run `build-addons` in parallel.
@jbergstroem jbergstroem force-pushed the fix/test-build-ordering branch from 8936dfc to a5c9ad3 Compare October 23, 2015 04:24
@jbergstroem
Copy link
Member Author

@bnoordhuis
Copy link
Member

.NOTPARALLEL makes every target serial, doesn't it? I don't think it restricts it to just its prerequisites.

@jbergstroem
Copy link
Member Author

Correct; couldn't find a better alternative.

@jbergstroem
Copy link
Member Author

To elaborate; make -j5 build-addons (or in my case, make -j5 test-addons will run both prerequisites of build-addons and its targets serially. This won't impact its prerequisites target though -- all will still be run in parallel if invoked from build-addons.

@bnoordhuis
Copy link
Member

I guess using .NOTPARALLEL is alright but I'd move it to the top of the Makefile and drop the prerequisite to make it more visible and less deceiving to the casual reader.

@jbergstroem
Copy link
Member Author

@bnoordhuis I placed it next to .PHONY. Should I move both? Also, which prereq are you talking about dropping? test needs build which needs all.

@bnoordhuis
Copy link
Member

I mean .NOTPARALLEL's build-addons prerequisite. If you're using it to document why parallel builds are disabled, a comment is probably in order to explain to casual readers that all targets are serial and why.

@jbergstroem
Copy link
Member Author

Closing since NOTPARALLEL seems to make all targets serial. I think the first post sums up the issue I was having if anyone else runs into this.

@jbergstroem jbergstroem closed this Nov 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants