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

configure: fix broken bashism resulting in logic failure #567

Merged
merged 1 commit into from Mar 8, 2024

Conversation

eli-schwartz
Copy link
Contributor

After carefully using configure checks designed to work on pre-unix wars bourne shells -- that is, the test "$var" = "" construct once upon a time wasn't specified to treat "" as a distinct argument, so the "x" padding provided a guaranteed comparison -- the configure.ac check then fails to run on any shells at all other than 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. 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...)

Delete this accidental bashism, and restore the ability to get correct ./configure behavior on systems where /bin/sh is something other than a symlink to GNU bash.

@whitslack
Copy link

whitslack commented Dec 20, 2023

the test "$var" = "" construct once upon a time wasn't specified to treat "" as a distinct argument, so the "x"

That's not the reason for using "x". An empty argument has always been passed as a distinct argument (a zero-length string). The issue is that $var might substitute as something like -n. Since test has to function as though it's a standalone binary utility, it has no way to know how its argument values were constructed; all it sees is three arguments: -n = (empty string). The first two arguments form a valid expression, testing whether = is a non-zero-length string, but then there's an extra argument that test doesn't know what to do with. By prepending "x", we can ensure that the first argument never gets interpreted as a unary operator. test x-n = x evaluates as intended.

@eli-schwartz
Copy link
Contributor Author

Pretty sure it is one of the reasons.

In this case, the variables can only be set to one of:

  • ""
  • "gs"
  • the value passed to configure via --with-gs-path=, which can technically be anything but is not assumed to be --with-gs-path="-n"

But eh, I can change the commit message.

After carefully using configure checks designed to work on pre-unix wars
bourne shells -- that is, the `test "$var" = ""` construct once upon a
time wasn't specified to treat "" as a distinct argument, and various
buggy implementations mishandled various forms where the first argument
started with a dash, so the "x" padding provided a guaranteed comparison
-- the configure.ac check then fails to run on any shells at all other
than 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. 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...)

Delete this accidental bashism, and restore the ability to get correct
./configure behavior on systems where /bin/sh is something other than a
symlink to GNU bash.
@whitslack
Copy link

In test "x$var" = xyes, if $var is known never to be equal to a value that test could interpret as an operator, then you don't need the xs and could simply write test "$var" = yes, but often we can't be certain of what values $var could take, so we use the xs just to be safe.

I don't really care about the commit message. I only commented to dispel the misconception, as it's important for new developers to understand the reasons behind common idioms.

@eli-schwartz
Copy link
Contributor Author

Gentle ping. :)

@tillkamppeter tillkamppeter merged commit 196a892 into OpenPrinting:master Mar 8, 2024
@eli-schwartz eli-schwartz deleted the bashism branch March 8, 2024 12:14
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 this pull request may close these issues.

None yet

3 participants