pypa / cibuildwheel

🎡 Build Python wheels for all the platforms with minimal configuration.
https://cibuildwheel.pypa.io
Other
1.86k stars 237 forks source link

Inconsistent treatment of curly braces in `test-command` #2036

Open sergiud opened 2 weeks ago

sergiud commented 2 weeks ago

Description

I have the following pyproject.toml setup

[tool.cibuildwheel]
test-requires = "pytest"
test-command = "pytest {project}/tests/python/test_{hogpp,repr}.py"

which works in manylinux builds with the following (relevant) output:

Successfully installed exceptiongroup-1.2.2 iniconfig-2.0.0 packaging-24.1 pluggy-1.5.0 pytest-8.3.3 tomli-2.0.2
    + mkdir -p /tmp/tmp.U5BgBhGzbO/test_cwd
    + sh -c 'pytest /project/tests/python/test_{hogpp,repr}.py'
============================= test session starts ==============================
platform linux -- Python 3.9.20, pytest-8.3.3, pluggy-1.5.0
rootdir: /project
configfile: pytest.ini
collected 1300 items

../../../project/tests/python/test_hogpp.py ..............xxxxxxxxxxxxxx [  2%]
xxx..................................................................... [  7%]
.....................xxxxxx............................................. [ 13%]
.xxxxxx................................................................. [ 18%]
........................xxxxxxxxxxxxxxxxxxxxxxxxxxxx.................... [ 24%]
..........xxx........................................................... [ 29%]
........................................................................ [ 35%]
........................................................................ [ 40%]
........................................................................ [ 46%]
........................................................................ [ 52%]
........................................................................ [ 57%]
........................................................................ [ 63%]
........................................................................ [ 68%]
........................................................................ [ 74%]
........................................................................ [ 79%]
........................................................................ [ 85%]
........................................................................ [ 90%]
........................................................................ [ 96%]
........................................                                 [ 99%]
../../../project/tests/python/test_repr.py ........                      [100%]

======================= 1240 passed, 60 xfailed in 2.76s =======================

but fails in a musllinux build (specifically cp39-musllinux_x86_64) as follows:

Successfully installed exceptiongroup-1.2.2 iniconfig-2.0.0 packaging-24.1 pluggy-1.5.0 pytest-8.3.3 tomli-2.0.2
    + mkdir -p /tmp/tmp.AbCKIN/test_cwd
    + sh -c 'pytest /project/tests/python/test_{hogpp,repr}.py'
ERROR: file or directory not found: /project/tests/python/test_{hogpp,repr}.py

============================= test session starts ==============================
platform linux -- Python 3.9.20, pytest-8.3.3, pluggy-1.5.0
rootdir: /tmp/tmp.AbCKIN/test_cwd
collected 0 items

============================ no tests ran in 0.00s =============================

                                                              ✕ 9.73s
Error: Command ['sh', '-c', 'pytest /project/tests/python/test_{hogpp,repr}.py'] failed with code 4.

I understand that the expansion of curly braces might be ambiguous here. However, it would be nice to have a consistent behavior across different shells. While the docs mention the expansion with respect to options and the use of bracex (albeit, in a slightly different context), it is not clear what the behavior should actually be.

Build log

No response

CI config

No response

Czaki commented 2 weeks ago

hm. At first look, it looks like sh in these two images handles the curly braces differently. But it may need more investigation to understand the problem.

sergiud commented 2 weeks ago

The behavior might be related to different default shells:

docker run -it quay.io/pypa/manylinux2014_x86_64:2024.09.16-1
[root@40b294b7b7ff /]# echo $0
/bin/bash
[root@40b294b7b7ff /]# echo $SHELL
/bin/bash
docker run -it quay.io/pypa/musllinux_1_2_x86_64:2024.09.16-1 
897e44acb83f:/# echo $0
/bin/bash
897e44acb83f:/# echo $SHELL
/bin/ash
henryiii commented 2 weeks ago

ash is BusyBox v1.36.1 (2024-06-12 06:28:12 UTC) multi-call binary. It's because we call sh rather than bash:

5c9baf48927b:/# sh -c "echo a{b,c}d"
a{b,c}d
5c9baf48927b:/# bash -c "echo a{b,c}d"
abd acd

($SHELL would do the same thing).

Maybe we should change this to bash?

joerick commented 2 weeks ago

I thought that if you called /bin/sh on Unix you were getting a POSIX-compliant shell, which can be implemented by a number of shells - for example, if that's symlinked to bash, bash runs in a POSIX-compliant mode. The same should be true for busybox too.

My slight preference would be to stick to simply POSIX compliance here, as it's fully specified and lets us not care about things like shell versions etc.

But clearly the above shows that either bash or busybox is deviating from that spec. I'd like to figure out the nature of that first I think.

joerick commented 2 weeks ago

It looks like the 'brace expansion' feature is actually a 'bashism'. So now I wonder why bash in POSIX mode still expands it.

sergiud commented 2 weeks ago

Possibly because brace expansion is part of the Bash syntax and thus cannot be disabled. The Bash manual also mentions that the POSIX mode changes only Bash defaults that deviate from the standard.

sergiud commented 2 weeks ago

In case standardizing the shell is not an option, a note in the documentation to warn users not rely on a specific shell behavior would be helpful. Users should then explicitly invoke the shell of their choice.

I found the following pytest invocation to work in my case:

[tool.cibuildwheel]
# ...
test-command = '/usr/bin/env bash -c "pytest {project}/tests/python/test_{hogpp,repr}.py"'

which could serve as a suggestion in the docs.