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

nix develop: Preserve stdin with -c #4233

Merged
merged 3 commits into from Nov 10, 2020
Merged

nix develop: Preserve stdin with -c #4233

merged 3 commits into from Nov 10, 2020

Conversation

Kha
Copy link
Contributor

@Kha Kha commented Nov 7, 2020

For use in editor integration, I would like to run a derivation taking input from stdin instead of $src, basically

echo "print('hi')" | nix develop .#foo -c python -

However, this currently breaks because of nix develop's use of --rcfile: the stdin is executed by the shell instead of being passed to the -c command. Not using --rcfile with -c, just like it's already being done with --phase, fixes this.

Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Nice :)

Left a couple of comments, but looks good otherwise

@@ -437,7 +437,7 @@ struct CmdDevelop : Common, MixEnvironment
std::vector<std::string> args;
for (auto s : command)
args.push_back(shellEscape(s));
script += fmt("exec %s\n", concatStringsSep(" ", args));
script += concatStringsSep(" ", args);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exec isn't needed anymore, so I thought I may as well simplify the code (since it's not being used for --phase either). Before, it was necessary for exiting the then-interactive shell.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong opinion on this, but it seems cleaner to keep it, if only to avoid having a useless bash process lying around while the command is running

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, why not. Reverted.

tests/nix-shell.sh Outdated Show resolved Hide resolved
Kha and others added 3 commits November 9, 2020 22:43
@edolstra edolstra merged commit 3f680c1 into NixOS:master Nov 10, 2020
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