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

doc/hacking: note a cause of ./configure failing under zsh #8455

Closed
wants to merge 1 commit into from

Conversation

lf-
Copy link
Member

@lf- lf- commented Jun 6, 2023

In the default configuration of zsh, unquoted parameters do not have field expansion performed, so ./configure $configureFlags will give ./configure only one argument in argv. The solution to this is to set the shell option SH_WORD_SPLIT or, easier, specify it as ${=configureFlags} to force this behaviour for one expansion.

Omission of this can cause configure to fail at finding rapidcheck.

The practical reason why someone might be using this guide with zsh is for example direnv or executing zsh with nix develop.

In the default configuration of zsh, unquoted parameters do not have
field expansion performed, so `./configure $configureFlags` will give
./configure only one argument in argv. The solution to this is to set
the shell option `SH_WORD_SPLIT` or, easier, specify it as
`${=configureFlags}` to force this behaviour for one expansion.

Omission of this can cause configure to fail at finding rapidcheck.

The practical reason why someone might be using this guide with zsh is
for example direnv or executing zsh with `nix develop`.
@lf- lf- requested a review from fricklerhandwerk as a code owner June 6, 2023 02:51
@fricklerhandwerk
Copy link
Contributor

Why not change the command invocation itself instead? It seems it wouldn't break calls from bash. We also have an (unfortunately highly redundant) section using stable commands that also needs updating, and it would certainly be more sustainable if we kept the amount of duplicated text minimal.

@lf-
Copy link
Member Author

lf- commented Jun 7, 2023

Why not change the command invocation itself instead? It seems it wouldn't break calls from bash.

Are you sure? It doesn't seem to work for me:

~ » bash

[jade@snowflake:~]$ export nya="1 2 3 4"

[jade@snowflake:~]$ python -c 'import sys;print(sys.argv)' ${=nya}
bash: ${=nya}: bad substitution

We also have an (unfortunately highly redundant) section using stable commands that also needs updating, and it would certainly be more sustainable if we kept the amount of duplicated text minimal.

Maybe I can move it into a common section? But it might be harder to see.

Comment on lines +52 to +53
> of `$configureFlags` so the configure flags are expanded to multiple words as
> they are in `sh` compatible shells.
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk Jun 7, 2023

Choose a reason for hiding this comment

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

#8455 (comment) Then the wording is misleading here. It reads to me as "using this syntax is the sh compatible way of expanding space-delimited strings into lists".

The entire premise may be off here. nix develop does some magic to hook you into the builder executable that happens to be bash. Assuming the user essentially does

nix develop
[nix-shell]$ zsh

is too much for this manual to take care of. It would be in the user's responsibility to know how to pass around environment variables in their shell of choice. This type of hack is nixos.wiki material in my opinion, which is essentially a large dump of such hacks (with a few exceptions that should become upstream documentation). Please point out if I'm completely mistaken here. Otherwise I suggest closing this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Use of direnv does not seem that unreasonable of a use case and I learned about this particularity of word expansion the day I filed this PR after using zsh as my main shell for five years. It's not a terribly obvious thing. But I understand your view as well.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-06-08-documentation-team-meeting-notes-53/28938/1

@fricklerhandwerk
Copy link
Contributor

Related: #9680

I'm closing this for the reasons stated previously #8455 (comment). We should instead solve the underlying issue: #7501

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.

3 participants