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

pants 2.5 dependency issues with cross-platform targets #12222

Closed
oliverdain opened this issue Jun 17, 2021 · 10 comments · Fixed by #12268
Closed

pants 2.5 dependency issues with cross-platform targets #12222

oliverdain opened this issue Jun 17, 2021 · 10 comments · Fixed by #12268

Comments

@oliverdain
Copy link

oliverdain commented Jun 17, 2021

I have a pants build with one target that tries to build a cross-platform pex file:

   platforms = [
      'linux-x86_64-cp-37-m',
      'macosx-10.14-x86_64-cp-37-m'
   ]

That built fine on pants 2.4 (and still does) but started to fail when I upgraded to pants 2.5 with the following:

(venv) oliver@marmaduke:~/Documents/code/central2/python$ ./pants package db_cli/::
10:51:26.74 [INFO] Completed: Resolving constraints.txt
10:51:26.74 [ERROR] Exception caught: (pants.engine.internals.scheduler.ExecutionError)
  File "/home/oliver/.cache/pants/setup/bootstrap-Linux-x86_64/2.5.0_py39/lib/python3.9/site-packages/pants/bin/local_pants_runner.py", line 234, in _run_inner
    return self._perform_run(goals)
  File "/home/oliver/.cache/pants/setup/bootstrap-Linux-x86_64/2.5.0_py39/lib/python3.9/site-packages/pants/bin/local_pants_runner.py", line 173, in _perform_run
    return self._perform_run_body(goals, poll=False)
  File "/home/oliver/.cache/pants/setup/bootstrap-Linux-x86_64/2.5.0_py39/lib/python3.9/site-packages/pants/bin/local_pants_runner.py", line 190, in _perform_run_body
    return self.graph_session.run_goal_rules(
  File "/home/oliver/.cache/pants/setup/bootstrap-Linux-x86_64/2.5.0_py39/lib/python3.9/site-packages/pants/init/engine_initializer.py", line 135, in run_goal_rules
    exit_code = self.scheduler_session.run_goal_rule(
  File "/home/oliver/.cache/pants/setup/bootstrap-Linux-x86_64/2.5.0_py39/lib/python3.9/site-packages/pants/engine/internals/scheduler.py", line 530, in run_goal_rule
    self._raise_on_error([t for _, t in throws])
  File "/home/oliver/.cache/pants/setup/bootstrap-Linux-x86_64/2.5.0_py39/lib/python3.9/site-packages/pants/engine/internals/scheduler.py", line 498, in _raise_on_error
    raise ExecutionError(
Exception message: 1 Exception encountered:
  ProcessExecutionFailure: Process 'Resolving constraints.txt' failed with exit code 1.
stdout:
stderr:
ERROR: Could not find a version that satisfies the requirement gast==0.2.2
ERROR: No matching distribution found for gast==0.2.2
pid 795584 -> /home/oliver/.cache/pants/named_caches/pex_root/venvs/7f48ea14fe9a31ad7cc2422df7a949fe8899372d/cc48858524bf3820a737c19c7f14d57d4a5c4208/pex --disable-pip-version-check --no-python-version-warning --exists-action a --isolated -q --cache-dir /home/oliver/.cache/pants/named_caches/pex_root --log /tmp/process-executionlRKkVb/.tmp/tmpyfr58o_p/pip.log download --dest /tmp/process-executionlRKkVb/.tmp/tmpjmhkeakd/macosx_10_14_x86_64-cp-37-cp37m --platform macosx_10_14_x86_64 --implementation cp --python-version 37 --abi cp37m --only-binary :all: --constraint constraints.txt absl-py==0.13.0 astor==0.8.1 attrs==21.2.0 backoff==1.10.0 cachetools==4.2.2 certifi==2021.5.30 chardet==4.0.0 gast==0.2.2 google-api-core==1.30.0 google-auth==1.31.0 google-cloud-core==1.7.0 google-cloud-firestore==2.1.3 google-pasta==0.2.0 googleapis-common-protos==1.53.0 grpcio==1.38.0 h5py==2.10.0 idna==2.10 importlib-metadata==4.5.0 iniconfig==1.1.1 keras-applications==1.0.8 keras-preprocessing==1.1.2 labelbox==2.6.0 markdown==3.3.4 ndjson==0.3.1 numpy==1.18.5 opencv-python-headless==4.5.2.54 opt-einsum==3.3.0 packaging==20.9 pluggy==0.13.1 proto-plus==1.18.1 protobuf==3.17.3 py==1.10.0 pyasn1-modules==0.2.8 pyasn1==0.4.8 pydantic==1.8.2 pyparsing==2.4.7 pytest==6.2.4 pytz==2021.1 requests==2.25.1 rsa==4.7.2 six==1.16.0 tensorboard==1.15.0 tensorflow-estimator==1.15.1 tensorflow==1.15.5 termcolor==1.1.0 toml==0.10.2 typing-extensions==3.10.0.0 urllib3==1.26.5 werkzeug==2.0.1 wheel==0.36.2 wrapt==1.12.1 zipp==3.4.1 --index-url https://pypi.org/simple/ --retries 5 --timeout 15 exited with 1 and STDERR:
None
(Use --print-stacktrace to see more error details.)

Relevant bits of pants.toml:

[python-setup]
interpreter_constraints = ['CPython==3.7.*']
requirement_constraints = 'constraints.txt'

Also note:

  1. We depend on gast only transitively via TensorFlow and this target does not depend on TensorFlow or gast so there's no need to resolve that dependency
  2. gast appears to be pure-python so there shouldn't be any issue finding a compatible version for OSX.
@stuhood
Copy link
Sponsor Member

stuhood commented Jun 17, 2021

Because this repo sets [python-setup] requirement_constraints, I think that this is related to the changes that lead to #11985. We're now always executing a single resolve (i.e., the nondeployables setting was deprecated), and in this context, that means that all of the requirements in the constraints file must be resolvable on all of the platforms used in the repo. cc @jsirois , @Eric-Arellano

A hitch in this hypothesis though is that gast should be just fine resolving for both platforms, so it's odd that that was the reported-missing package.

@jsirois
Copy link
Contributor

jsirois commented Jun 17, 2021

There is no hitch. There is no wheel: https://pypi.org/project/gast/0.2.2/#files

Foreign resolves (platforms are used here instead of ICs) must have all dists available as pre-built wheels. Pex has an option to try to turn platforms into local interpreters so it can build sdists when that's all that's available, but Pants doesn't use that option or plumb it.

@jsirois
Copy link
Contributor

jsirois commented Jun 17, 2021

So the bug here is a "lock file" really only works for a single interpreter (or a single platform - aka foreign interpreter). I had pointed out Pants own lockfile is buggy. If you regenerate Pants constraints.txt with python 3.7 and python 3.9 you get different results. That much is correct! Pants internals though assume 1 "lockfile" works for many interpreters, that can be incorrect. And this bug shows we further don't consider the platform case.

@jsirois
Copy link
Contributor

jsirois commented Jun 17, 2021

See #12200

@Eric-Arellano
Copy link
Contributor

Thanks @oliverdain!

In the meantime to redesigning our lockfile support in 2.6 or 2.7, I think the best fix is to change our pex_from_targets code to not use the global resolve (but still use constraints file) if platforms is specified. Thoughts?

@jsirois
Copy link
Contributor

jsirois commented Jun 17, 2021

That sounds right. The bug is still present for IC ranges as layed out in #12200 after that fix, but this gets back status quo.

@jsirois
Copy link
Contributor

jsirois commented Jun 17, 2021

Well, using constraints at all really isn't correct, since who knows what the constraints are for that Linux platform when I'm running on Mac, but I think that was status quo.

@Eric-Arellano
Copy link
Contributor

Cool. Ack that the status quo is still broken. I'll pair with @wilsonliam today on fixing this and cherry-pick to 2.5.

Eric-Arellano pushed a commit that referenced this issue Jul 2, 2021
…orms` (#12268)

Closes #12222 and adds an associated test for this edge case.

[ci skip-rust]
[ci skip-build-wheels]
@oliverdain
Copy link
Author

Thanks for the fix everyone!

and cherry-pick to 2.5

Does this mean there's a patch release I can pull and start using?

@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented Jul 2, 2021

@wilsonliam is cherry-picking into 2.5 and 2.6 right now, and then I'll try to do a release this afternoon before logging off for July 4 weekend.

Pardon the delay the past 2 weeks!

wilsonliam added a commit to wilsonliam/pants that referenced this issue Jul 2, 2021
…orms` (pantsbuild#12268)

Closes pantsbuild#12222 and adds an associated test for this edge case.

[ci skip-rust]
[ci skip-build-wheels]
wilsonliam added a commit to wilsonliam/pants that referenced this issue Jul 2, 2021
…orms` (pantsbuild#12268)

Closes pantsbuild#12222 and adds an associated test for this edge case.

[ci skip-rust]
[ci skip-build-wheels]
Eric-Arellano added a commit that referenced this issue Jul 14, 2021
This allows you to use the new lockfile format, generated by pip-tools via `./pants --tag=-lockfile_ignore lock ::` and #12300. 

A lockfile cannot be used at the same time as a constraints file. This makes the code easier to implement and means that we don't break any prior APIs. We will likely deprecate constraints when the dust settles.

There are several major deficiencies:

- Only `pex_from_targets.py` consumes this lockfile. This means that tool lockfiles will now have no constraints and no lockfile, for now.
- Does not handle requirements disjoint to the lockfile.
- Does not support multiple user lockfiles, which, for example, is necessary to properly handle `platforms` with `pex_binary` and `python_awslambda`: we need one lockfile per platform, as demonstrated in https://github.com/Eric-Arellano/lockfile-platforms-problem/tree/main/multiple_pex__stu_proposal.


### Lockfile vs. constraints file (and `[python-setup].resolve_all_constraints`)

We're currently using pip's `--constraints` file support, which allows you to specify constraints that may not actually be used. At the same time, we default to `[python-setup].resolve_all_constraints`, which _does_ first install the entire constraints file, and then uses Pex's repository PEX feature to extract the relevant subset. This is generally a performance optimization, but there are some times `--resolve-all-constraints` is not desirable:

1. It is not safe to first install the superset, and you can only install the proper subset. This especially can happen when `platforms` are used. See #12222.
     - We proactively disable `--resolve-all-constraints` when `platforms` are used.
2. User does not like the performance tradeoff, e.g. because they have a huge repository PEX so it's slow to access.

--

In contrast, this PR stops using `--constraints` and roughly always does `[python-setup].resolve_all_constraints` (we now run `pex -r requirements.txt --no-transitive` and use repository PEXes). Multiple user lockfiles will allow us to solve the above issues:

> 1. It is not safe to first install the superset, and you can only install the proper subset.

We'll have a distinct lockfile for each `platform`, which avoids this situation. See https://github.com/Eric-Arellano/lockfile-platforms-problem/tree/main/multiple_pex__stu_proposal for an example.

> 2. User does not like the performance tradeoff

They can use multiple lockfiles to work around this.

--

Always using `[python-setup].resolve_all_constraints` reduces complexity: less code to support, fewer concepts for users to learn.

Likewise, if we did still want to use `--constraints`, we would also need to upgrade Pex to use Pip 21+, which gained support for URL constraints. We [hacked around URL constraints before](#11907), but that isn't robust. However, Pip 21+ drops Python 2 and 3.5 support: we'd need to release Pex 3 w/o Py2 support, and upgrade Pants to have workarounds that allow Py2 to still be used. To avoid project creep, it's better to punt on Pex 3.

[ci skip-rust]
[ci skip-build-wheels]
stuhood added a commit that referenced this issue Apr 7, 2022
#14995 shows that #12222 re-broke for the `enable_resolves` case, since the fix for the issue was only checking the `resolve_all_constraints` field. Fixes #14995.
stuhood added a commit to stuhood/pants that referenced this issue Apr 7, 2022
…ild#15031)

pantsbuild#14995 shows that pantsbuild#12222 re-broke for the `enable_resolves` case, since the fix for the issue was only checking the `resolve_all_constraints` field. Fixes pantsbuild#14995.
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
stuhood added a commit that referenced this issue Apr 7, 2022
…ick of #15031) (#15034)

#14995 shows that #12222 re-broke for the `enable_resolves` case, since the fix for the issue was only checking the `resolve_all_constraints` field. Fixes #14995.

[ci skip-rust]
[ci skip-build-wheels]
stuhood added a commit that referenced this issue Apr 7, 2022
…ick of #15031) (#15033)

#14995 shows that #12222 re-broke for the `enable_resolves` case, since the fix for the issue was only checking the `resolve_all_constraints` field. Fixes #14995.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants