Skip to content

Most of MANIFEST.in is obsolete, get rid of it#2620

Closed
DimitriPapadopoulos wants to merge 2 commits into
codespell-project:masterfrom
DimitriPapadopoulos:MANIFEST.in
Closed

Most of MANIFEST.in is obsolete, get rid of it#2620
DimitriPapadopoulos wants to merge 2 commits into
codespell-project:masterfrom
DimitriPapadopoulos:MANIFEST.in

Conversation

@DimitriPapadopoulos

Copy link
Copy Markdown
Collaborator

Fom pypa /setuptools_scm:

Additionally setuptools_scm provides setuptools with a list of files that are managed by the SCM (i.e. it automatically adds all of the SCM-managed files to the sdist). Unwanted files must be excluded by discarding them via MANIFEST.in.

@larsoner

Copy link
Copy Markdown
Member

It seems like we should still check-manifest, no? For example once we merge #2595 then having bin/codespell in MANIFEST.in seems like an error that we should catch

@DimitriPapadopoulos

Copy link
Copy Markdown
Collaborator Author

As far as I can see, check-manifest is not setuptools_scm-aware, it insists files such as COPYING are added to MANIFEST.in:

$ check-manifest --no-build-isolation
lists of files in version control and sdist do not match!
missing from VCS:
  codespell_lib/_version.py
missing from sdist:
  COPYING
suggested MANIFEST.in rules:
  include COPYING
$ 

@larsoner

Copy link
Copy Markdown
Member

If there are things to ignore, we should be able to add them to the --ignore I think, e.g.,

https://github.com/mne-tools/mne-python/blob/8f01c9002a8ec6d89824e0cc1c6415be73ab1987/Makefile#L120

@DimitriPapadopoulos

DimitriPapadopoulos commented Nov 28, 2022

Copy link
Copy Markdown
Collaborator Author

I'm not sure what check-manifest is supposed to check now that all files in the VCS are implicitly and automatically included. What is left to check?

MNE-Python is a poor example, it does not use setuptools_scm.

@larsoner

Copy link
Copy Markdown
Member

I'm not sure what check-manifest is supposed to check now that all files in the VCS are implicitly and automatically included. What is left to check?

If this is the case, shouldn't this PR rm MANIFEST.in entirely rather than just remove some of its lines?

@DimitriPapadopoulos

Copy link
Copy Markdown
Collaborator Author

MANIFEST.in is still useful to exclude files, but not to include files:

Additionally setuptools_scm provides setuptools with a list of files that are managed by the SCM (i.e. it automatically adds all of the SCM-managed files to the sdist). Unwanted files must be excluded by discarding them via MANIFEST.in.

@DimitriPapadopoulos

DimitriPapadopoulos commented Nov 28, 2022

Copy link
Copy Markdown
Collaborator Author

By the way, the failing tests seem unrelated to this merge request. They appear to be caused by #2626, which added python -m build without adding build as dependency to pyproject.toml.

@larsoner

Copy link
Copy Markdown
Member

Okay, feel free to add build to the dev deps here

	https://github.com/pypa/setuptools_scm

	Additionally `setuptools_scm` provides setuptools with a list of
	files that are managed by the SCM (i.e. it automatically adds all
	of the SCM-managed files to the sdist). Unwanted files must be
	excluded by discarding them via `MANIFEST.in`.
Required after ba4e71d, which introduced `python -m build`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

packaging packaging related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants