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

Python zip built with bootstrap_impl=script not runnable on macOS #2030

Closed
linzhp opened this issue Jul 2, 2024 · 3 comments · Fixed by #2052
Closed

Python zip built with bootstrap_impl=script not runnable on macOS #2030

linzhp opened this issue Jul 2, 2024 · 3 comments · Fixed by #2052

Comments

@linzhp
Copy link
Contributor

linzhp commented Jul 2, 2024

🐞 bug report

Affected Rule

The issue is caused by the rule: python/private/stage1_bootstrap_template.sh

Is this a regression?

Yes, the Python zip built with bootstrap_impl=system_python works on macOS

Description

The stage 1 bootstrapper for Python zip file cannot run on macOS

🔬 Minimal Reproduction

  1. Build a zip file with bazel build --build_python_zip --@rules_python//python/config_settings:bootstrap_impl=script
  2. Run the zip file on macOS

🔥 Exception or Error


mktemp: unrecognized option `--suffix'
usage: mktemp [-d] [-p tmpdir] [-q] [-t prefix] [-u] template ...
       mktemp [-d] [-p tmpdir] [-q] [-u] -t prefix

🌍 Your Environment

Operating System:

macOS Sonoma 14.5

Output of bazel version:

7.2.0

Rules_python version:

0.33.2

Anything else relevant?
This line is triggered when running a Python zip file. However, the mktemp command on macOS doesn't have the option --suffix.

@dws
Copy link

dws commented Jul 2, 2024

One approach might be to change the line

 zip_dir=$(mktemp -d --suffix Bazel.runfiles_)

to

zip_dir=$(mktemp -d --suffix Bazel.runfiles_ 2>/dev/null || mktemp -d)

That would preserve existing behavior where --suffix Bazel.runfiles_ is available, and just drop it when it's not available.

@linzhp
Copy link
Contributor Author

linzhp commented Jul 2, 2024

Or we can use prefix instead

@rickeylev
Copy link
Collaborator

I was poking this a bit yesterday. I can't find anything that indicates why the Bazel.runfiles_ substring is meaningful. There's some code in the stage2 bootstrap, that looks for a similar pattern, but the "Bazel." prefix and the "_" suffix would cause that logic to not match. So yeah, best guess, it was just added as a human-friendly informative marker.

I'll just change the code to simply mktemp -d.

github-merge-queue bot pushed a commit that referenced this issue Jul 16, 2024
Macs have an older version of `mktemp`, one that doesn't support the
`--suffix` arg. This
caused the combination of Macs and `--build_python_zip
--bootstrap_impl=script` to fail.

To fix, remove the `--suffix` arg. As far as I can tell, the suffix
string,
"Bazel.runfiles_", is just informational, so this is fine to remove.

Also adds tests to verify that a binary runs with/without zip and for
the script
bootstrap.

Fixes #2030
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants