makefile: fix parallel make check failing on the first run (#177)
#178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
isaac-io
requested review from
mrambacher
and removed request for
udi-speedb
mrambacher
approved these changes
Sep 29, 2022
| @@ -27,11 +28,11 @@ ifeq ($(BASE_TMPDIR),) | |||
| BASE_TMPDIR :=/tmp | |||
| endif | |||
| # Use /dev/shm on Linux if it has the sticky bit set (otherwise, /tmp or other | |||
| # base dir), and create a randomly-named rocksdb.XXXX directory therein. | |||
| # base dir), and create a randomly-named rocksdb.XXXXXX directory therein. | |||
There was a problem hiding this comment.
Is there a reason why you changed the randomness/added more XXX? A comment somewhere (either in the source or the PR) would suffice.
There was a problem hiding this comment.
I'll add a comment in the Makefile, but I did explain it in the commit message:
Also use 6 Xs for the mktemp template in order to support BusyBox mktemp and maybe some others as well, that require the template to end with 6 Xs exactly.
After the changes in #141, parallel `make check` started failing with the following GNU Parallel error: > Option tmpdir requires an argument This is caused by the fact that `test_config.mk` uses `?=` for setting `TEST_TMPDIR` and on make restarts this doesn't work for some reason (perhaps having to do with empty `TEST_TMPDIR` being somehow exported). Replace it with a check for empty value and unconditional assignment instead. While at it, include `test_config.mk` only on make restarts for check targets instead of forcing everything that makes use of `TEST_TMPDIR` to reuse the unit tests configuration. Also use 6 Xs for the mktemp template in order to support BusyBox mktemp and maybe some others as well, that require the template to end with 6 Xs exactly.
isaac-io
force-pushed
the
177-parallel-make-check-fails-on-the-first-run
branch
from
cc5d00b to
862d67b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After the changes in #141, parallel
make checkstarted failing with the following GNU Parallel error:This is caused by the fact that
test_config.mkuses?=for settingTEST_TMPDIRand on make restarts this doesn't work for some reason (perhaps having to do with emptyTEST_TMPDIRbeing somehow exported).Replace it with a check for empty value and unconditional assignment instead. While at it, include
test_config.mkonly on make restarts for check targets instead of forcing everything that makes use ofTEST_TMPDIRto reuse the unit tests configuration.Also use 6 Xs for the mktemp template in order to support BusyBox mktemp and maybe some others as well, that require the template to end with 6 Xs exactly.