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.

Sign up for GitHub

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

Jump to bottom

Ambiguous pex_binary entry_point fields get ambiguous dep inference warnings that cannot be disambiguated in 2.20 #20806

Open
huonw opened this issue Apr 17, 2024 · 4 comments · May be fixed by #20853
Open

Ambiguous pex_binary entry_point fields get ambiguous dep inference warnings that cannot be disambiguated in 2.20 #20806

huonw opened this issue Apr 17, 2024 · 4 comments · May be fixed by #20853
Labels
backend: Python Python backend-related issues bug
Milestone
2.20.x

Comments

@huonw
Copy link
Contributor

huonw commented Apr 17, 2024

Describe the bug

In 2.20, we have a new warning for if the entry_point refers to a known file path that is ambiguous, such as if a file is owned by a parametrised target. This warning appears to be:

  • shown for existing working PEXes
  • unsilenceable
08:53:11.79 [WARN] The pex_binary target //:ambiguous has the field `entry_point='./example.py'`, which maps to the Python module `example`, but Pants cannot safely infer a dependency because more than one target owns this module, so it is ambiguous which to use: ['//:src@tags=a', '//:src@tags=b'].

Please explicitly include the dependency you want in the `dependencies` field of //:ambiguous, or ignore the ones you do not want by prefixing with `!` or `!!` so that one or no targets are left.

Alternatively, you can remove the ambiguity by deleting/changing some of the targets so that only 1 target owns this module. Refer to https://www.pantsbuild.org/2.20/docs/using-pants/troubleshooting-common-issues#import-errors-and-missing-dependencies.
08:53:11.79 [WARN] Pants cannot resolve the entrypoint for the target //:ambiguous. The entrypoint EntryPoint(module='./example.py', function=None) might refer to the following:

  * //:src@tags=a
  * //:src@tags=b

The first warning is visible in 2.19 and earlier, while the second is new to 2.20.

Having diagnostics is good as silently failing to infer means the PEX doesn't contain the code people expect, but, unfortunately, it seems to be impossible silence the second one, by either disambiguating it properly, providing dependencies or just overruling the warning for the specific PEX (i.e. tell Pants "I know, it's fine"):

  • attempts to disambiguate by specifying the target name like entry_point="example.py:src@tags=a or entry_point="//:src@tags=a" (as implied by the error message) result in an invalid entry point
  • adding the required dependency explicitly still shows the second warning (but this does silence the first warning), and I don't think there's a way to squash unowned-dependency warnings for a single target, only globally?

As a resolution:

  • potentially these two warnings are redundant, and we should only have one, maybe the first one?
  • the second one should behave like the first and be silenced with a manual dependency specification

Reproducer that sets up three variations of this sort of pex_binary and runs them (not just package, so that we see if we got a working PEX):

cd $(mktemp -d)

cat > pants.toml <<EOF
[GLOBAL]
pants_version = "2.20.0"

backend_packages = [
  "pants.backend.python",
]
[python]
interpreter_constraints = ["CPython==3.10.*"]
EOF

cat > BUILD <<EOF
python_source(name="src", source="example.py", tags=parametrize(a=["a"], b=["b"]))

pex_binary(name="ambiguous", entry_point="./example.py")

pex_binary(name="with-target", entry_point="./example.py:src@tags=a")

pex_binary(name="with-deps", entry_point="./example.py", dependencies=[":src@tags=a", "!:src@tags=b"])
EOF

echo 'print("worked!")' > example.py

# force the builds to run fresh
export PANTS_LOCAL_STORE_DIR=$(mktemp -d)

set -x

# BUG:
# Two warnings:
# 1. `The pex_binary target //:ambiguous has the field ...`
# 2. `Pants cannot resolve the entrypoint ...`
# Fails to run: `ImportError: No module named example`
pants run :ambiguous

# BUG:
# Same two warnings.
# Fails to run: `ValueError: Invalid entry point specification: run = example:src@tags=a.`
pants run :with-target

# BUG:
# Just warning 2
# Runs successfully.
pants run :with-deps

# Comparison to 2.19:
# OKAY: just warning 1, fails to run
PANTS_VERSION=2.19.0 pants run :ambiguous
# OKAY: no warnings, runs successfully
PANTS_VERSION=2.19.0 pants run :with-deps

Output of pants run :ambiguous (note the two warnings):

09:00:38.64 [WARN] The pex_binary target //:ambiguous has the field `entry_point='./example.py'`, which maps to the Python module `example`, but Pants cannot safely infer a dependency because more than one target owns this module, so it is ambiguous which to use: ['//:src@tags=a', '//:src@tags=b'].

Please explicitly include the dependency you want in the `dependencies` field of //:ambiguous, or ignore the ones you do not want by prefixing with `!` or `!!` so that one or no targets are left.

Alternatively, you can remove the ambiguity by deleting/changing some of the targets so that only 1 target owns this module. Refer to https://www.pantsbuild.org/2.20/docs/using-pants/troubleshooting-common-issues#import-errors-and-missing-dependencies.
09:00:38.64 [WARN] Pants cannot resolve the entrypoint for the target //:ambiguous. The entrypoint EntryPoint(module='./example.py', function=None) might refer to the following:

  * //:src@tags=a
  * //:src@tags=b
