pyroma icon indicating copy to clipboard operation
pyroma copied to clipboard

Pyroma outputs verbose build backend output when building metadata

Open wesleybl opened this issue 1 year ago • 8 comments

I'm getting the warning:

warning: no previously-included files matching '.isort' found anywhere in distribution

I already ignored the file in MANIFEST.in:

global-exclude .isort

However, the pyroma still keeps showing the warning. Is there any way to remove this warning?

wesleybl avatar Sep 06 '22 13:09 wesleybl

This warning is bleeding through from your build backend, rather than Pyroma itself, and would likely need to be fixed on that end. Since the build backend is run by a standard frontend in an isolated environment, there isn't any easy (or perhaps appropriate) way for Pyroma to intercept it. As such, this is really more an issue for your build backend, and if you're running it the standard fashion, you'll see it during normal builds of your distribution as well, if you look carefully at the full output.

That being said, I can try to explain what's going on here—the warning isn't in spite of global-exclude .isort, but rather because of it. Your build backend, presumably Setuptools since you're using a MANIFEST.in, is complaining that it can't find any files included by previous rules to exclude that match the pattern .isort (i.e. an isort config file), which could indicate that either you've mistakenly not included something in previous step you thought you did, or simply that the rule has no effect as no files matching it are in your source tree. If that's expected, you could either remove it, or simply ignore it—I'm not sure of a way to explicitly silence it for a specific rule, but there could well be one, so let me know as I run into this myself on occasion.

CAM-Gerlach avatar Sep 15 '22 18:09 CAM-Gerlach

@CAM-Gerlach well, the files in MANIFEST.in exist. For example:

$ cd /tmp
$ git clone https://github.com/plone/Products.CMFPlone.git
$ cd Products.CMFPlone
$ python3 -m venv .
$ ./bin/pip install pyroma
$ ./bin/pyroma .
------------------------------
Checking .
Getting metadata for wheel...
running dist_info
creating /tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info
writing /tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info/PKG-INFO
writing dependency_links to /tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info/dependency_links.txt
writing namespace_packages to /tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info/namespace_packages.txt
writing requirements to /tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info/requires.txt
writing top-level names to /tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info/top_level.txt
writing manifest file '/tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info/SOURCES.txt'
reading manifest file '/tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no previously-included files matching '*.pyc' found anywhere in distribution
warning: no previously-included files matching '*' found under directory 'news'
warning: no previously-included files found matching 'news'
adding license file 'LICENSE'
writing manifest file '/tmp/tmp1xcrgtd8/Products.CMFPlone.egg-info/SOURCES.txt'
creating '/tmp/tmp1xcrgtd8/Products.CMFPlone.dist-info'
adding license file "LICENSE" (matched pattern "LICEN[CS]E*")
Found Products.CMFPlone
------------------------------
Final rating: 10/10
Your cheese is so fresh most people think it's a cream: Mascarpone
------------------------------

See what we have:

warning: no previously-included files found matching 'news'

news is on MANIFEST.in

Maybe the way pyroma calls setuptools is not considering MANIFEST.in? If I remove news from MANIFEST.in it has the same message.

wesleybl avatar Sep 15 '22 22:09 wesleybl

Thanks for the additional information and the link to an example package on which you're seeing this. This is helpful in confirming my previous explanation, and helping me describe what's going on in your case with further specificity.

As mentioned, this is a valid warning produced by your build backend (Setuptools, in this case) due to your package configuration, and occurs entirely independent of pyroma. Your MANIFEST.in is indeed being picked up, as these warnings are each directly about a specific respective line in that file, and would not occur otherwise. The build frontend tool (build, pip wheel, etc) you are using to build your package artifacts for distribution may or may not be showing you the output of your build backend (Setuptools), or it may be buried in the copious output Setuptools produces by default, but that output does indeed include these warnings. Indeed, I reproduced this myself on a fresh clone of your repo running the standard, officially-recommended build frontend:

Build output
$ git clone https://github.com/plone/Products.CMFPlone.git
$ cd Products.CMFPlone
$ python -m build
* Creating venv isolated environment...
* Installing packages in isolated environment... (setuptools >= 40.8.0, wheel)
* Getting dependencies for sdist...
C:\Users\CAM~1.GER\AppData\Local\Temp\build-env-t050psxl\lib\site-packages\setuptools\dist.py:286: SetuptoolsDeprecationWarning: The namespace_packages parameter is deprecated, consider using implicit namespaces instead (PEP 420).
  warnings.warn(msg, SetuptoolsDeprecationWarning)
