build: Revise MANIFEST.in strategy to properly use prune #1449

matthewfeickert · 2021-05-12T05:21:31Z

Description

Based on notes from scikit-build/scikit-build#537, the .* in

global-exclude __pycache__ *.py[cod] .*

added in an attempt to exclude dotfiles (c.f. PR #791) is not actually needed (I think given the prune *) and appears to be causing trouble (?) in some other cases (c.f. scikit-build/scikit-build#537). In an effort to avoid problems, this PR removes the .* from the global exclude and adds the AUTHORS file (as I noticed it wasn't there and doesn't hurt).

Edit:

As it seems that prune * wasn't doing what we thought it was, additionally also use prune ** pattern and then manually include the files that the PyPA notes are automatically added to an sdist (but which we remove with prune ** which clobbers everything). Also ads a step to the CI that lists out the contents of the sdist tar.gz, so it should be a lot easier to spot check in the future.

Checklist Before Requesting Reviewer

Tests are passing
"WIP" removed from the title of the pull request
Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

Summarize commit messages into a comprehensive review of the PR

* Remove global exclude of dotfiles from MANIFEST.in to avoid potential problems with build systems
   - c.f. https://github.com/scikit-build/scikit-build/issues/537
* Use `prune **` to remove all files from the sdist
   - c.f. https://packaging.python.org/guides/using-manifest-in/#manifest-in-commands
   - "Setuptools also has undocumented support for ** matching zero or more characters including forward slash, backslash, and colon."
* Manually include all "default" files for a sdist in MANIFEST.in
   - c.f. https://packaging.python.org/guides/using-manifest-in/#how-files-are-included-in-an-sdist
* Include AUTHORS in MANIFEST.in
* Add list of sdist contents to package publishing CI
   - Allow for easier checking of the sdist contents in CI logs

This reverts commit 7d14afe.

This reverts commit f060240.

codecov · 2021-05-12T05:25:26Z

Codecov Report

Merging #1449 (750ca1d) into master (2c681a6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1449   +/-   ##
=======================================
  Coverage   97.65%   97.65%           
=======================================
  Files          64       64           
  Lines        4008     4008           
  Branches      565      565           
=======================================
  Hits         3914     3914           
  Misses         55       55           
  Partials       39       39

Flag	Coverage Δ
contrib	`25.52% <ø> (ø)`
unittests	`97.43% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c681a6...750ca1d. Read the comment docs.

matthewfeickert · 2021-05-12T05:37:13Z

@lukasheinrich @kratsg I'm putting this back into Draft mode given that if I look inside the tar.gz that gets built if we remove the .* then there are dotfiles there:

" tar.vim version v29
" Browsing tarfile /home/feickert/Code/GitHub/pyhf/dist/pyhf-0.6.2.dev46.tar.gz
" Select a file with cursor and press ENTER

pyhf-0.6.2.dev46/
pyhf-0.6.2.dev46/.ackrc
pyhf-0.6.2.dev46/.bumpversion.cfg
pyhf-0.6.2.dev46/.coveragerc
pyhf-0.6.2.dev46/.dockerignore
pyhf-0.6.2.dev46/.git_archival.txt
pyhf-0.6.2.dev46/.gitattributes
pyhf-0.6.2.dev46/.gitignore
pyhf-0.6.2.dev46/.hadolint.yaml
pyhf-0.6.2.dev46/.pre-commit-config.yaml
pyhf-0.6.2.dev46/.readthedocs.yml
pyhf-0.6.2.dev46/.zenodo.json
pyhf-0.6.2.dev46/AUTHORS
pyhf-0.6.2.dev46/CODE_OF_CONDUCT.md
pyhf-0.6.2.dev46/CONTRIBUTING.md
pyhf-0.6.2.dev46/LICENSE
pyhf-0.6.2.dev46/MANIFEST.in
pyhf-0.6.2.dev46/PKG-INFO
pyhf-0.6.2.dev46/README.rst
pyhf-0.6.2.dev46/codecov.yml
pyhf-0.6.2.dev46/codemeta.json
pyhf-0.6.2.dev46/pyproject.toml
pyhf-0.6.2.dev46/setup.cfg
pyhf-0.6.2.dev46/setup.py
pyhf-0.6.2.dev46/src/

and even when the .* is added back in there are files that we aren't explicitly adding that show up.

" tar.vim version v29
" Browsing tarfile /home/feickert/Code/GitHub/pyhf/dist/pyhf-0.6.2.dev46.tar.gz
" Select a file with cursor and press ENTER

pyhf-0.6.2.dev46/
pyhf-0.6.2.dev46/AUTHORS
pyhf-0.6.2.dev46/CODE_OF_CONDUCT.md
pyhf-0.6.2.dev46/CONTRIBUTING.md
pyhf-0.6.2.dev46/LICENSE
pyhf-0.6.2.dev46/MANIFEST.in
pyhf-0.6.2.dev46/PKG-INFO
pyhf-0.6.2.dev46/README.rst
pyhf-0.6.2.dev46/codecov.yml
pyhf-0.6.2.dev46/codemeta.json
pyhf-0.6.2.dev46/pyproject.toml
pyhf-0.6.2.dev46/setup.cfg
pyhf-0.6.2.dev46/setup.py
pyhf-0.6.2.dev46/src/

Sooooooo seems that I have more to learn about how MANIFEST.in actually works as prune * isn't working like I think it should.

https://packaging.python.org/guides/using-manifest-in/#how-files-are-included-in-an-sdist The following files are included in a source distribution by default: * all Python source files implied by the py_modules and packages setup() arguments * all C source files mentioned in the ext_modules or libraries setup() arguments * scripts specified by the scripts setup() argument * all files specified by the package_data and data_files setup() arguments * the file specified by the license_file option in setup.cfg (setuptools 40.8.0+) * all files specified by the license_files option in setup.cfg (setuptools 42.0.0+) * all files matching the pattern test/test*.py * setup.py (or whatever you called your setup script) * setup.cfg * README * README.txt * README.rst (Python 3.7+ or setuptools 0.6.27+) * README.md (setuptools 36.4.0+) * pyproject.toml (setuptools 43.0.0+) * MANIFEST.in

matthewfeickert · 2021-05-12T06:17:25Z

Okay. If I follow the advice of the PyPA's "Including files in source distributions with MANIFEST.in" guide it notes that

Setuptools also has undocumented support for ** matching zero or more characters including forward slash, backslash, and colon.

Which means that using prune ** does what we were hoping to get with prune *.

It also notes that

The following files are included in a source distribution by default:

all Python source files implied by the py_modules and packages setup() arguments

all C source files mentioned in the ext_modules or libraries setup() arguments

scripts specified by the scripts setup() argument

all files specified by the package_data and data_files setup() arguments

the file specified by the license_file option in setup.cfg (setuptools 40.8.0+)

all files specified by the license_files option in setup.cfg (setuptools 42.0.0+)

all files matching the pattern test/test*.py

setup.py (or whatever you called your setup script)

setup.cfg

README

README.txt

README.rst (Python 3.7+ or setuptools 0.6.27+)

README.md (setuptools 36.4.0+)

pyproject.toml (setuptools 43.0.0+)

MANIFEST.in

After adding the above files to the sdist, the commands in MANIFEST.in (if such a file exists) are executed in order to add and remove further files to & from the sdist. Default files can even be removed from the sdist with the appropriate MANIFEST.in command.

So in some situations one could maybe view our use of prune ** as an antipattern (cc @henryiii as hist takes a different approach to MANIFEST.in)?

If I add in

include setup.py
include setup.cfg
include LICENSE
include README.rst
include pyproject.toml
include MANIFEST.in
include AUTHORS

to match these default files (with the exception of the tests) then we get

" tar.vim version v29
" Browsing tarfile /home/feickert/Code/GitHub/pyhf/dist/pyhf-0.6.2.dev51.tar.gz
" Select a file with cursor and press ENTER

pyhf-0.6.2.dev51/
pyhf-0.6.2.dev51/AUTHORS
pyhf-0.6.2.dev51/LICENSE
pyhf-0.6.2.dev51/MANIFEST.in
pyhf-0.6.2.dev51/PKG-INFO
pyhf-0.6.2.dev51/README.rst
pyhf-0.6.2.dev51/pyproject.toml
pyhf-0.6.2.dev51/setup.cfg
pyhf-0.6.2.dev51/setup.py
pyhf-0.6.2.dev51/src/

For comparison, this is what we get if I we look at the v0.6.1 sdist tar.gz on PyPI:

" tar.vim version v29
" Browsing tarfile /home/feickert/Desktop/tmp/pyhf-0.6.1.tar.gz
" Select a file with cursor and press ENTER

pyhf-0.6.1/
pyhf-0.6.1/AUTHORS
pyhf-0.6.1/CODE_OF_CONDUCT.md
pyhf-0.6.1/CONTRIBUTING.md
pyhf-0.6.1/LICENSE
pyhf-0.6.1/MANIFEST.in
pyhf-0.6.1/PKG-INFO
pyhf-0.6.1/README.rst
pyhf-0.6.1/codecov.yml
pyhf-0.6.1/codemeta.json
pyhf-0.6.1/pyproject.toml
pyhf-0.6.1/setup.cfg
pyhf-0.6.1/setup.py
pyhf-0.6.1/src/

matthewfeickert added 4 commits May 12, 2021 00:11

Remove exclude dotfiles

7d14afe

Revert "Remove exclude dotfiles"

f060240

This reverts commit 7d14afe.

Revert "Revert "Remove exclude dotfiles""

e4721cb

This reverts commit f060240.

Add AUTHORS file to manifest

be7c338

matthewfeickert added the chore Other changes that don't modify src or test files label May 12, 2021

matthewfeickert self-assigned this May 12, 2021

matthewfeickert requested review from kratsg and lukasheinrich May 12, 2021 05:21

Add backin dotfile exclude

4b8d8a8

matthewfeickert marked this pull request as draft May 12, 2021 05:32

matthewfeickert added 5 commits May 12, 2021 00:48

Use prun ** to actually remove all

49874f0

Add setup.py and setup.cfg

647126c

remove lower bounds

5014fd4

Add space

64141b2

matthewfeickert added build Changes that affect the build system or external dependencies and removed chore Other changes that don't modify src or test files labels May 12, 2021

matthewfeickert changed the title ~~chore: Add AUTHORS and remove dotfiles exclude from MANIFEST.in~~ build: Revise MANIFEST.in strategy to properly use prune May 12, 2021

matthewfeickert marked this pull request as ready for review May 12, 2021 06:23

matthewfeickert mentioned this pull request May 12, 2021

Revision of MANIFEST.in section use of prune scikit-hep/scikit-hep.github.io#130

Closed

Add list of sdist for easier checking

750ca1d

matthewfeickert added the CI CI systems, GitHub Actions label May 13, 2021

kratsg approved these changes May 13, 2021

View reviewed changes

matthewfeickert merged commit 6b769fd into master May 13, 2021

matthewfeickert deleted the chore/remove-bit-from-manifest branch May 13, 2021 03:43

matthewfeickert mentioned this pull request May 13, 2021

fix: Revise prune strategy for MANIFEST.in to properly remove all scikit-hep/scikit-hep.github.io#131

Merged

matthewfeickert mentioned this pull request Jun 19, 2021

build: Revise MANIFEST.in strategy to properly use prune scikit-hep/pylhe#95

Merged

This was referenced Jul 31, 2021

build: Revise MANIFEST.in strategy matthewfeickert/heputils#73

Closed

build: Revise MANIFEST.in strategy to properly use prune matthewfeickert/heputils#75

Merged

matthewfeickert mentioned this pull request Aug 18, 2021

chore: Revise MANIFEST.in strategy to properly use prune dguest/pandamonium#61

Merged

matthewfeickert mentioned this pull request Sep 7, 2021

feat: Add MANIFEST.in scikit-hep/cabinetry#278

Merged

matthewfeickert mentioned this pull request Oct 1, 2021

build: Revise MANIFEST to use prune strategy recast-hep/recast-atlas#56

Merged

matthewfeickert mentioned this pull request Nov 9, 2021

build: Revise MANIFEST.in strategy to properly use prune eschanet/simplify#14

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Revise MANIFEST.in strategy to properly use prune #1449

build: Revise MANIFEST.in strategy to properly use prune #1449

matthewfeickert commented May 12, 2021 •

edited

Loading

codecov bot commented May 12, 2021 •

edited

Loading

matthewfeickert commented May 12, 2021

matthewfeickert commented May 12, 2021

build: Revise MANIFEST.in strategy to properly use prune #1449

build: Revise MANIFEST.in strategy to properly use prune #1449

Conversation

matthewfeickert commented May 12, 2021 • edited Loading

Description

Checklist Before Requesting Reviewer

Before Merging

codecov bot commented May 12, 2021 • edited Loading

Codecov Report

matthewfeickert commented May 12, 2021

matthewfeickert commented May 12, 2021

matthewfeickert commented May 12, 2021 •

edited

Loading

codecov bot commented May 12, 2021 •

edited

Loading