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

Issue warnings for missing py_modules? #136

Open
solsword opened this issue Apr 14, 2022 · 3 comments
Open

Issue warnings for missing py_modules? #136

solsword opened this issue Apr 14, 2022 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@solsword
Copy link

Hello,

After more hours than I'd care to admit trying to figure out why setuptools was giving me a "no module found" error when using attr: in a config file, I finally tracked the issue down to a silly mistake: I'd specified py_modules instead of packages in the config file, and my code was in a package. The mistake stemmed from a copy-paste error: I'd copied an old configuration file because I didn't remember all the details of the format, and renamed things for my new project without remembering that py_modules would have to change to packages since the new project used a package (see pypa/setuptools#3266).

The change I'd like to see it for the distutils code that checks modules to issue a warning when a listed module is not found, as opposed to silently ignoring this case, since that would have saved me a lot of time, and might also save time for others, on top of being quite reasonable behavior. The relevant code is lines 273-278 of command/build_py.py:

            # XXX perhaps we should also check for just .pyc files
            # (so greedy closed-source bastards can distribute Python
            # modules too)
            module_file = os.path.join(package_dir, module_base + ".py")
            if not self.check_module(module, module_file):
                continue

The equivalent code for finding packages raises an error on lines 189-195:

            if not os.path.exists(package_dir):
                raise DistutilsFileError(
                      "package directory '%s' does not exist" % package_dir)
            if not os.path.isdir(package_dir):
                raise DistutilsFileError(
                       "supposed package directory '%s' exists, "
                       "but is not a directory" % package_dir)

Since the comment in the modules code suggests the existence of .pyc files might be a reason for not being strict about modules existing, I think issuing a warning rather than an error is probably best to minimize what breaks. If this is something that maintainers here think is reasonable, I could put together a PR for raising a DistutilsFileWarning before continuing in the missing module case, which would include adding a DistutilsFileWarning class to errors.py.

@jaraco
Copy link
Member

jaraco commented Apr 29, 2022

Why not raise an error, congruent to what happens for a missing package?

@solsword
Copy link
Author

I'd be fine with raising an error as well, but was assuming that at least for one release issuing a warning instead would make sense just in case some people are depending on this behavior do to funky things with .pyc files somehow?

I'm not invested in either strategy, so whatever maintainers here think is best I'm happy to submit a PR for.

@jaraco
Copy link
Member

jaraco commented May 9, 2022

Let's go with the error approach. We can soften it to a warning if it proves necessary, but a warning will most likely go unnoticed and a multi-phase rollout for this change could take years.

@jaraco jaraco added enhancement New feature or request help wanted Extra attention is needed labels Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants