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

[FR] Add some warnings if the package has entry-points which don't appear to resolve anywhere #3987

Open
1 task done
wimglenn opened this issue Jul 17, 2023 · 7 comments
Open
1 task done
Labels
enhancement Needs Triage Issues that need to be evaluated for severity and status.

Comments

@wimglenn
Copy link
Contributor

wimglenn commented Jul 17, 2023

What's the problem this feature will solve?

It's easy to create an entry-point which doesn't actually resolve in the installed package

Describe the solution you'd like

Suppose you have a package with an entry-point:

# pyproject.toml
[build-system]
requires = ["setuptools"]
build-backend = "setuptools.build_meta"

[project]
name = "mypackage"
version = "0.0.1"

[project.scripts]
hello = "mypackage:hello_world"

source:

# mypackage/__init__.py
def helloworld():
	print("hello, world!!")

Can you spot the bug?

The build/packaging/install all works normally but the script will be unusable:

...
Successfully installed mypackage-0.0.1
$ hello
Traceback (most recent call last):
  File ".venv/bin/hello", line 5, in <module>
    from mypackage import hello_world
ImportError: cannot import name 'hello_world' from 'mypackage' (.venv/lib/python3.11/site-packages/mypackage/__init__.py)

It would be nice if the build backend would log a warning saying that mypackage:hello_world doesn't appear to exist, to catch these kind of issues at packaging time.

Alternative Solutions

No response

Additional context

  • The wrapper script is auto-generated, so it's an area that is easy to miss in unit tests. You can have 100% test coverage and still publish a broken package this way.
  • Also easy to do when the "script" target is in a subdirectory and this wasn't included in packaging (i.e. not found by automatic discovery patterns, or forgot to add explicitly in the toml e.g. packages = ["mypackage", "mypackage.subpackage"]
  • Detecting whether the entry point target exists is very difficult (impossible?) in the most pathological cases (e.g. it could be dynamically generated by a module __getattr__). It doesn't have to catch all pathological cases to be useful, just logging a WARNING would be helpful most of the time. The code could reuse the existing AST parsing code which attempts to find a __version__ attribute in the source.

Code of Conduct

  • I agree to follow the PSF Code of Conduct
@wimglenn wimglenn added enhancement Needs Triage Issues that need to be evaluated for severity and status. labels Jul 17, 2023
@wimglenn wimglenn changed the title [FR] [FR] Add some warnings if the package has entry-points which don't appear to resolve anywhere Jul 19, 2023
@SpecLad
Copy link

SpecLad commented Aug 9, 2023

I'm not sure this would be a good idea, as a project's entry point does not have to refer to a module installed by the project itself. It would be a module from a dependency.

Moreover, you can't reliably check if an attribute is defined in a module without importing said module, which might not be possible due to missing dependencies. It might also involve side effects.

@wimglenn
Copy link
Contributor Author

wimglenn commented Aug 10, 2023

@SpecLad Can you give an example of a distribution which defines an entrypoint resolved within a dependency instead of within the project itself? I don't think I've ever seen that done.

Regarding checking for attributes, I addressed this in the third bullet point above. Setuptools already does this with its attr: directive, documented under special directives. It does not need to be 100% reliable to still be useful.

@abravalheri
Copy link
Contributor

@SpecLad Can you give an example of a distribution which defines an entrypoint resolved within a dependency instead of within the project itself? I don't think I've ever seen that done.

This is especially useful for pipx.run entry-points, when the name of the main package differs from the name of the executable it exposes via console_scripts. You can publish a secondary package with the name of the executable, so users have a unified experience running either executable-name-not-related-to-dist-name or pipx run executable-name-not-related-to-dist-name. I maintain such a tool and have been planning to do that for a while (haven't done yet by lack of time).

@wimglenn
Copy link
Contributor Author

@abravalheri Hmm, I have instead used pipx run --spec package-name script-name in that case of console script name not matching distribution package name (or distribution packages which provide multiple scripts). It seems impolite to commandeer the PyPI name for your script-name just for this use case, it's quite niche.

But, point taken - how about a way for user to opt-in to checking for resolvable entry-points, similar to the --config-settings editable_mode=strict config?

@abravalheri
Copy link
Contributor

It seems impolite to commandeer the PyPI name for your script-name just for this use case, it's quite niche.

I have thought about that in the past, but I think it is actually better to block the name to avoid attacks caused by name confusion. Anyway, a whole different discussion...

But, point taken - how about a way for user to opt-in to checking for resolvable entry-points, similar to the --config-settings editable_mode=strict config?

I have no problem in adding support for that (if someone is interested in contributing :P)1.

Something that I noticed however is that there has been an increase in request for setuptools trying to pre-emptively catch misconfuration or correct common user mistakes. While I think that is very useful and nice, there is a complicated balance in terms of maintenance efforts.

Maintaining and evolving code that has been created with excessively defensive code can be a pain in the neck 😅 (and the complexity/size of the code base tends to increase).

An alternative to that would be for projects whose primary goal is to run checks (like linters and pre-commit hooks) to implement some of those checks. E.g. check-wheel-contents could be a good candidate (if the maintainers are interested).

Maybe that is even an easier target, because you need to finish building the wheel to be able to validate these entry-points (e.g. binary extensions). So it is not like setuptools can halt the build while processing the config...

Footnotes

  1. There is a technical challenge with --config-settings right now that I tried to describe (and maybe did a bad job) in https://github.com/pypa/setuptools/issues/3896#issuecomment-1656714771. The TL;DR is that because the way PEP 517 works (each hook is called in a brand new subprocess) + the need for supporting setup.py imperative scripts we have a level of isolation between what setuptools.build_meta receives and what the remaining of the setuptools code has access to. That is the root of the issue why setuptools does not offer first class support for config_settings and why we don't re-use the dist-info directory that has already been generated and so on...

@wimglenn
Copy link
Contributor Author

wimglenn commented Aug 11, 2023

Yep, I see where you're coming from. It also sounds like a good feature for check-wheel-contents (@jwodder what do you think?)

My only comment on using a post-build check instead of a build check is the target audience would be much more limited that way. I've only heard about check-wheel-contents fairly recently, and I only starting using it a couple of months ago - it's a great tool, but (unlike setuptools) you only find it once you've already been burned 😆

@wimglenn
Copy link
Contributor Author

Added jwodder/check-wheel-contents#40 but not getting any response there..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
Development

No branches or pull requests

3 participants