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

How to turn backend warnings into errors? #482

Open
matthewfeickert opened this issue Jun 10, 2022 · 10 comments
Open

How to turn backend warnings into errors? #482

matthewfeickert opened this issue Jun 10, 2022 · 10 comments
Labels
enhancement New feature or request

Comments

@matthewfeickert
Copy link

In scikit-hep/pyhf#1881 I noticed and fixed a SetuptoolsDeprecationWarning that pyhf had in its setup. As pyhf runs build in its CI to ensure things are working I would like to have these warnings get turned into errors so that the dev team is alerted to them in advance (I just noticed this SetuptoolsDeprecationWarning by looking at the logs by chance).

My naive attempt at

$ python -Werror -m build .

doesn't work, but I guess I'm unclear on how one should pass in warning filters to build and the commands that it will run as

$ python -m build --config-setting=-Werror .

also doesn't work (though that makes sense as --config-setting is going to the backend (setuptools) and not to Python).

@layday
Copy link
Member

layday commented Jun 10, 2022

@matthewfeickert
Copy link
Author

As @agoose77 points out, build runs in a new process by default, and Python isn't passing along the warning flag, so by setting the shell environmental variable PYTHONWARNINGS the information can be propagated. So

$ PYTHONWARNINGS=error python -m build .

works as desired. 👍

@matthewfeickert
Copy link
Author

So

$ PYTHONWARNINGS=error python -m build .

works as desired. +1

That was spoken too soon. I guess I really just want to turn SetuptoolsDeprecationWarning into errors and not catch DeprecationWarnings of other things.

@agoose77
Copy link

OK, I looked into this and it looks like the commandline warning configuration is pretty lacklustre right now in Python. You can pass a module.attr string to resolve the classname for PYTHONWARNINGS / -w ... but this happens before the site packages are enabled, so it won't work. You can work around this with PYTHONPATH pointing to the location of setuptools, but that is not easy unless you run build without build isolation.

@layday layday added the enhancement New feature or request label Jun 13, 2022
@henryiii
Copy link
Contributor

Can't you use colons? The (little used) fourth colon separated part is a module name, IIRC.

@matthewfeickert
Copy link
Author

matthewfeickert commented Jun 13, 2022

Can't you use colons?

If colons worked, then shouldn't this

$ PYTHONWARNINGS=error::SetuptoolsDeprecationWarning python -m build .

work?

This allows for

/tmp/build-env-mnsbs_hq/lib/python3.10/site-packages/setuptools/config/setupcfg.py:459: SetuptoolsDeprecationWarning: The license_file parameter is deprecated, use license_files instead.

to pass without error.

The (little used) fourth colon separated part is a module name, IIRC.

Yeah, the warnings docs say

Individual warnings filters are specified as a sequence of fields separated by colons:

action:message:category:module:line

but

$ PYTHONWARNINGS=error::SetuptoolsDeprecationWarning:setuptools python -m build .

gives

Invalid -W option ignored: unknown warning category: 'SetuptoolsDeprecationWarning'

@agoose77
Copy link

You could raise any warnings from setuptools instead of the specific deprecation warning. I think that's what @henryiii is implying?

I've tested that it works, though it will not discriminate between warning kinds.

@matthewfeickert
Copy link
Author

You could raise any warnings from setuptools instead of the specific deprecation warning.

I've tested that it works, though it will not discriminate between warning kinds.

Hm, I'm confused as to what I'm doing wrong then as

$ PYTHONWARNINGS=error:::setuptools python -m build .

fails to detect

/tmp/build-env-3r3f9oxf/lib/python3.10/site-packages/setuptools/config/setupcfg.py:459: SetuptoolsDeprecationWarning: The license_file parameter is deprecated, use license_files instead.
  warnings.warn(msg, warning_class)

but

$ PYTHONWARNINGS=error python -m build .

properly raises

ERROR Backend subprocess exited when trying to invoke get_requires_for_build_sdist

@layday
Copy link
Member

layday commented Jun 14, 2022

It expects the fully-qualified module name, which in this case would be setuptools.config.setupcfg. You can filter using regexes with the programmatic API but not with -W or the env var.

@matthewfeickert
Copy link
Author

You can filter using regexes with the programmatic API but not with -W or the env var.

Thanks @layday — this is the last bit that I was missing.

kratsg pushed a commit to scikit-hep/pyhf that referenced this issue Aug 12, 2022
* Add `PYTHONWARNINGS=error,default::DeprecationWarning` to the environment to cause
Python to treat warnings as errors (but use default for DeprecationWarning as probably
internal to build tools) during the package build to ensure that the build configuration
conforms to the latest standards. Use the 'PYTHONWARNINGS' environment variable as `build` runs in a new process and so something like `python -Werror -m build .` won't be able to
pass the flags into the new process, but the shell environment will be picked up.
   - c.f. https://docs.python.org/3/library/warnings.html#describing-warning-filters
   - c.f. pypa/build#482
* Remove `--outdir` option from `build` to use default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants