-
Notifications
You must be signed in to change notification settings - Fork 251
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.
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
Update wheel.py to support generic tag when no extensions found #598
base: main
Are you sure you want to change the base?
Conversation
If no extensions are built (due to conditions) then do not tag as platform specific wheel.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Surely there are lots of other ways for platform-specific files to make their way into a wheel eg as scripts, or by being built out-of-band and copied around, or whatever. This change will break the wheel name for anyone who is relying on such things. (Also as a matter of coding style: Also: no testcase. |
I'm open to suggestions on fixing this use case. See the issue I raised alongside with a mypyc example (but also applies to cython).
I disagree. It's quite common to have a tenary variable being true, false and "don't know".
I added a comment to this effect and, now, given your comment suggests this PR is DoA perhaps writing a test case would have been futile in any case. |
Unfortunately it is easier to pick holes than to propose solutions. Building with extensions etc is an under-loved part of the code and may need more invasive changes to address this sort of thing.
but this isn't a ternary variable: it never takes the value |
It doesn't mean it's not a ternary variable. At the outset the variable is indeterminate as no test has been done to test if it "has libs" or not. Once it has been determined there are no libs then it is set to False. I could have added if not libs:
# The result of building the extensions
# does not exist, this may due to conditional
# builds, so we assume that it's okay
self._has_libs = False
return
else:
self._has_libs = True but there would be no point as the variable will never be tested for being True. It's still a ternary variable. However, I feel we are arguing over semantics here. |
it's certainly easier to debate whether a variable that takes two values can properly be described as ternary, than it is to figure out how to achieve the goal of this MR!
Exactly! This is what shows that |
Oh wow!, look its been used in this repo several times already as well as thousands more times in other prominent python projects. |
I think we are talking past each other. Of course At least in the first of those examples that you linked - I didn't go through them all! - In this MR there are only two states that the code cares about: "I have built and there are no libs" or the opposite of that. So a two-state type is appropriate. People often do use Your code is the other way round: you might just as well have used |
I've created a workaround plugin poetry-plugin-ignore-build-script. poetry self add poetry-plugin-ignore-build-script
poetry build --ignore-build-script |
If no extensions are built (due to conditions) then do not tag as platform specific wheel.
Resolves: python-poetry/poetry#8039
If this PR looks reasonable I'm happy to add some documentation. I'd need pointers on where to add tests.