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

run clean-all before switching pants python version runner scripts #7151

Merged
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7cf44e5
run clean-all before switching pants python version runner scripts
cosmicexplorer Jan 25, 2019
d16c292
add sentinel file to gitignore, with an issue link
cosmicexplorer Jan 25, 2019
c003ebc
respond to review comments
cosmicexplorer Jan 25, 2019
9d5eb2b
introduce a temporary env var to avoid triggering a clean in CI
cosmicexplorer Jan 25, 2019
78dac49
don't use an environment variable, just check tty before cleaning
cosmicexplorer Jan 28, 2019
abdb335
add set -x to trace what's failing
cosmicexplorer Jan 29, 2019
e8c3060
Revert "add set -x to trace what's failing"
cosmicexplorer Feb 3, 2019
e7e8ab1
correctly check whether we are in the CI environment
cosmicexplorer Feb 6, 2019
82d193f
try adding an env var to .travis.yml again
cosmicexplorer Feb 7, 2019
3b9d1e6
copy the single python version env var to the wheel building shards
cosmicexplorer Feb 10, 2019
0658c8b
add ONLY_USING_SINGLE_PYTHON_VERSION to all docker command lines
cosmicexplorer Mar 10, 2019
a96856e
move ONLY_USING_SINGLE_PYTHON_VERSION to the travis_ci Dockerfile
cosmicexplorer Mar 10, 2019
aecfc2b
add the env var to the ucs2 dockerfile as well
cosmicexplorer Mar 10, 2019
0a26141
use full python version for output file
cosmicexplorer Mar 10, 2019
25e0023
refactor interpreter checking logic
cosmicexplorer Mar 10, 2019
2efeb79
respond to review comments
cosmicexplorer Mar 11, 2019
075a7d2
remove references to #7152
cosmicexplorer Mar 13, 2019
0642281
move .pants-python-version to .pants.d!
cosmicexplorer Mar 13, 2019
30667d0
use interpreter constraints as the cache key and remove unnecessary e…
cosmicexplorer Mar 15, 2019
760edec
respond to review comments
cosmicexplorer Mar 15, 2019
4693413
move interpreter constraints file out of .pants.d AND make interprete…
cosmicexplorer Mar 16, 2019
107d821
add ONLY_USING_SINGLE_PYTHON_VERSION to hermetic env whitelist
cosmicexplorer Mar 16, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

move interpreter constraints file out of .pants.d AND make interprete…

…r constraints a parameter to the clean-all check function
  • Loading branch information...
cosmicexplorer committed Mar 16, 2019
commit 4693413bff01903a46e57fbabc6522e6e5347657
@@ -53,3 +53,5 @@ GRTAGS
GSYMS
GTAGS
.mypy_cache/
# Stores the version of python last used to run pants. See ./pants.
/.python-interpreter-constraints
12 pants
@@ -4,9 +4,11 @@

REPO_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}") && pwd -P)

interpreter_constraints_sentinel_file_path='.pants.d/.python-interpreter-constraints'
interpreter_constraints_sentinel_file_path='.python-interpreter-constraints'

function clean_if_interpreter_constraints_changed() {
new_interpreter_constraints="$1"

if [[ -n "${recursive_pants_shell_script_invocation:-}" ]]; then
return 0
fi
This conversation was marked as resolved by Eric-Arellano

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Jan 25, 2019

Contributor

Can use the Bash ternary idiom 😎

major_version="2"; [ "${PANTS_USE_PYTHON3}" = true ] && python_bin_name="3"
echo $major_version

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Jan 25, 2019

Author Contributor

I was thinking about that, but thought it would be unclear. However, given your framing of it as default 2 then 3, this makes sense. Thanks! This could even be outside of a function at this point.

This comment has been minimized.

Copy link
@cosmicexplorer
@@ -15,7 +17,7 @@ function clean_if_interpreter_constraints_changed() {

if [[ -f "${interpreter_constraints_file}" ]]; then
previous_interpreter_constraints="$(cat "${interpreter_constraints_file}")"
if [[ "${PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS}" == "${previous_interpreter_constraints}" ]]; then
if [[ "${new_interpreter_constraints}" == "${previous_interpreter_constraints}" ]]; then
return 0
fi
else
@@ -29,10 +31,10 @@ EOF
cat >&2 <<EOF
Different interpreter constraints were set than were used in the previous Pants run.
Clearing the cache and preparing a Python environment with these interpreter constraints:
${PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS}
${new_interpreter_constraints}
EOF
./pants clean-all
echo "${PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS}" > "${interpreter_constraints_file}"
echo "${new_interpreter_constraints}" > "${interpreter_constraints_file}"
unset recursive_pants_shell_script_invocation
}

@@ -103,7 +105,7 @@ export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="${PANTS_PYTHON_SETUP_INTERPRE

# We can currently assume the CI environment does not switch python versions within a shard.
if [[ "${ONLY_USING_SINGLE_PYTHON_VERSION:-false}" != 'true' ]]; then

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 15, 2019

Contributor

Danny and I were talking over Slack about alternatively naming this DISABLE_AUTO_CLEAN_ALL. We have to decide if we want to describe the property the environment has or what we're specifically doing to it.

Preferences from others?

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 15, 2019

Author Contributor

I think if we used DISABLE_AUTO_CLEAN_ALL and then want to do something completely different (such as eventually not clean-all), we would have to change the variable name, and it runs the danger of making this confusing part of our init more confusing. I think describing the reason why we're not doing the clean-all, then leaving a comment (which itself can be updated) about what that means is a more stable approach.

clean_if_interpreter_constraints_changed
clean_if_interpreter_constraints_changed "${PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS}"
fi

PANTS_EXE="${HERE}/src/python/pants/bin/pants_loader.py"
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.