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 repl doesn't work if there's distribution targets #16985

Open
oliverdain opened this issue Sep 23, 2022 · 7 comments
Open

pants repl doesn't work if there's distribution targets #16985

oliverdain opened this issue Sep 23, 2022 · 7 comments
Assignees
Labels
backend: Python Python backend-related issues bug

Comments

@oliverdain
Copy link

Describe the bug
I'm having issues with how pants repl interacts with distribution targets. I have a sample library that depends on pyyaml . That library has a target named lib which is just a python_sources goal and a target named example_lib which is a python_distribution target. If I do:

./pants repl src/example_lib/:lib

everything works fine. But suppose I want a repl with all the libs in my monorepo available. The obvious way to do that would be ./pants repl :: but that gives me:

$ ./pants repl ::
Traceback (most recent call last):
  File "/home/oliver/.pyenv/versions/3.10.0a7/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/oliver/.pyenv/versions/3.10.0a7/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/__main__.py", line 103, in <module>
    bootstrap_pex(__entry_point__, execute=__execute__, venv_dir=__venv_dir__)
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/.bootstrap/pex/pex_bootstrapper.py", line 601, in bootstrap_pex
    pex.PEX(entry_point).execute()
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/.bootstrap/pex/pex.py", line 519, in execute
    self.activate()
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/.bootstrap/pex/pex.py", line 156, in activate
    self._activated_dists = self._activate()
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/.bootstrap/pex/pex.py", line 143, in _activate
    activated_dists.extend(env.activate())
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/.bootstrap/pex/environment.py", line 321, in activate
    self._activated_dists = self._activate()
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/.bootstrap/pex/environment.py", line 671, in _activate
    resolved = self.resolve()
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/.bootstrap/pex/environment.py", line 502, in resolve
    for fingerprinted_distribution in self.resolve_dists(all_reqs)
  File "/home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/aa78b3981cc88964a18a80d8fa2d708ffe5155b2/.bootstrap/pex/environment.py", line 589, in resolve_dists
    raise ResolveError(
pex.environment.ResolveError: Failed to resolve requirements from PEX environment @ /home/oliver/.cache/pants/named_caches/pex_root/unzipped_pexes/452071acbd3c0c2b132caf245582cb4d49e6d821.
Needed cp310-cp310-manylinux_2_31_x86_64 compatible dependencies for:
 1: pyyaml
    Required by:
      example-lib 1.0.0
    But this pex had no ProjectName(raw='pyyaml', normalized='pyyaml') distributions.
 2: types-PyYAML
    Required by:
      example-lib 1.0.0
    But this pex had no ProjectName(raw='types-PyYAML', normalized='types-pyyaml') distributions.

Note that the actual built distribution has its dependencies specified correctly:

$ tar -O -zxvf dist/example_lib-1.0.0.tar.gz example_lib-1.0.0/setup.py
example_lib-1.0.0/setup.py

# DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS
# Target: src/example_lib:example_lib

from setuptools import setup

setup(**{
    'install_requires': (
        'pyyaml',
        'types-PyYAML',
    ),
    'name': 'example_lib',
    'namespace_packages': (
    ),
    'package_data': {
    },
    'packages': (
        'example_lib',
    ),
    'python_requires': '==3.10.*',
    'version': '1.0.0',
})

I have a minimal repo that reproduces the issue in a git bundle which I will attach to this bug.

Pants version
2.13.0

OS
Linux (Debian Bullseye)

Additional info

There was some discussion about this on Slack: https://pantsbuild.slack.com/archives/C046T6T9U/p1663632448606739

@oliverdain oliverdain added the bug label Sep 23, 2022
@oliverdain
Copy link
Author

It seems that Github only allows attachements types from a white-list (e.g. JPEG, PNG, etc.) and git bundles are not on that list. However, you can find the bundle with the repo in the Slack theread linked above or via this direct link: https://files.slack.com/files-pri/T046T6T8L-F043JRSPRH7/download/example.bundle?origin_team=T046T6T8L

@oliverdain
Copy link
Author

Per request, I have pushed the example to a public repo here: https://gitlab.com/companionlabs-opensource/pants-bug

@benjyw
Copy link
Sponsor Contributor

benjyw commented Sep 26, 2022

Thanks!

@benjyw
Copy link
Sponsor Contributor

benjyw commented Sep 26, 2022

@jsirois any ideas on this? I took a look and nothing came to me after a few minutes of poking at it.

@jsirois
Copy link
Contributor

jsirois commented Sep 26, 2022

This goes back to not yet landing wholistic proper treatment of python_distribution targets / aka the cython et. al, hack.

I find:

$ ./pants repl :: --keep-sandboxes=always --no-local-cache
...
10:26:51.05 [INFO] Preserving local process execution dir /tmp/pants-sandbox-YA9o6M for Building local_dists.pex with 1 requirement: example_lib-1.0.0-py3-none-any.whl
10:26:51.87 [INFO] Completed: Building local_dists.pex with 1 requirement: example_lib-1.0.0-py3-none-any.whl
Traceback (most recent call last):
  File "/home/jsirois/.pyenv/versions/3.10.7/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/jsirois/.pyenv/versions/3.10.7/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/__main__.py", line 103, in <module>
    bootstrap_pex(__entry_point__, execute=__execute__, venv_dir=__venv_dir__)
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/.bootstrap/pex/pex_bootstrapper.py", line 601, in bootstrap_pex
    pex.PEX(entry_point).execute()
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/.bootstrap/pex/pex.py", line 519, in execute
    self.activate()
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/.bootstrap/pex/pex.py", line 156, in activate
    self._activated_dists = self._activate()
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/.bootstrap/pex/pex.py", line 143, in _activate
    activated_dists.extend(env.activate())
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/.bootstrap/pex/environment.py", line 321, in activate
    self._activated_dists = self._activate()
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/.bootstrap/pex/environment.py", line 671, in _activate
    resolved = self.resolve()
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/.bootstrap/pex/environment.py", line 502, in resolve
    for fingerprinted_distribution in self.resolve_dists(all_reqs)
  File "/home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/922870db4f63142441afd2d34d6c28106b571065/.bootstrap/pex/environment.py", line 589, in resolve_dists
    raise ResolveError(
pex.environment.ResolveError: Failed to resolve requirements from PEX environment @ /home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/c7ac02162ddb22f8cf11c007b0023f58f03f4d33.
Needed cp310-cp310-manylinux_2_35_x86_64 compatible dependencies for:
 1: pyyaml
    Required by:
      example-lib 1.0.0
    But this pex had no ProjectName(raw='pyyaml', normalized='pyyaml') distributions.
 2: types-PyYAML
    Required by:
      example-lib 1.0.0
    But this pex had no ProjectName(raw='types-PyYAML', normalized='types-pyyaml') distributions.

Looking at the venv, it appears to be intransitive (requirements is correct - just 1 root requirement, but distributions is not):

$ jq . /home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/c7ac02162ddb22f8cf11c007b0023f58f03f4d33/PEX-INFO
{
  "bootstrap_hash": "2cc968a28896b1e02c9129c996a43073132f1b13",
  "build_properties": {
    "pex_version": "2.1.102"
  },
  "code_hash": "17086230d7879fdc75576a869caf15c63c4df059",
  "distributions": {
    "example_lib-1.0.0-py3-none-any.whl": "eb25f474e3fb2772cd2837aebf36b5be0cdd44232a0583665933a871ac113946"
  },
  "emit_warnings": false,
  "ignore_errors": false,
  "includes_tools": false,
  "inherit_path": "false",
  "interpreter_constraints": [],
  "pex_hash": "c7ac02162ddb22f8cf11c007b0023f58f03f4d33",
  "pex_path": null,
  "requirements": [
    "example-lib==1.0.0"
  ],
  "strip_pex_env": true,
  "venv": false,
  "venv_bin_path": "false",
  "venv_copies": false,
  "venv_site_packages_copies": false
}

The distributions should include these if transitive:

$ grep Requires /home/jsirois/.cache/pants/named_caches/pex_root/unzipped_pexes/c7ac02162ddb22f8cf11c007b0023f58f03f4d33/.deps/example_lib-
1.0.0-py3-none-any.whl/example_lib-1.0.0.dist-info/METADATA
Requires-Python: ==3.10.*
Requires-Dist: pyyaml
Requires-Dist: types-PyYAML

Of course the problem is we build these distributions intransitive here:

@rule(desc="Building local distributions")
async def build_local_dists(
request: LocalDistsPexRequest,
) -> LocalDistsPex:
transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses))
applicable_targets = [
tgt for tgt in transitive_targets.closure if PythonDistributionFieldSet.is_applicable(tgt)
]
local_dists_wheels = await MultiGet(
Get(LocalDistWheels, PythonDistributionFieldSet, PythonDistributionFieldSet.create(target))
for target in applicable_targets
)
# The primary use-case of the "local dists" feature is to support consuming native extensions
# as wheels without having to publish them first.
# It doesn't seem very useful to consume locally-built sdists, and it makes it hard to
# reason about possible sys.path collisions between the in-repo sources and whatever the
# sdist will place on the sys.path when it's installed.
# So for now we simply ignore sdists, with a warning if necessary.
provided_files: set[str] = set()
wheels: list[str] = []
wheels_digests = []
for local_dist_wheels in local_dists_wheels:
wheels.extend(local_dist_wheels.wheel_paths)
wheels_digests.append(local_dist_wheels.wheels_digest)
provided_files.update(local_dist_wheels.provided_files)
wheels_digest = await Get(Digest, MergeDigests(wheels_digests))
dists_pex = await Get(
Pex,
PexRequest(
output_filename="local_dists.pex",
requirements=PexRequirements(wheels),
interpreter_constraints=request.interpreter_constraints,
additional_inputs=wheels_digest,
internal_only=request.internal_only,
additional_args=["--intransitive"],
),
)

In the past, the python_distribution targets were built transitively. The problem there was that these transitive deps were not accounted for in resolves and caused issues there. In short, we've never actually resolved handling python_distribution targets comprehensively and instead chosen a bug to live with. Here that bug is exposed.

@jsirois jsirois removed their assignment Sep 26, 2022
@jsirois
Copy link
Contributor

jsirois commented Sep 26, 2022

I've unassigned since I think I've pin-pointed the issue. Solving it will take someone committing the time to dig in. We've avoided that investment up until now.

@jsirois
Copy link
Contributor

jsirois commented Sep 26, 2022

For more background the breadcrumbs lead here to the intro if --intransitive:

@benjyw benjyw self-assigned this Sep 26, 2022
@thejcannon thejcannon added the backend: Python Python backend-related issues label Sep 30, 2022
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