09:00:40.84 [INFO] Starting: Building local_dists.pex
09:00:42.59 [INFO] Completed: Building local_dists.pex
09:00:42.59 [INFO] Starting: Building ambiguous.pex
09:00:44.25 [INFO] Completed: Building ambiguous.pex
Traceback (most recent call last):
...
ImportError: No module named example

Output of pants run :with-target:

09:05:06.67 [WARN] The pex_binary target //:with-target has the field `entry_point='./example.py:src@tags=a'`, which maps to the Python module `example`, but Pants cannot safely infer a dependency because more than one target owns this module, so it is ambiguous which to use: ['//:src@tags=a', '//:src@tags=b'].

Please explicitly include the dependency you want in the `dependencies` field of //:with-target, or ignore the ones you do not want by prefixing with `!` or `!!` so that one or no targets are left.

Alternatively, you can remove the ambiguity by deleting/changing some of the targets so that only 1 target owns this module. Refer to https://www.pantsbuild.org/2.20/docs/using-pants/troubleshooting-common-issues#import-errors-and-missing-dependencies.
09:05:06.67 [WARN] Pants cannot resolve the entrypoint for the target //:with-target. The entrypoint EntryPoint(module='./example.py', function='src@tags=a') might refer to the following:

  * //:src@tags=a
  * //:src@tags=b
09:05:06.67 [INFO] Starting: Building with-target.pex
09:05:07.95 [INFO] Completed: Building with-target.pex
Traceback (most recent call last):
...
  File "/Users/huon/.pex/unzipped_pexes/b5e9f44dc6235d1d08264c298fe901a5320f636f/.bootstrap/pex/dist_metadata.py", line 933, in parse
    raise ValueError("Invalid entry point specification: {spec}.".format(spec=spec))
ValueError: Invalid entry point specification: run = example:src@tags=a.

Output of pants run :with-dips:

09:05:08.40 [WARN] Pants cannot resolve the entrypoint for the target //:with-deps. The entrypoint EntryPoint(module='./example.py', function=None) might refer to the following:

  * //:src@tags=a
  * //:src@tags=b
09:05:08.41 [INFO] Starting: Building with-deps.pex
09:05:09.67 [INFO] Completed: Building with-deps.pex
worked!

Pants version
2.20

OS

macOS

Additional info

I think the new "Pants cannot resolve the entrypoint" warning was added in #20390. Presumably it was added because the existing warning wasn't appearing in some cases?
@huonw huonw added the bug label Apr 17, 2024
@huonw huonw added this to the 2.20.x milestone Apr 17, 2024
@huonw
Copy link
Contributor Author

huonw commented Apr 17, 2024

Heya @lilatomic I think you might've added this new warning (thanks for improving diagnostics!). Could you describe the background for #20390? Did you find circumstances where the existing warning didn't appear?

@huonw huonw changed the title Ambiguous pex_binary entry_point fields get ambiguous dep inference warnings that cannot be disambiguated Ambiguous pex_binary entry_point fields get ambiguous dep inference warnings that cannot be disambiguated in 2.20 Apr 17, 2024
@huonw huonw added the backend: Python Python backend-related issues label Apr 17, 2024
@lilatomic
Copy link
Contributor

The warning ought to resolve with an explicit dep. We make the call to explicitly_provided_deps.disambiguated.
I added the links to the slack conversations in the original MR, I didn't find a GH issue.

@lilatomic
Copy link
Contributor

OK, I think I found the problem:

ExplicitlyProvidedDependencies.disambiguated checks (in order):

  1. if there are no ambiguous addresses, we don't disambiguate
  2. if any of the includes are covered by explicit deps, we don't disambiguate. I think the logic here is that the ambiguity is now explicitly caused by the user, so we honour that.
  3. try to remove inferred deps with ignores

So pex_binary(name="with-ignore-only", entry_point="./example.py", dependencies=["!:src@tags=b"]) works.
I think it's a bug that adding the dependency that is already inferred changes the inference behaviour.
I think it's unexpected that adding an explicit dep does not resolve the ambiguity, although I'm not sure it's incorrect. I think we could change the logic so that we mark as disambiguated if the intersection of the ambiguous owners and the explicitly provided dependencies results in exactly 1 item.

I don't think adding desired dependency explicitly works to disambiguate python sources either, except that (like here) it also forces the dependency to be included (by being a hard link).

@lilatomic
Copy link
Contributor

Also I'd expect "./example.py:src@tags=a" to not work. The entrypoint is supposed to be a python entrypoint, (in PEX becoming --entry-point MODULE[:SYMBOL]. I think putting a Pants target here would result in surprises. We'd have to then convert it into a module path somehow, and there are edge cases with that.

@lilatomic lilatomic linked a pull request Apr 28, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backend: Python Python backend-related issues bug
Projects
None yet
Milestone
2.20.x
Development

Successfully merging a pull request may close this issue.

Allow explicit dependencies to resolve ambiguities lilatomic/pants
2 participants
@huonw @lilatomic