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

Jump to bottom

fix overwriting wheel on macos #1129

Merged
merged 3 commits into from Jun 17, 2022
Merged

fix overwriting wheel on macos #1129

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
526c554
fix overwriting wheel on macos
dlech Jun 9, 2022
e6513a9
use Path.unlink() instead of os.unlink()
dlech Jun 16, 2022
9f4303b
use FileNotFoundError instead of OSError
dlech Jun 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions cibuildwheel/macos.py
View file
Open in desktop
Expand Up @@ -535,6 +535,11 @@ def build(options: Options, tmp_path: Path) -> None:

# we're all done here; move it to output (overwrite existing)
if abi3_wheel is None:
try:
os.remove(build_options.output_dir / repaired_wheel.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this isn't using Path.unlink (perhaps with missing_ok=True)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This missing_ok parameter was added in Python 3.8. According to the setup.cfg in this repo python_requires = >=3.6 so it still needs to run on Python 3.6.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about just unlink and keeping the try, then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both functions do the same, so I don't really see any difference in one over the other.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are already using pathlib, so why mix that with os? Also this would transition more elegantly to missing_ok once 3.6 & 3.7 are dropped. If actions/setup-python#411 is merged in some form, we might be able to drop 3.6 for running.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I also changed OSError to FileNotFoundError to be more specific.

except OSError:
pass

shutil.move(str(repaired_wheel), build_options.output_dir)
built_wheels.append(build_options.output_dir / repaired_wheel.name)

Expand Down