running egg_info
creating Products.CMFPlone.egg-info
writing Products.CMFPlone.egg-info\PKG-INFO
writing dependency_links to Products.CMFPlone.egg-info\dependency_links.txt
writing namespace_packages to Products.CMFPlone.egg-info\namespace_packages.txt
writing requirements to Products.CMFPlone.egg-info\requires.txt
writing top-level names to Products.CMFPlone.egg-info\top_level.txt
writing manifest file 'Products.CMFPlone.egg-info\SOURCES.txt'
reading manifest file 'Products.CMFPlone.egg-info\SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no previously-included files matching '*.pyc' found anywhere in distribution
warning: no previously-included files matching '*' found under directory 'news'
warning: no previously-included files found matching 'news'
adding license file 'LICENSE'
writing manifest file 'Products.CMFPlone.egg-info\SOURCES.txt'
* Building sdist...
# etc etc

As mentioned, this warning is due to an exclude not matching any file that was previously included. In this particular example, you have a directory news that is in your source tree, but it is not matched by any include rule (you have include *, but that only matches top-level files, not recursive directory contents). Then, exclude news and recursive-exclude news * both don't match anything, as news and its contents are never included to begin with, thus the (valid) warning.

To resolve it, you can simply remove the useless exclude news (and redundant recursive-exclude news *), since news is never included to begin with (as the warning is telling you).

You also have a warning for not matching any compiled *.pyc files, but that is useful to retain and can simply be ignored (unfortunately, I don't know of a straightforward way for you to silence it, but you'll only see it when viewing the full developer-facing build output; you could adopt a whitelist rather than blacklist approach for extensions in your code directory, but that runs the risk of not including a desired file—at that point, better to use Setuptools-SCM or a more modern build backend that just uses your .gitignore to include all VCS-tracked files automatically).

As a sidenote, include pyproject.toml is also redundant since you already have include *, which includes everything in the root directory anyway.

Hopefully this helps further clarity what is going on for you.

CAM-Gerlach avatar Sep 16 '22 00:09 CAM-Gerlach

@CAM-Gerlach thanks for your detailed explanation!

Really when I remove these two lines:

https://github.com/plone/Products.CMFPlone/blob/6.0.0b2/MANIFEST.in#L6-L7

news folder warning goes away.

I saw that the build.ProjectBuilder class has a runner parameter.

So I changed this line:

https://github.com/regebro/pyroma/blob/2c0c3fb6e5c216d40870060d3bdda431d009fff6/pyroma/projectdata.py#L27

to

import pep517
...

metadata_dir = build.ProjectBuilder(str(path),runner=pep517.quiet_subprocess_runner).prepare("wheel", tempdir)

See: https://pep517.readthedocs.io/en/latest/callhooks.html#pep517.quiet_subprocess_runner

So I was able to suppress the setuptools output:

 ./bin/pyroma .
------------------------------
Checking .
Getting metadata for wheel...
Found Products.CMFPlone
------------------------------
Final rating: 10/10
Your cheese is so fresh most people think it's a cream: Mascarpone

I think these outputs are more of a hindrance than a help, in the context of pyroma. (Or not. They can help people learn, like I learned here :smile: )

wesleybl avatar Sep 16 '22 02:09 wesleybl

Ah, perfect—I'd looked for something like that in the build docs, but didn't see it in the underlying pep517 library's. Great find! Would you like to submit a PR with that? I was also thinking that would be an improvement, though my one concern would be it could hide details useful for debugging build failures with user packages. Perhaps adding a -v verbose flag to Pyroma's CLI that uses the default chatty runner?

CAM-Gerlach avatar Sep 16 '22 19:09 CAM-Gerlach

Would you like to submit a PR with that?

Time is short. So I won't promise anything for now. But if I have time, I can give it a try.

wesleybl avatar Sep 20 '22 02:09 wesleybl

You've already done the great majority of the work toward the change, so I wanted to give you the chance to do the honors, but I'm happy to take care of it in a PR with appropriate attribution, if you're short on time.

CAM-Gerlach avatar Sep 23 '22 00:09 CAM-Gerlach

I've opened #87 to take care of this following your proposal, thanks

CAM-Gerlach avatar Sep 23 '22 00:09 CAM-Gerlach