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

NoCompatibleResolveException should suggest remedies #14864

Closed
stuhood opened this issue Mar 21, 2022 · 4 comments · Fixed by #15416
Closed

NoCompatibleResolveException should suggest remedies #14864

stuhood opened this issue Mar 21, 2022 · 4 comments · Fixed by #15416
Assignees
Milestone

Comments

@stuhood
Copy link
Sponsor Member

stuhood commented Mar 21, 2022

The remedy for a NoCompatibleResolveException (both for Python and for the JVM) will generally be to either:

  1. change the resolve of a target to the one that was expected
  2. create a copy of the target with a separate resolve, either:
    • In 2.10.x: manually, with a unique name.
    • In 2.11.x: via parametrize.

We should include some more information about how to resolve the issue in the error message, possibly by linking to a page on the subject.

@stuhood stuhood added this to the 2.10.x milestone Mar 21, 2022
@kaos
Copy link
Member

kaos commented Mar 22, 2022

possibly by linking to a page on the subject.

+1 on that, as it'll allow for expanding and clarifying the topic based on experiences/feedback without adding the delay of a new release & upgrade cycle.

@Eric-Arellano Eric-Arellano self-assigned this Apr 15, 2022
@Eric-Arellano
Copy link
Contributor

Related: this error message does not mention what the root target is.

pants.backend.python.util_rules.pex_from_targets.NoCompatibleResolveException: The resolve chosen for the root target was pants-plugin, but some of its dependencies are not compatible with that resolve:

default-toolchain:
  * 3rdparty/python#pytest-responses

pants-plugin:
  * 3rdparty/python:pants-plugin-reqs#requests

@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented May 4, 2022

Round 1: more clear message

NOTE: The final paragraph does not mention how you should try to find if a target already exists and then how to use that. Realistically, I don't know how to suggest users do that - asking them to use peek is too noisy. I think we probably need to skip right to Round 2 for this reason. Otherwise, users might create new targets when they did not actually need to.

--

Bad dependencies, single root target (eg test):

The target path/to:foo uses the `resolve` `pants-plugin`, but some of its 
dependencies are not compatible with that resolve:

  * 3rdparty/python#pytest-responses (default-toolchain)

All dependencies must work with the same `resolve`. To fix this, either change 
the `resolve=` field on those dependencies to `pants-plugins`, or change 
the `resolve=` of the target path/to:foo. If those dependencies should work 
with multiple resolves, use the `parametrize` mechanism with the `resolve=` 
field or manually create multiple targets for the same entity.

For more information, see <docs_url>.

Bad dependencies, multiple roots (eg repl, Flake8):

The inputted targets use the `resolve` `pants-plugin`, but some of their dependencies 
are not compatible with that resolve:

Input targets:

* path/to:foo
* path/to:foo2

Bad dependencies:

  * 3rdparty/python#pytest-responses (default-toolchain)

All dependencies must work with the same `resolve`. To fix this, either change the `resolve=` field 
on those dependencies to `pants-plugins`, or change the `resolve=` of the 
input targets. If those dependencies should work with multiple resolves, use 
the `parametrize` mechanism with the `resolve=` field or manually create 
multiple targets for the same entity.

For more information, see <docs_url>.

Bad input roots, which is only repl because every other goal already partitions the input roots. Note that repl.py enriches the message already.

The input targets did not have a resolve in common.

default-toolchain:

* foo

pants-plugins:

* bar

Targets used together must use the same resolve, set by the `resolve` field. For more information on "resolves" (lockfiles), see <docs_url>.

To work around this, choose which resolve you want to use from above. Then run <....>. These queries will result in opening a REPL with only targets using the desired resolve.

Round 2: "did you mean?"

Will use dependency inference's module mapping to see if there already is a target that uses the same resolve.

--

The target path/to:foo uses the `resolve` `pants-plugin`, but some of its 
dependencies are not compatible with that resolve:

