pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.33k stars 638 forks source link

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

Open huonw opened 7 months ago

huonw commented 7 months ago

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:

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"):

As a resolution:

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 commented 7 months ago

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?

lilatomic commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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.