build does not validate that the "source directory" argument is a valid source directory

[pfmoore]

If I run build in an empty directory (or any directory that isn't a Python project), rather than reporting the error, the tool appears to run setuptools and creates an empty project called UNKNOWN. If there's no pyproject.toml, I guess it's assuming setuptools as a fallback. But if there's no setup.py or setup.cfg this is clearly incorrect, and so should fail.

As it stands, the build runs and results in unwanted (and useless) build, dist and UNKNOWN.egg-info directories being created in my project. Luckily, I wasn't already using build or dist as actual directories for my code...

[FFY00]

I am tempted to say this is an issue with setuptools? Though it might not be that clear. IMO setuptools itself should fail if the setuptools.build_meta:__legacy__ hook is called and there aren't any of those files, or others that it may support in the future. What do you think?

[pfmoore]

Personally, I think that build should fail early if it detects it's being run from the wrong directory. In a purist sense, that means there's no pyproject.toml. But because you fall back to setuptools, IMO you need to check for setup.py or setup.cfg as well.

Yes, there's a setuptools issue here, as it shouldn't blindly package up something that's obviously wrong, but as a quality of implementation feature, build should check and fail early. After all, if it's left to setuptools, you would have to wait for the whole build environment to be created before you get told "nope, this isn't a Python project"...

[gaborbernat]

I'd accept a PR that fails early 👍

[layday]

But if there's no setup.py or setup.cfg this is clearly incorrect, and so should fail.

PEP 517 requires the existence of setup.py for legacy projects - it does not say anything about setup.cfg - and I don't think we should apply the fallback to setup.py-less projects because these projects would not be possible to build without PEP 517.

[FFY00]

My argument against that is that I feel like it is not very correct, and might not work all that well in the long run. Checking for setup.py is fine-ish, but checking for a setup.cfg, or any other configration file, is setuptools-defined behavior that I do not feel good depending on. From the top of my mind, what if setuptools deprecates it and starts ignoring it?
The issue never gets properly fixed in this project, I think it should be in setuptools, which would also fix this behavior in all other PEP 517 frontends.

PEP 517 also says, if there is no pyproject.toml, invoke setuptools.

If the pyproject.toml file is absent, or the build-backend key is missing, the source tree is not using this specification, and tools should revert to the legacy behaviour of running setup.py (either directly, or by implicitly invoking the setuptools.build_meta:__legacy__ backend).

I understand your point, and I agree it would be good to have better behavior for users, but I do not really like to do this at the expense of correctness. This probably comes from my experience as arch packager, where I've had to deal with all sorts of different build systems and issues, and have seen how these decisions play out, and when things go wrong it's usually in a way I am left scratching my head and be forced to dive into the build system internals. A lot of times, things go fine, but a lot of time they also don't. I think the best path forward is to put pressure on getting this fixed in setuptools, rather than introduce potentially inconsistent behavior here.

[FFY00]

PEP 517 requires the existence of setup.py for legacy projects - it does not say anything about setup.cfg - and I don't think we should apply the fallback to setup.py-less projects because these projects would not be possible to build without PEP 517.

I don't really think it requires it necessarily, my interpretation is that it just hints.

There is an existing, legacy source tree format involving setup.py.

Regardless, we would end up with inconsistent behavior.

[layday]

The PEP says "tools should revert to the legacy behaviour of running setup.py". I would take that to mean that the fallback should only be applied if setup.py exists. However, it then goes on to say, "... or by implicitly invoking the setuptools.build_meta:__legacy__ backend", so it is not clear whose responsibility it would be to verify that setup.py exists if the legacy backend is used.

That setuptools does not verify that setup.cfg exists before calling setup() in setup.py-less projects is a bug in setuptools; but if it is also desired that a build environment is not bootstrapped if pyproject.toml does not exist or build-backend is absent from pyproject.toml and setup.py does not exist, we'd have to account for that in build.

I think the logic surrounding the fallback is complex (see https://discuss.python.org/t/remove-wheel-as-minimum-requirement-from-pep-518/7513/18) and I appreciate not wanting to add another gate in the circuitry.

[pfmoore]

Can we take a step back here? The issue I'm concerned about here¹ is that build is "A simple, correct PEP517 package builder" and is supplied with a "source directory" by the user (which defaults to the current directory). If the user passes an invalid argument - in this case a directory which isn't a valid Python source directory, then the tool should report that. It's a fairly simple matter of input argument validation at that level. The problem is that detecting that the user has passed an invalid source directory argument is not as simple as we'd like it to be.

I did some additional tests, and apparently build doesn't even check that the "source directory" argument is a directory! It quite happily accepts non-existent directories, or filenames. I think those should be checked and reported to the user, rather than passing them to setuptools (which is the current behaviour).

Getting into less obvious cases, I believe that a directory without pyproject.toml or setup.py doesn't conform to either definition of a source tree in PEP 517, neither the PEP 517 form (which requires a pyproject.toml) nor the "legacy source tree format involving setup.py". So I think that such a directory is also an invalid argument to pass to the "source directory" argument of build.

I'm currently putting together a PR that checks for setup.py, but I'm happy to expand that to add proper validation of the srcdir argument to ProjectBuilder, in a way that covers all of the above cases. But whether or not you accept my PR, I think that build should validate the source directory passed to it. I'm going to update the title of this issue to reflect that.

¹ I'm happy to report the fact that setuptools will build junk in a directory with no setup.py or setup.cfg as a bug to setuptools, but IMO that's independent.

[henryiii]

I wrote the block below before testing pip. Pip does not support setup.cfg only projects, only build does. This should be still be clearly noted in the changelog, since it's a removal of existing functionality, but it looks like that functionality was likely incorrectly available before.

---

I'm in favor of the check (we do a simliar one in cibuildwheel), but the check needs to include setup.cfg (as discussed in #260, support for setup.cfg-only projects was added in setuptools v40.9). That is also a valid source directory by convention if not by standard; the previous releases of build supports it and pip supports it. The point of the check is to ensure that passing an incorrect directory that does not contain the required files fails nicely. Adding setup.cfg to the check will not cause any incorrect folders to start passing, but will allow the setuptools setup.cfg only projects that have no pyproject.toml's to keep working.

PS: I'm not totally sure why setup.cfg-only projects work without setting setuptools.build_meta as the build-backend, I'm getting the legacy backend happens to pick this up too? If this is an error by the standard, pip should be fixed first, then build.

[FFY00]

I do not agree with the check including setup.cfg as it is nowhere to be seen in PEP 517 and is merely a setuptools detail. Introducing such check would make us have to keep up with setuptools if they decide to introduce new ways projects are valid without a setup.py or pyproject.toml. IMO, the pip behavior is correct, PEP 517 only mentions setup.py or pyproject.toml.

[henryiii]

Yes, I was fine with this not being added when I realized that pip doesn't support it; currently only build supports a setup.cfg only project with no pyproject.toml. I think it's fine to match pip here. My only remaining point was that it might be mentioned in the changlog as a changed behavior, but it could be only people like me who throw together an example to check the behavior ( I think I did this when adding checks to cibuildwheel), but I'm pretty sure any real project would also require that a pip install without running build first works (such as in their test suite, etc). If that's the only case, then a mention is likely not needed.

[FFY00]

Yes, I am gonna add a breaking change section before a new release.

[hroncok]

@FFY00 In Fedora, we choose the setuptool's legacy backed if pyproject.toml is not found. Do you think we should assert setup.py is present before we delegate to the backed to match the behavior of build and pip?

I've opened pypa/setuptools#2329 about this ~1 year ago :/

[hroncok]

@FFY00 In Fedora, we choose the setuptool's legacy backed if pyproject.toml is not found. Do you think we should assert setup.py is present before we delegate to the backed to match the behavior of build and pip?

Actually, since we delegate to pip (or in future, build) to build the wheel, we should. I've noted it down in https://bugzilla.redhat.com/show_bug.cgi?id=1976459

[layday]

Your build system validation logic is also subtly different than what we have in build, see

build/src/build/__init__.py

Lines 204 to 218 in 5a3c524

	build_system = spec.get('build-system')
	# if pyproject.toml is missing (per PEP 517) or [build-system] is missing (per PEP 518),
	# use default values.
	if build_system is None:
	_find_typo(spec, 'build-system')
	build_system = _DEFAULT_BACKEND
	# if [build-system] is present, it must have a ``requires`` field (per PEP 518).
	elif 'requires' not in build_system:
	_find_typo(build_system, 'requires')
	raise BuildException("Missing 'build-system.requires' in {}".format(spec_file))
	# if ``build-backend`` is missing, inject the legacy setuptools backend
	# but leave ``requires`` alone to emulate pip.
	elif 'build-backend' not in build_system:
	_find_typo(build_system, 'build-backend')
	build_system['build-backend'] = _DEFAULT_BACKEND['build-backend']

.