| Dependency                       | Resolve           | Did you mean?                                  |
|----------------------------------|-------------------|------------------------------------------------|
| 3rdparty/python#pytest-responses | default-toolchain | 3rdparty/python:pants-plugins#pytest-responses |
| 3rdparty/python#another          | default-toolchain |                                                |

All dependencies must work with the same `resolve`. 

To fix this, usually, you only need to update the `dependencies` field of foo:bar to use the entry from the "Did you mean?" column.

If "Did you mean?" is empty for a certain dependency, you likely need to create 
a target that sets `resolve={resolve}` by either using the `parametrize` 
mechanism or manually creating a new target. Then update the 
`dependencies` field of foo:bar to use your new target.

Alternatively, you can change the `resolve=` foo:bar.

For more information, see <docs_url>.

Bad dependencies, multiple roots (eg repl, Flake8):

The inputted targets use the `resolve` `pants-plugin`, but some of their dependencies 
are not compatible with that resolve:

| Dependency                       | Dependee(s)      | Resolve           | Did you mean?                                  |
|----------------------------------|------------------|-------------------|------------------------------------------------|
| 3rdparty/python#pytest-responses | foo:bar          | default-toolchain | 3rdparty/python:pants-plugins#pytest-responses |
| 3rdparty/python#another          | foo:bar, foo:baz | default-toolchain |                                                |

All dependencies must work with the same `resolve`. 

To fix this, usually, you only need to update the `dependencies` field of the problematic
"dependees" to use the entry from the "Did you mean?" column.

If "Did you mean?" is empty for a certain dependency, you likely need to create 
a target that sets `resolve={resolve}` by either using the `parametrize` 
mechanism or manually creating a new target. Then update the 
`dependencies` field of the problematic "dependees" to use your new target.

Alternatively, you can change the `resolve=` of some or all of the input targets (the `dependees` column).

For more information, see <docs_url>.

Bad input roots is the same, given that it only impacts repl and we already have the enhanced message. I think that "did you mean?" would be noisy.

@stuhood
Copy link
Sponsor Member Author

stuhood commented May 4, 2022

Thanks: the round one error messages look good, but I don't think that you need different text for the "single root" vs "multiple roots" case.

With regard to "Round Two", I don't think that we should expend much effort on offering suggestions for explicit deps, since explicit deps are rare... and in general, are for things which don't need resolves anyway (resources, primarily). Additionally, in cases where you do actually have multiple choices of resolve, it will usually be in the name anyway, likely via parametrization.

So: yes, Round One makes sense. But in terms of improving suggestions, rather than Round Two, would suggest either investing time in enabling --unowned-dependency-behavior=warn by default, and/or embedding a secondary lookup during dependency inference where we enrich the warning for a miss with a hint that the dependency exists in another resolve (EDIT: as described in #15326: thanks!).

@Eric-Arellano Eric-Arellano modified the milestones: 2.10.x, 2.11.x May 10, 2022
Eric-Arellano added a commit that referenced this issue May 13, 2022
…ves (#15416)

Closes #14864.

This code is written generically so that we can hook it up to JVM easily.

[ci skip-rust]
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this issue May 13, 2022
…ves (pantsbuild#15416)

Closes pantsbuild#14864.

This code is written generically so that we can hook it up to JVM easily.

[ci skip-rust]
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this issue May 13, 2022
…ves (pantsbuild#15416)

Closes pantsbuild#14864.

This code is written generically so that we can hook it up to JVM easily.

[ci skip-rust]
stuhood pushed a commit that referenced this issue May 13, 2022
…ves (Cherry-pick of #15416) (#15440)

Closes #14864.

This code is written generically so that we can hook it up to JVM easily.

[ci skip-rust]
Eric-Arellano added a commit that referenced this issue May 13, 2022
…ves (Cherry-pick of #15416) (#15439)

Closes #14864.

This code is written generically so that we can hook it up to JVM easily.

[ci skip-rust]
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.

3 participants