gh-117787: Autoconf: fix bashisms/semantic breakage of iOS checks #117788
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
|
Thanks! Consider using |
|
I considered that but there are several hundred plain 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. |
|
@freakboy3742: FYI |
erlend-aasland
changed the title
erlend-aasland
approved these changes
Apr 11, 2024
|
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 = YYYor[ 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:
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.