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

Warn when dependency inference does not infer something in your resolve #15326

Closed
Eric-Arellano opened this issue May 4, 2022 · 3 comments · Fixed by #15334
Closed

Warn when dependency inference does not infer something in your resolve #15326

Eric-Arellano opened this issue May 4, 2022 · 3 comments · Fixed by #15334

Comments

@Eric-Arellano
Copy link
Contributor

Relates to #14864, but that is specifically for the dependencies field and does not tackle dependency inference.

Two competing approaches:

@stuhood
Copy link
Member

stuhood commented May 4, 2022

Two competing approaches:

I don't know if they're necessarily competing: it's possible that we should both 1) enable unownded_dependency_behavior at warn, 2) when we warn/error, first do a lookup in other resolves.

Eric-Arellano added a commit that referenced this issue May 5, 2022
The dogfooding is good, especially as we consider changing the default for Pants to be warn or error (#15326).
@Eric-Arellano
Copy link
Contributor Author

Eric-Arellano commented May 9, 2022

If all deps belong to other resolve

UnownedDependencyError: Pants cannot infer owners for the following imports in the file src/python/pants/util/strutil_test.py (from the target src/python/pants/util/strutil_test.py:tests):

  • fake (line: 6)

These imports are already known by Pants, only are not defined in the same "resolve" of foo used by the target src/python/pants/util/strutil_test.py:tests. Instead, they use the following resolves from these targets:

  • fake: 'bar' from path/to:tgt, 'baz' from path/to:tgt

Pants can only infer imports on targets that share the same resolve. You may need to set the resolve= field for some of those targets, and often use the parametrize mechanism so that the same import works with multiple resolves. See doc_url().

Otherwise, try the below ideas.

--

If you do not expect an import to be inferrable, add # pants: no-infer-dep to the import line.

Is the import from a third-party dependency (see https://www.pantsbuild.org/v2.12/docs/python-third-party-dependencies)? Some common issues:

  • Pants does not know about the requirement, i.e. there is not a python_requirement target for it. You can run ./pants filter --target-type=python_requirement :: to see all python_requirement targets. If you are using requirements.txt, Poetry, or Pipenv, make sure you have python_requirements, poetry_requirements, and pipenv_requirements target generators, respectively. Double check that the requirement is in the respective file like requirements.txt.
  • The third-party requirement exposes modules different than Pants's default, e.g. ansicolors exposing colors. Set the modules or module_mapping fields.
  • Ambiguity. See below.

Is the import from first-party code? Some common issues:

  • The file does not exist.
  • The file is missing an owning target like python_sources. Run ./pants list path/to/file.py to see if there are any owners. If not, run ./pants tailor to generate targets.
  • The source root is not correctly set up, which is how Pants knows for example that the file src/py/my_project/app.py exposes the module my_project.app. See https://www.pantsbuild.org/v2.12/docs/source-roots.
  • Ambiguity. See below.

Some common issues with both first and third-party imports:

  • Ambiguity. See above logs if there are warnings and instructions.

If some deps belong to other resolve

Only change is second paragraph

UnownedDependencyError: Pants cannot infer owners for the following imports in the file src/python/pants/util/strutil_test.py (from the target src/python/pants/util/strutil_test.py:tests):

  • fake (line: 6)
  • another (line: 7)

Some of these imports are already known by Pants, only are not defined in the same "resolve" of foo used by the target src/python/pants/util/strutil_test.py:tests. Instead, they use the following resolves from these targets:

  • fake: 'bar' from path/to:tgt, 'baz' from path/to:tgt

Pants can only infer imports on targets that share the same resolve. You may need to set the resolve= field for some of those targets, and often use the parametrize mechanism so that the same import works with multiple resolves. See doc_url().

Otherwise, try the below ideas.

--

If you do not expect an import to be inferrable, add # pants: no-infer-dep to the import line.

Is the import from a third-party dependency (see https://www.pantsbuild.org/v2.12/docs/python-third-party-dependencies)? Some common issues:

  • Pants does not know about the requirement, i.e. there is not a python_requirement target for it. You can run ./pants filter --target-type=python_requirement :: to see all python_requirement targets. If you are using requirements.txt, Poetry, or Pipenv, make sure you have python_requirements, poetry_requirements, and pipenv_requirements target generators, respectively. Double check that the requirement is in the respective file like requirements.txt.
  • The third-party requirement exposes modules different than Pants's default, e.g. ansicolors exposing colors. Set the modules or module_mapping fields.
  • Ambiguity. See below.

Is the import from first-party code? Some common issues:

  • The file does not exist.
  • The file is missing an owning target like python_sources. Run ./pants list path/to/file.py to see if there are any owners. If not, run ./pants tailor to generate targets.
  • The source root is not correctly set up, which is how Pants knows for example that the file src/py/my_project/app.py exposes the module my_project.app. See https://www.pantsbuild.org/v2.12/docs/source-roots.
  • Ambiguity. See below.

Some common issues with both first and third-party imports:

  • Ambiguity. See above logs if there are warnings and instructions.

@stuhood
Copy link
Member

stuhood commented May 9, 2022

IMO, we should absolutely lean on this being documented on the docsite, and stop after the high level explanation.

UnownedDependencyError: Pants cannot infer owners for the following imports in the file src/python/pants/util/strutil_test.py (from the target src/python/pants/util/strutil_test.py:tests):

fake (line: 6)

These imports are not in the current resolve for the target ($current-resolve), but they were present in other resolves:

fake: 'bar' from path/to:tgt, 'baz' from path/to:tgt

Pants will only infer imports on targets that share the same resolve. You may need to set the resolve= field for some of those targets, or use the parametrize mechanism so that the same import works with multiple resolves. See doc_url().

...and then most of the explanation to the "multiple resolves" section on https://www.pantsbuild.org/docs/python-third-party-dependencies#multiple-lockfiles

Eric-Arellano added a commit that referenced this issue May 10, 2022
…cy_behavior` (#15334)

This mostly points at https://www.pantsbuild.org/v2.11/docs/troubleshooting#import-errors-and-missing-dependencies because we decided it was too noisy of a warning/error message to reproduce the whole guide. We want the terminal to highlight diagnostics unique to the particular issue, and leave general guidance elsewhere.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

Closes #15326 by improving the error message when resolves are likely the culprit.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

These imports are not in the resolve used by the target (`another`), but they were present in other resolves:

  * pants.util.strutil.bullet_list: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_binary: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_text: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.first_paragraph: 'python-default' from src/python/pants/util/strutil.py
...
  * pytest: 'python-default' from 3rdparty/python#pytest

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

[ci skip-rust]
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this issue May 10, 2022
…cy_behavior` (Cherry-pick of pantsbuild#15334)

This mostly points at https://www.pantsbuild.org/v2.11/docs/troubleshooting#import-errors-and-missing-dependencies because we decided it was too noisy of a warning/error message to reproduce the whole guide. We want the terminal to highlight diagnostics unique to the particular issue, and leave general guidance elsewhere.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

Closes pantsbuild#15326 by improving the error message when resolves are likely the culprit.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

These imports are not in the resolve used by the target (`another`), but they were present in other resolves:

  * pants.util.strutil.bullet_list: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_binary: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_text: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.first_paragraph: 'python-default' from src/python/pants/util/strutil.py
...
  * pytest: 'python-default' from 3rdparty/python#pytest

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

[ci skip-rust]
# Conflicts:
#	src/python/pants/backend/python/dependency_inference/rules.py

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
Eric-Arellano added a commit that referenced this issue May 10, 2022
…cy_behavior` (Cherry-pick of #15334) (#15389)

* Improve `help` and error message for `[python-infer].unowned_dependency_behavior` (Cherry-pick of #15334)

This mostly points at https://www.pantsbuild.org/v2.11/docs/troubleshooting#import-errors-and-missing-dependencies because we decided it was too noisy of a warning/error message to reproduce the whole guide. We want the terminal to highlight diagnostics unique to the particular issue, and leave general guidance elsewhere.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

Closes #15326 by improving the error message when resolves are likely the culprit.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

These imports are not in the resolve used by the target (`another`), but they were present in other resolves:

  * pants.util.strutil.bullet_list: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_binary: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_text: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.first_paragraph: 'python-default' from src/python/pants/util/strutil.py
...
  * pytest: 'python-default' from 3rdparty/python#pytest

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

[ci skip-rust]
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this issue May 10, 2022
…cy_behavior` (pantsbuild#15334)

This mostly points at https://www.pantsbuild.org/v2.11/docs/troubleshooting#import-errors-and-missing-dependencies because we decided it was too noisy of a warning/error message to reproduce the whole guide. We want the terminal to highlight diagnostics unique to the particular issue, and leave general guidance elsewhere.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

Closes pantsbuild#15326 by improving the error message when resolves are likely the culprit.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

These imports are not in the resolve used by the target (`another`), but they were present in other resolves:

  * pants.util.strutil.bullet_list: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_binary: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_text: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.first_paragraph: 'python-default' from src/python/pants/util/strutil.py
...
  * pytest: 'python-default' from 3rdparty/python#pytest

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

[ci skip-rust]
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
Eric-Arellano added a commit that referenced this issue May 11, 2022
…cy_behavior` (Cherry-pick of #15334) (#15391)

This mostly points at https://www.pantsbuild.org/v2.11/docs/troubleshooting#import-errors-and-missing-dependencies because we decided it was too noisy of a warning/error message to reproduce the whole guide. We want the terminal to highlight diagnostics unique to the particular issue, and leave general guidance elsewhere.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

Closes #15326 by improving the error message when resolves are likely the culprit.

```
UnownedDependencyError: Pants cannot infer owners for the following imports from the target src/python/pants/util/strutil_test.py:tests:

  * pants.util.strutil.bullet_list (line: 9)
  * pants.util.strutil.ensure_binary (line: 10)
...
  * pants.util.strutil.strip_prefix (line: 18)
  * pants.util.strutil.strip_v2_chroot_path (line: 19)
  * pytest (line: 6)

These imports are not in the resolve used by the target (`another`), but they were present in other resolves:

  * pants.util.strutil.bullet_list: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_binary: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.ensure_text: 'python-default' from src/python/pants/util/strutil.py
  * pants.util.strutil.first_paragraph: 'python-default' from src/python/pants/util/strutil.py
...
  * pytest: 'python-default' from 3rdparty/python#pytest

If you do not expect an import to be inferrable, add `# pants: no-infer-dep` to the import line. Otherwise, see https://www.pantsbuild.org/v2.12/docs/troubleshooting#import-errors-and-missing-dependencies for common problems.
```

[ci skip-rust]
@stuhood
Copy link
Member

stuhood commented Jul 23, 2022

So, I just tried out adding another resolve to example-python, and it looks like we don't trigger these warnings by default in 2.12 and 2.13. I guess that that's because we never got around to changing the default of [python-infer].unowned_dependency_behavior to warn?

@Eric-Arellano , @thejcannon: It seemed like there was consensus around doing that: I can post a change next week, so that at least folks onboarding to 2.14 see it.

EDIT: #16281

stuhood added a commit that referenced this issue Jul 26, 2022
…ult. (#16281)

Rough consensus was reached on #15326 and #14975 that enabling `unowned_dependency_behavior="warning"` by default would:
1. be helpful for new users, to guide them through fixing their missing dependencies
2. be useful in the case of adding additional resolves (due to the work done in #15326 to enrich the warning for that case)
3. reduce the chances of CI errors in #14975 (when users go a step further to make the warning an error)

`2.14.x` is a relatively quiet release so far, so it seems like there is room in the budget for a new warning like this.

The rendered message for this warning is very self-explanatory due to @thejcannon's original work, and @Eric-Arellano's followup work in #15326, so I don't see any obvious documentation changes that need to be made.

[ci skip-build-wheels]
[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
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants