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

wimglenn commented 1 year ago

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

[X] I agree to follow the PSF Code of Conduct

SpecLad commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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...

[^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 commented 1 year ago

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 :laughing:

wimglenn commented 1 month ago

Added https://github.com/jwodder/check-wheel-contents/issues/40 but not getting any response there..

pypa / setuptools