cibuildwheel icon indicating copy to clipboard operation
cibuildwheel copied to clipboard

[docs] Make shell interpreter explicit

Open JP-Ellis opened this issue 1 year ago • 5 comments

Summary

The docs should make it clear what shell is used when running commands such as before-build, before-all, before-test, etc.

Motivation

cibuildwheel allows for an arbitrary shell script to be executed in many places. The problem here is that it is unclear what shell is actually being used.

Naively, I would expect that:

  • Linux builds use /bin/sh or /bin/bash.
  • macOS builds use /bin/zsh as that is the default shell for macOS, though I would not be surprised if /bin/bash was used for consistency with Linux.
  • Windows build use either a Windows bash.exe for consistency, or the native cmd.exe or pwsh.exe.

I would rather avoid the trial-and-error, and the docs should make it really clear what shell is used.

Build log

No response

CI config

No response

JP-Ellis avatar Mar 07 '24 02:03 JP-Ellis

This is just subprocess.run with shell=True. Python respects the $SHELL environment variable on Posix. Generally, this is whatever you are running on macOS. On Linux, since it runs in the manylinux docker container, you'll have bash. Not sure if it's always cmd on Windows or if it respects what you run it from.

In general, it's probably best to keep this simple and cross-platform, and then manually run a bash/bat/whatever script with complex commands if you need them. Or a Python script. :)

henryiii avatar Mar 07 '24 03:03 henryiii

Yeah, I had a hunch it would just be calling subprocess.run. I guess for me, it just wasn't clear and other than trial and error, it was difficult to figure out.

I think there's a few things that could help here:

  1. Improving the docs to make what you explained really clear.
  2. Perhaps adding some clearer diagnostics in the CI logs that show which shell interpreter is being used.

Ultimately, it would be awesome to be able to specify the interpreter to allow something like:

before-build = {
  shell = "python",
  script = 'import shutil; shutil.rm("{package}/bin")'
}

Though I'm not sure if there's a desire for that, and it might complicate things significantly.

JP-Ellis avatar Mar 07 '24 04:03 JP-Ellis

You can do that today. Use

before-build = [
 """python -c 'import shutil; shutil.rm("{package}/bin")'""",
]

Same for bash, etc. Better yet, stick the contents in a file, stick #!/usr/bin/env python (or bash, etc) at the top, make it executable, then run the file, like

before-build = [
 "./tools/remove_bin.py",
]

These are all cross platform and don’t depend on the shell Python uses.

Also, your example was invalid TOML (though it will be valid in TOML 1.1 eventually). Inline tables must be one line in 1.0. :(

henryiii avatar Mar 07 '24 06:03 henryiii

(Not disagreeing with better docs, btw)

henryiii avatar Mar 07 '24 06:03 henryiii

Agreed on the docs. It's one of those things that was implied and never said.

joerick avatar Mar 07 '24 08:03 joerick