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.

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

"make [force_]release" attempts upload even if build fails #1660

Closed
EliahKagan opened this issue Sep 13, 2023 · 0 comments · Fixed by #1661
Closed

"make [force_]release" attempts upload even if build fails #1660

EliahKagan opened this issue Sep 13, 2023 · 0 comments · Fixed by #1661

Comments

@EliahKagan
Copy link
Contributor

EliahKagan commented Sep 13, 2023

The force_release target in Makefile (which is also used by the release target after it attempts to check some things separate from this issue) had previously stopped after a failed attempt to build. But since #1654 it runs twine to attempt the upload whether or not the build succeeded.

The problem is that a command of the form a || b succeeds when a succeeds, but also when a fails but b succeeds:

python3 -m build --sdist --wheel || echo "Use a virtual-env with 'python -m venv env && source env/bin/activate' instead"

So that command fails only in the rare case that echo fails to write to standard output.

But the message about how using a virtual environment may remedy a build failure is valuable and should not be removed. Instead, the logic can be adjusted to make sure the command fails if it gets that far.

There are some other improvements I'd like to propose to Makefile, and I think it will be convenient to include a fix for this with them. But I wanted to open an issue for this, because I think this behavior is unexpected and could cause confusion while making a release if it is not anticipated (if the next release is made before it is fixed and something keeps the build command from succeeding).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants