Clean up variables for temporary directory #9961
Closed
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
Summary: Having all of TMPD, TMPDIR and TEST_TMPDIR as configuration parameters is confusing. This change simplifies a number of things by standardizing on TEST_TMPDIR, while still recognizing the old names also. In detail: * crash_test.mk also needs to use TEST_TMPDIR for crash test, so put in shared common.mk (an upgrade of python.mk) * Always exporting TEST_TMPDIR eliminates the need to propagate TMPD or export TEST_TMPDIR in selective places. * Use --tmpdir option to gnu_parallel so that it doesn't need TMPDIR environment variable * Remove obsolete parloop and parallel_check Makefile targets * Remove undefined, unused function ResetTmpDirForDirectIO() Test Plan: manual
|
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
riversand963
approved these changes
May 6, 2022
mrambacher
reviewed
May 8, 2022
| $(parallel_redir)' \ | ||
|
|
||
| CLEAN_FILES += t LOG $(TMPD) | ||
| CLEAN_FILES += t LOG $(TEST_TMPDIR) |
There was a problem hiding this comment.
I am not sure this (or the changed to rm TEST_TMPDIR) are good ideas. If I set TEST_TMPDIR to /tmp, won't the changes will attempt to delete everything there, not just the stuff related to RocksDB or this run?
riversand963
added a commit
to riversand963/rocksdb
that referenced
this pull request
May 9, 2022
Summary: tools/check_format.sh does `make clean` a few times during execution. A recent change facebook#9961 changes the behavior of `make clean` so that it also deletes $TEST_TMPDIR where check_format.sh keeps scripts and test data. Since then, format-compatibility checking has been failing. To fix, add a new target to Makefile and `make clean` does not remove $TEST_TMPDIR. Test Plan: ``` $tools/check_format.sh ```
pdillinger
added a commit
to pdillinger/rocksdb
that referenced
this pull request
May 9, 2022
Summary: facebook#9961 broke format_compatible check because of `make clean` referencing TEST_TMPDIR. The Makefile behavior seems reasonable to me, so here's a fix in check_format_compatible.sh Test Plan: manual run: SHORT_TEST=1 ./tools/check_format_compatible.sh
facebook-github-bot
pushed a commit
that referenced
this pull request
May 9, 2022
Summary: #9961 broke format_compatible check because of `make clean` referencing TEST_TMPDIR. The Makefile behavior seems reasonable to me, so here's a fix in check_format_compatible.sh Apparently I also included removing a redundant part of our CircleCI config. Pull Request resolved: #9970 Test Plan: manual run: SHORT_TEST=1 ./tools/check_format_compatible.sh Reviewed By: riversand963 Differential Revision: D36258172 Pulled By: pdillinger fbshipit-source-id: d46507f04614e888b414ff23b88d040ae2b5c294
isaac-io
pushed a commit
to speedb-io/speedb
that referenced
this pull request
Aug 19, 2022
This picks up the "Clean up variables for temporary directory" commit (e03d958) from RocksDB for easier upstreaming and merging in the future. Summary: Having all of TMPD, TMPDIR and TEST_TMPDIR as configuration parameters is confusing. This change simplifies a number of things by standardizing on TEST_TMPDIR, while still recognizing the old names also. In detail: * crash_test.mk also needs to use TEST_TMPDIR for crash test, so put in shared common.mk (an upgrade of python.mk) * Always exporting TEST_TMPDIR eliminates the need to propagate TMPD or export TEST_TMPDIR in selective places. * Use --tmpdir option to gnu_parallel so that it doesn't need TMPDIR environment variable * Remove obsolete parloop and parallel_check Makefile targets * Remove undefined, unused function ResetTmpDirForDirectIO() Pull Request resolved: facebook/rocksdb#9961 Test Plan: manual + CI Reviewed By: riversand963 Differential Revision: D36212178 Pulled By: pdillinger fbshipit-source-id: b76c1876c4f4d38b37789c2779eaa7c3026824dd
isaac-io
pushed a commit
to speedb-io/speedb
that referenced
this pull request
Aug 19, 2022
This picks the "Clean up variables for temporary directory" commit (e03d958) from RocksDB for easier upstreaming and merging in the future. Summary: Having all of TMPD, TMPDIR and TEST_TMPDIR as configuration parameters is confusing. This change simplifies a number of things by standardizing on TEST_TMPDIR, while still recognizing the old names also. In detail: * crash_test.mk also needs to use TEST_TMPDIR for crash test, so put in shared common.mk (an upgrade of python.mk) * Always exporting TEST_TMPDIR eliminates the need to propagate TMPD or export TEST_TMPDIR in selective places. * Use --tmpdir option to gnu_parallel so that it doesn't need TMPDIR environment variable * Remove obsolete parloop and parallel_check Makefile targets * Remove undefined, unused function ResetTmpDirForDirectIO() Pull Request resolved: facebook/rocksdb#9961 Test Plan: manual + CI Reviewed By: riversand963 Differential Revision: D36212178 Pulled By: pdillinger fbshipit-source-id: b76c1876c4f4d38b37789c2779eaa7c3026824dd
isaac-io
pushed a commit
to speedb-io/speedb
that referenced
this pull request
Aug 22, 2022
This picks the "Clean up variables for temporary directory" commit (e03d958) from RocksDB for easier upstreaming and merging in the future. Summary: Having all of TMPD, TMPDIR and TEST_TMPDIR as configuration parameters is confusing. This change simplifies a number of things by standardizing on TEST_TMPDIR, while still recognizing the old names also. In detail: * crash_test.mk also needs to use TEST_TMPDIR for crash test, so put in shared common.mk (an upgrade of python.mk) * Always exporting TEST_TMPDIR eliminates the need to propagate TMPD or export TEST_TMPDIR in selective places. * Use --tmpdir option to gnu_parallel so that it doesn't need TMPDIR environment variable * Remove obsolete parloop and parallel_check Makefile targets * Remove undefined, unused function ResetTmpDirForDirectIO() Pull Request resolved: facebook/rocksdb#9961 Test Plan: manual + CI Reviewed By: riversand963 Differential Revision: D36212178 Pulled By: pdillinger fbshipit-source-id: b76c1876c4f4d38b37789c2779eaa7c3026824dd
isaac-io
pushed a commit
to speedb-io/speedb
that referenced
this pull request
Aug 25, 2022
This picks the "Clean up variables for temporary directory" commit (e03d958) from RocksDB for easier upstreaming and merging in the future. Summary: Having all of TMPD, TMPDIR and TEST_TMPDIR as configuration parameters is confusing. This change simplifies a number of things by standardizing on TEST_TMPDIR, while still recognizing the old names also. In detail: * crash_test.mk also needs to use TEST_TMPDIR for crash test, so put in shared common.mk (an upgrade of python.mk) * Always exporting TEST_TMPDIR eliminates the need to propagate TMPD or export TEST_TMPDIR in selective places. * Use --tmpdir option to gnu_parallel so that it doesn't need TMPDIR environment variable * Remove obsolete parloop and parallel_check Makefile targets * Remove undefined, unused function ResetTmpDirForDirectIO() Pull Request resolved: facebook/rocksdb#9961 Test Plan: manual + CI Reviewed By: riversand963 Differential Revision: D36212178 Pulled By: pdillinger fbshipit-source-id: b76c1876c4f4d38b37789c2779eaa7c3026824dd
isaac-io
pushed a commit
to speedb-io/speedb
that referenced
this pull request
Aug 25, 2022
This picks the "Clean up variables for temporary directory" commit (e03d958) from RocksDB for easier upstreaming and merging in the future. Summary: Having all of TMPD, TMPDIR and TEST_TMPDIR as configuration parameters is confusing. This change simplifies a number of things by standardizing on TEST_TMPDIR, while still recognizing the old names also. In detail: * crash_test.mk also needs to use TEST_TMPDIR for crash test, so put in shared common.mk (an upgrade of python.mk) * Always exporting TEST_TMPDIR eliminates the need to propagate TMPD or export TEST_TMPDIR in selective places. * Use --tmpdir option to gnu_parallel so that it doesn't need TMPDIR environment variable * Remove obsolete parloop and parallel_check Makefile targets * Remove undefined, unused function ResetTmpDirForDirectIO() Pull Request resolved: facebook/rocksdb#9961 Test Plan: manual + CI Reviewed By: riversand963 Differential Revision: D36212178 Pulled By: pdillinger fbshipit-source-id: b76c1876c4f4d38b37789c2779eaa7c3026824dd
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.
Summary: Having all of TMPD, TMPDIR and TEST_TMPDIR as configuration
parameters is confusing. This change simplifies a number of things by
standardizing on TEST_TMPDIR, while still recognizing the old names
also. In detail:
shared common.mk (an upgrade of python.mk)
export TEST_TMPDIR in selective places.
environment variable
Test Plan: manual + CI