Skip to content

Conversation

@ken2812221
Copy link
Contributor

@ken2812221 ken2812221 commented Jun 14, 2018

@fanquake
Copy link
Member

From make: Special Targets:

.NOTPARALLEL

If .NOTPARALLEL is mentioned as a target, then this invocation of make will be run serially, even if the ‘-j’ option is given. Any recursively invoked make command will still run recipes in parallel (unless its makefile also contains this target). Any prerequisites on this target are ignored.

@fanquake fanquake requested a review from theuni June 14, 2018 01:22
@laanwj
Copy link
Member

laanwj commented Jun 14, 2018

Will this make the entire build non-parallel? If so, that would be bad.

@ken2812221
Copy link
Contributor Author

ken2812221 commented Jun 14, 2018

Will this make the entire build non-parallel? If so, that would be bad.

As @fanquake posted

Any recursively invoked make command will still run recipes in parallel

The code building targets are still can run in parallel because they are in src/Makefile, this change only affect the target in the top Makefile.

@laanwj
Copy link
Member

laanwj commented Jun 14, 2018

ok utACK from me in that case

@theuni
Copy link
Member

theuni commented Jun 14, 2018

If there's a race condition, something's broken. We should clarify the dependencies rather than serializing. Any idea where things break down?

@promag
Copy link
Contributor

promag commented Jun 14, 2018

Agree with @theuni, looks like this avoids a problem instead of fixing it.

@theuni
Copy link
Member

theuni commented Jun 14, 2018

I think that this should be enough to fix it. Untested:

diff --git a/Makefile.am b/Makefile.am
index 8a8debb..3a725d3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -95,7 +95,7 @@ $(OSX_APP)/Contents/Resources/bitcoin.icns: $(OSX_INSTALLER_ICONS)
        $(MKDIR_P) $(@D)
        $(INSTALL_DATA) $< $@

-$(OSX_APP)/Contents/MacOS/Bitcoin-Qt: $(BITCOIN_QT_BIN)
+$(OSX_APP)/Contents/MacOS/Bitcoin-Qt: all-recursive
        $(MKDIR_P) $(@D)
        STRIPPROG="$(STRIP)" $(INSTALL_STRIP_PROGRAM)  $< $@

I believe the issue is that the top-level calls a sub-make for all, as well as for src/qt/bitcoin-qt, and those end up racing. Solve that by having deploy depend on the sub-make already being complete, which also matches the way the Windows dependencies are handled.

As a side-effect, this makes make deploy do what you wanted it to do in the beginning :)

@ken2812221
Copy link
Contributor Author

@theuni Fixed

@promag
Copy link
Contributor

promag commented Jun 14, 2018

utACK cf01fd6, makes sense!

@theuni
Copy link
Member

theuni commented Jun 15, 2018

@ken2812221 Thanks!

Tested locally. I can easily reproduce the racy build failure in master, and this appears to fix it.

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jun 15, 2018
cf01fd6 Avoid concurrency issue (Chun Kuan Lee)

Pull request description:

  From bitcoin#13406, changed travis job target for Mac to `all deploy`, but this could cause a race condition.
  Simply add `.NOTPARALLEL` to avoid it.
  Related jobs:
  https://travis-ci.org/bitcoin/bitcoin/jobs/391863281
  https://travis-ci.org/bitcoin/bitcoin/jobs/391907335
  Close bitcoin#13469

Tree-SHA512: 75c6585fe770dc70e6256dcdf97af37274f95a9240ed5a5cea2ca92b8411893b80327335587270351b128f56cb2e00f684db7c19b1602048132b734dad6ececa
@maflcko maflcko merged commit cf01fd6 into bitcoin:master Jun 15, 2018
@ken2812221 ken2812221 deleted the no_parallel branch June 15, 2018 18:20
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 7, 2020
cf01fd6 Avoid concurrency issue (Chun Kuan Lee)

Pull request description:

  From bitcoin#13406, changed travis job target for Mac to `all deploy`, but this could cause a race condition.
  Simply add `.NOTPARALLEL` to avoid it.
  Related jobs:
  https://travis-ci.org/bitcoin/bitcoin/jobs/391863281
  https://travis-ci.org/bitcoin/bitcoin/jobs/391907335
  Close bitcoin#13469

Tree-SHA512: 75c6585fe770dc70e6256dcdf97af37274f95a9240ed5a5cea2ca92b8411893b80327335587270351b128f56cb2e00f684db7c19b1602048132b734dad6ececa
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intermittent Mac CI failure

6 participants