Skip to content

gh-117787: Autoconf: fix bashisms/semantic breakage of iOS checks#117788

Merged
erlend-aasland merged 1 commit intopython:mainfrom
eli-schwartz:bashism
Apr 11, 2024
Merged

gh-117787: Autoconf: fix bashisms/semantic breakage of iOS checks#117788
erlend-aasland merged 1 commit intopython:mainfrom
eli-schwartz:bashism

Conversation

@eli-schwartz
Copy link
Copy Markdown
Contributor

@eli-schwartz eli-schwartz commented Apr 11, 2024

In commit bee7bb3, support was added for iOS. Although many string comparisons were added to configure.ac, one of them used some code only valid in GNU bash.

Bash provides the standard test XXX = YYY or [ XXX = YYY ] utilities. It also provides the ability to spell the equals sign as a double equals. This does nothing whatsoever -- it adds no new functionality to bash, it forbids nothing, it is literally an exact alias.

It should never be used under any circumstances. Honestly, bash itself should really drop support for it. Certainly, all developers must immediately forget that it exists. Using it is non-portable and does not work in /bin/sh scripts such as configure scripts, and it results in dangerous muscle memory when used in bash scripts because it makes people unthinkingly use the double equals even in /bin/sh scripts. To add insult to injury, it makes scripts take up more disk space (by a whole byte! and sometimes even a few bytes... ;) ;))

Anyway, in terms of practical difference, this results in:

POSIX sh systems such as:

  • macOS if /private/var/select/sh is tweaked
  • Linux if /bin/sh is dash and the unofficial cctools-port toolchain is used

always reporting that ac_sys_system is NOT iOS, and never adding the python framework to MODULE_DEPS_SHARED, even when the rest of the CPython build is configured for iOS.

It also reports a bad status message in the output of ./configure, which I caught via Gentoo's QA framework:

 * QA Notice: Abnormal configure code *
 * ./configure: 24692: test: Linux: unexpected operator

In commit bee7bb3, support was added
for iOS. Although many string comparisons were added to configure.ac,
one of them used some code only valid in GNU bash.

Bash provides the standard `test XXX = YYY` or `[ XXX = YYY ]`
utilities. It also provides the ability to spell the equals sign as a
double equals. This does nothing whatsoever -- it adds no new
functionality to bash, it forbids nothing, it is literally an exact
alias.

It should never be used under any circumstances. Honestly, bash itself
should really drop support for it. Certainly, all developers must
immediately forget that it exists. Using it is non-portable and does not
work in /bin/sh scripts such as configure scripts, and it results in
dangerous muscle memory when used in bash scripts because it makes
people unthinkingly use the double equals even in /bin/sh scripts. To
add insult to injury, it makes scripts take up more disk space (by a
whole byte! and sometimes even a few bytes... ;) ;))

Anyway, in terms of practical difference, this results in:

POSIX sh systems such as:
- macOS if /private/var/select/sh is tweaked
- Linux if /bin/sh is dash and the unofficial cctools-port toolchain is
  used

always reporting that ac_sys_system is NOT iOS, and
never adding the python framework to MODULE_DEPS_SHARED, even when the
rest of the CPython build is configured for iOS.

It also reports a bad status message in the output of ./configure, which
I caught via Gentoo's QA framework:

 * QA Notice: Abnormal configure code
 *
 * ./configure: 24692: test: Linux: unexpected operator
@erlend-aasland
Copy link
Copy Markdown
Contributor

Thanks! Consider using AS_VAR_IF :)

@eli-schwartz
Copy link
Copy Markdown
Contributor Author

I considered that but there are several hundred plain test uses so I wasn't sure if it made sense to convert them. :D

autoconf documentation seems to assume people will correctly use POSIX syntax, but suggests that AS_VAR_IF prevents "expansion error upon interrupt or term signal", which I suppose can happen in theory but in practice I don't see much to protect against in that scenario. The other thing it advertises is the ability to define a shell fragment that evaluates to the name of a variable, in case you need the indirection, which this case doesn't happen to need.

@mhsmith
Copy link
Copy Markdown
Member

mhsmith commented Apr 11, 2024

@freakboy3742: FYI

@erlend-aasland erlend-aasland changed the title gh-117787: configure: fix bashisms/semantic breakage of iOS checks gh-117787: Autoconf: fix bashisms/semantic breakage of iOS checks Apr 11, 2024
Copy link
Copy Markdown
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

No worries, this is also fine :) I just prefer the Autoconf macros. Thanks for the fix!

@erlend-aasland erlend-aasland merged commit fd2bab9 into python:main Apr 11, 2024
@freakboy3742
Copy link
Copy Markdown
Contributor

I apologise for the egregious waste of disk space... 😝

diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants