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

[python] interpreter_constraints is not used as the default IC for user resolves. #17978

Closed
jsirois opened this issue Jan 12, 2023 · 9 comments
Closed
Labels
backend: Python Python backend-related issues bug

Comments

@jsirois
Copy link
Contributor

jsirois commented Jan 12, 2023

As of Pants 2.16.0.dev4 (main HEAD currently), the following configuration is needed to ensure the python-default resolve gets the repo-local default IC and not the Pants-embedded default IC:

pants/pants.toml

Lines 130 to 147 in 05b9465

[python]
# N.B.: When adjusting this to support new Python versions, you must update the Pants
# `python_distrobution` targets, currently:
# + src/python/pants:pants-packaged
# + src/python/pants/testutil:testutil_wheel
interpreter_constraints = [">=3.7,<3.10"]
macos_big_sur_compatibility = true
enable_resolves = true
[python.resolves]
python-default = "3rdparty/python/user_reqs.lock"
helm-post-renderer = "src/python/pants/backend/helm/subsystems/post_renderer.lock"
helm-k8s-parser = "src/python/pants/backend/helm/subsystems/k8s_parser.lock"
[python.resolves_to_interpreter_constraints]
# Without setting this explicitly, the 'python-default' resolve uses the Pants default
# ">=3.7,<4" IC, which is incorrect and can lead to a failed lock for systems with >=3.10.
python-default = [">=3.7,<3.10"]

This feels like a bug. If the default ICs written down by the user are not picked as the default IC for a resolve written down by a user, what is the default IC used for then?

@jsirois jsirois added the bug label Jan 12, 2023
@tdyas tdyas added the backend: Python Python backend-related issues label Jan 15, 2023
@cognifloyd
Copy link
Member

@benjyw Does the recent default IC deprecation resolve this?

@benjyw
Copy link
Contributor

benjyw commented May 20, 2023

AFAICT this isn't necessary now, but the proof is in the CI run for #19065.

Note that running ./pants generate-lockfiles --resolve=python-default with that change yielded no changes at all to the lockfile, and running tests and packaging worked.

However, I'm not sure why this was ever necessary. I synced back to #17957 and the same diff worked in the same way.

@benjyw
Copy link
Contributor

benjyw commented May 20, 2023

