Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build_meta: produce informative error when a dist is not found #2608

Merged
merged 5 commits into from
Mar 22, 2021
Merged

build_meta: produce informative error when a dist is not found #2608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
024b1e1
build_meta: produce informative error when a dist is not found
layday Mar 16, 2021
8f2cc7a
fixup! build_meta: produce informative error when a dist is not found
layday Mar 16, 2021
f3ba213
fixup! fixup! build_meta: produce informative error when a dist is no…
layday Mar 21, 2021
2cec54e
fixup! build_meta: produce informative error when a dist is not found
layday Mar 21, 2021
e0655f6
Remove superfluous else.
jaraco Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.d/2608.change.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Added informative error message to PEP 517 build failures owing to
an empty ``setup.py`` -- by :user:`layday`
7 changes: 6 additions & 1 deletion setuptools/build_meta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,12 @@ def _file_with_extension(directory, extension):
f for f in os.listdir(directory)
if f.endswith(extension)
)
file, = matching
try:
file, = matching
except ValueError:
raise ValueError(
'No distribution was found. Ensure that `setup.py` '
'is not empty and that it calls `setup()`.')
Comment on lines +107 to +109
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be raise ValueError(...) from None, to simplify the output? The original ValueError from unpacking the tuple is not interesting to anyone and adding noise, since it's literally the only thing done here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then again, somehing else might raise a ValueError since the generator isn't consumed before it's unpacked and ValueError is so ubiquitous that I'm a little wary of erasing it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you'd like to convert the generator to a list comprehension and then raise from None, I think that'd be fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the generator is created on the line above, there currently should be no way to for it to be exhausted before it hits this point?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, hence it might raise an unrelated value error in the try block.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I see what you mean.

return file


Expand Down
11 changes: 11 additions & 0 deletions setuptools/tests/test_build_meta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import tarfile
import importlib
from concurrent import futures
import re

import pytest
from jaraco import path
Expand Down Expand Up @@ -442,6 +443,16 @@ def test_sys_argv_passthrough(self, tmpdir_cwd):
with pytest.raises(AssertionError):
build_backend.build_sdist("temp")

@pytest.mark.parametrize('build_hook', ('build_sdist', 'build_wheel'))
def test_build_with_empty_setuppy(self, build_backend, build_hook):
files = {'setup.py': ''}
path.build(files)

with pytest.raises(
ValueError,
match=re.escape('No distribution was found.')):
getattr(build_backend, build_hook)("temp")


class TestBuildMetaLegacyBackend(TestBuildMetaBackend):
backend_name = 'setuptools.build_meta:__legacy__'
Expand Down