Implement metadata validation (not used)

[pganssle]

Summary of changes

Change to the Distribution class to require that some metadata exists before actually doing anything.

This is just a draft. It still needs test, presumably linting and formatting, and I'm not even confident that this is conceptually the right place for this logic, but I thought I'd throw this approach out there as an option for discussion.

Closes #2799, #2329

Pull Request Checklist

• Changes have tests
• News fragment added in changelog.d/.
  (See documentation for details)

[webknjaz]

This is just a draft.

Mind converting it to a draft, then? So that it's visually distinctive in the PR list..

[webknjaz]

Nit: use a guard expression to dedent this.

Suggested change

	if missing:
	if len(missing) == 1:
	message = "%s attribute" % missing[0]
	else:
	message = "%s and %s attributes" % (", ".join(missing[:-1]),
	missing[-1])
	raise DistutilsSetupError(
	"Required package metadata is missing: please supply the %s." % message
	)
	if not missing:
	return
	if len(missing) == 1:
	message = "%s attribute" % missing[0]
	else:
	message = "%s and %s attributes" % (", ".join(missing[:-1]),
	missing[-1])
	raise DistutilsSetupError(
	"Required package metadata is missing: please supply the %s." % message
	)

[webknjaz]

I'm not even confident that this is conceptually the right place for this logic, but I thought I'd throw this approach out there as an option for discussion.

Validating the user input early sounds like a good idea. The implementation seems a bit overengineered, though.

[jaraco]

I have some ideas I'll add to the issue.

[jaraco]

I notice that without this change, the tests still pass. I suspect you originally intended to require both name and version in _validate_metadata.

This additional code (the need for a version='1.0' in every test, even when irrelevant to the test) feels a bit like noise.

[jaraco]

I spent some more time looking at this today with the expectation that I'd merge it without the version requirement, but as I looked into it more, I'm far from confident that overriding run_commands is the right hook point. I suspect it should happen late in Distribution.finalize_options or perhaps explicitly between construction and running the commands, something like:

diff --git a/setuptools/_distutils/core.py b/setuptools/_distutils/core.py
index d603d4a45..5ab4bbddf 100644
--- a/setuptools/_distutils/core.py
+++ b/setuptools/_distutils/core.py
@@ -142,6 +142,8 @@ def setup (**attrs):
     if _setup_stop_after == "commandline":
         return dist
 
+    getattr(dist, '_validate_metadata', lambda: None)()
+
     # And finally, run all the commands found on the command line.
     if ok:
         try:

But I'm not even sure that's early enough. In theory, additional commands could alter the metadata (e.g. setup.py name_from_folder egg_info install). Of course, such a usage is deprecated, but that doesn't mean it isn't a supported configuration now, and it would be invalidated be the proposed change.

I'd like to avoid adding more interactions between distutils and Setuptools classes (Distribution in this case).

So for now, I'd like to keep the _validate_metadata method, and defer the method of integration.