Update wheel.py to support generic tag when no extensions found

[bsdz]

If no extensions are built (due to conditions) then do not tag as platform specific wheel.

Resolves: python-poetry/poetry#8039

• [ ] Added tests for changed code.
• [ ] Updated documentation for changed code.

If this PR looks reasonable I'm happy to add some documentation. I'd need pointers on where to add tests.

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[dimbleby]

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: bool | None is a weird type for a field that can take two values, it wants to be a boolean - perhaps with a better name! And I don't know that str(boolean).lower() is really an improvement on "true" if boolean else "false").

Also: no testcase.

[bsdz]

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.

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).

(Also as a matter of coding style: bool | None is a weird type for a field that can take two values, it wants to be a boolean - perhaps with a better name! And I don't know that str(boolean).lower() is really an improvement on "true" if boolean else "false").

I disagree. It's quite common to have a tenary variable being true, false and "don't know".

Also: no testcase.

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.

[dimbleby]

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.

... ternary variable ...

but this isn't a ternary variable: it never takes the value True.

[bsdz]

... ternary variable ...

but this isn't a ternary variable: it never takes the value True.

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.

[dimbleby]

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!

never be tested for being True.

Exactly! This is what shows that bool | None is the wrong type. Invalid states should be unrepresentable: you have two valid states and should reflect that with a type that has two valid values.

[bsdz]

Also as a matter of coding style: bool | None is a weird type for a field that can take two values..

Oh wow!, look its been used in this repo several times already as well as thousands more times in other prominent python projects.

[dimbleby]

I think we are talking past each other. Of course bool | None is not in itself a bad type - for cases where there are three distinct states that the code cares about.

At least in the first of those examples that you linked - I didn't go through them all! - None and True and False have different meanings.

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 bool | None, defaulting to None, and then treating None as false-y. They might just as well have used bool and defaulted to False.

Your code is the other way round: you might just as well have used bool and defaulted to True.

[bsdz]

I've created a workaround plugin poetry-plugin-ignore-build-script.

poetry self add poetry-plugin-ignore-build-script
poetry build --ignore-build-script