These lines (which are all the places I've found where resolves_to_interpreter_constraints is consulted) imply that the right thing is happening:

resolve, python_setup.interpreter_constraints

python_setup.interpreter_constraints,

request.resolve, python_setup.interpreter_constraints

@benjyw
Copy link
Contributor

benjyw commented May 20, 2023

@jsirois am I missing something? Do you remember what the evidence was that this was necessary?

@jsirois
Copy link
Contributor Author

jsirois commented May 20, 2023

If I go back to the linked Pex 2.1.120 upgrade PR where it hit main: 505ab26

Then comment out the edit:

jsirois@Gill-Windows:~/dev/pantsbuild/pants ((505ab264f7...) *) $ git diff pants.toml
diff --git a/pants.toml b/pants.toml
index ed9942170e..dd083c5aa4 100644
--- a/pants.toml
+++ b/pants.toml
@@ -144,7 +144,7 @@ helm-k8s-parser = "src/python/pants/backend/helm/subsystems/k8s_parser.lock"
 [python.resolves_to_interpreter_constraints]
 # Without setting this explicitly, the 'python-default' resolve uses the Pants default
 # ">=3.7,<4" IC, which is incorrect and can lead to a failed lock for systems with >=3.10.
-python-default = [">=3.7,<3.10"]
+#python-default = [">=3.7,<3.10"]

 [python-infer]
 assets = true

I find:

$ ./build-support/bin/generate_all_lockfiles.sh --pex
12:46:36.73 [WARN] DEPRECATED: `pants.engine.environment.Environment` is scheduled to be removed in version 2.17.0.dev0.

Use `pants.engine.env_vars.EnvironmentVars`.
12:46:36.73 [WARN] DEPRECATED: `pants.engine.environment.EnvironmentRequest` is scheduled to be removed in version 2.17.0.dev0.

Use `pants.engine.env_vars.EnvironmentVarsRequest`.
12:46:36.74 [WARN] DEPRECATED: `pants.engine.environment.CompleteEnvironment` is scheduled to be removed in version 2.17.0.dev0.

Use `pants.engine.env_vars.CompleteEnvironmentVars`.
12:47:03.75 [INFO] Completed: Generate lockfile for python-default
12:47:03.75 [ERROR] 1 Exception encountered:

Engine traceback:
  in select
    ..
  in pants.core.goals.generate_lockfiles.generate_lockfiles_goal
    `generate-lockfiles` goal

Traceback (most recent call last):
  File "/home/jsirois/dev/pantsbuild/pants/src/python/pants/engine/internals/selectors.py", line 626, in native_engine_generator_send
    res = rule.send(arg) if err is None else rule.throw(throw or err)
  File "/home/jsirois/dev/pantsbuild/pants/src/python/pants/core/goals/generate_lockfiles.py", line 423, in generate_lockfiles_goal
    for req in all_requests
  File "/home/jsirois/dev/pantsbuild/pants/src/python/pants/engine/internals/selectors.py", line 360, in MultiGet
    return await _MultiGet(tuple(__arg0))
  File "/home/jsirois/dev/pantsbuild/pants/src/python/pants/engine/internals/selectors.py", line 167, in __await__
    result = yield self.gets
  File "/home/jsirois/dev/pantsbuild/pants/src/python/pants/engine/internals/selectors.py", line 626, in native_engine_generator_send
    res = rule.send(arg) if err is None else rule.throw(throw or err)
  File "/home/jsirois/dev/pantsbuild/pants/src/python/pants/backend/python/goals/lockfile.py", line 190, in generate_lockfile
    cache_scope=ProcessCacheScope.PER_SESSION,
  File "/home/jsirois/dev/pantsbuild/pants/src/python/pants/engine/internals/selectors.py", line 120, in __await__
    result = yield self
pants.engine.process.ProcessExecutionFailure: Process 'Generate lockfile for python-default' failed with exit code 1.
stdout:

stderr:
pid 10034 -> /home/jsirois/.cache/pants/named_caches/pex_root/venvs/0a632cc5fc321767d8f0f1a96c21966cc516ae56/e24c8aee663ef1f4e963687618d4c779b1b907f5/bin/python -sE /home/jsirois/.cache/pants/named_caches/pex_root/venvs/0a632cc5fc321767d8f0f1a96c21966cc516ae56/e24c8aee663ef1f4e963687618d4c779b1b907f5/pex --disable-pip-version-check --no-python-version-warning --exists-action a --no-input --isolated -q --cache-dir /home/jsirois/.cache/pants/named_caches/pex_root/pip_cache --log /tmp/pants-sandbox-u5Q2cY/.tmp/pex-pip-log.1yqhdtbe/pip.log download --dest /tmp/pants-sandbox-u5Q2cY/.tmp/tmpg8hvlc02/home.jsirois..pyenv.versions.3.10.11.bin.python3.10 PyYAML<7.0,>=6.0 ansicolors==1.1.8 beautifulsoup4==4.11.1 chevron==0.14.0 debugpy==1.6.0 fastapi==0.78.0 fasteners==0.16.3 freezegun==1.2.1 humbug==0.2.7 ijson==3.1.4 importlib_resources==5.0.* mypy-typing-asserts==0.1.1 packaging==21.3 pex==2.1.120 psutil==5.9.0 pydevd-pycharm==203.5419.8 pytest<7.1.0,>=6.2.4 python-gnupg==0.4.9 python-lsp-jsonrpc==1.0.0 requests[security]>=2.28.1 setproctitle==1.3.2 setuptools<64.0,>=63.1.0 starlette==0.19.1 strawberry-graphql[fastapi]==0.114.0 toml==0.10.2 types-PyYAML==6.0.3 types-freezegun==1.1.6 types-requests==2.28.1 types-setuptools==62.6.1 types-toml==0.10.8 typing-extensions==4.3.0 uvicorn[standard]==0.17.6 --index-url https://pypi.org/simple/ --retries 5 --timeout 15 exited with 1 and STDERR:
ERROR: Could not find a version that satisfies the requirement pex==2.1.120
ERROR: No matching distribution found for pex==2.1.120




Use `--keep-sandboxes=on_failure` to preserve the process chroot for inspection.


Traceback (most recent call last):
  File "/tmp/pants-sandbox-a79QM5/./.cache/pex_root/venvs/101fe39c3b8494a801d84acf4cbdcf13f391c891/d6c8637f58f115f4f4b928f4612e8865e7f6a763/pex", line 270, in <module>
    runpy.run_module(module_name, run_name="__main__", alter_sys=True)
  File "/home/jsirois/.pyenv/versions/3.7.16/lib/python3.7/runpy.py", line 205, in run_module
    return _run_module_code(code, init_globals, run_name, mod_spec)
  File "/home/jsirois/.pyenv/versions/3.7.16/lib/python3.7/runpy.py", line 96, in _run_module_code
    mod_name, mod_spec, pkg_name, script_name)
  File "/home/jsirois/.pyenv/versions/3.7.16/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/tmp/pants-sandbox-a79QM5/build-support/bin/_generate_all_lockfiles_helper.py", line 244, in <module>
    main()
  File "/tmp/pants-sandbox-a79QM5/build-support/bin/_generate_all_lockfiles_helper.py", line 233, in main
    update_internal_lockfiles(specified=["python-default"])
  File "/tmp/pants-sandbox-a79QM5/build-support/bin/_generate_all_lockfiles_helper.py", line 205, in update_internal_lockfiles
    subprocess.run(args, check=True)
  File "/home/jsirois/.pyenv/versions/3.7.16/lib/python3.7/subprocess.py", line 512, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['./pants', '--concurrent', "--python-interpreter-constraints=['CPython>=3.7,<4']", '--python-enable-resolves', 'generate-lockfiles', "--resolve=['python-default']"]' returned non-zero exit status 1.

Note the IC on the last line just above.

@jsirois
Copy link
Contributor Author

jsirois commented May 20, 2023

Ah, right. So this is not a Pants core bug, but a bug in the (old) generate lockfiles script?

@benjyw
Copy link
Contributor

benjyw commented May 20, 2023

I think so, raw generate-lockfiles seems to work, I am verifying that script at that SHA right now.

@benjyw
Copy link
Contributor

benjyw commented May 20, 2023

Yep - this is the nonsense:

f"--python-interpreter-constraints={repr(PythonSetup.default_interpreter_constraints)}",

@benjyw
Copy link
Contributor

benjyw commented May 20, 2023

Yep, verified that the new script is fine, so will close this.

@benjyw benjyw closed this as completed May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues bug
Projects
None yet
Development

No branches or pull requests

4 participants