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

print-dev-env: stop inadvertently adding . to PATH #8033

Merged
merged 3 commits into from
Mar 13, 2023

Conversation

lbodor
Copy link
Contributor

@lbodor lbodor commented Mar 12, 2023

Motivation

Ensure that . <(nix print-dev-env) correctly handles the case when PATH is empty.

Context

Fixes #8032 by changing the output of

nix print-dev-env nixpkgs#hello | grep nix_saved_

from

nix_saved_PATH="$PATH"
nix_saved_XDG_DATA_DIRS="$XDG_DATA_DIRS"
PATH="$PATH:$nix_saved_PATH"
XDG_DATA_DIRS="$XDG_DATA_DIRS:$nix_saved_XDG_DATA_DIRS"

to

nix_saved_PATH="$PATH"
nix_saved_XDG_DATA_DIRS="$XDG_DATA_DIRS"
PATH="$PATH${nix_saved_PATH:+:$nix_saved_PATH}"
XDG_DATA_DIRS="$XDG_DATA_DIRS${nix_saved_XDG_DATA_DIRS:+:$nix_saved_XDG_DATA_DIRS}"

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@lbodor lbodor requested a review from edolstra as a code owner March 12, 2023 11:57
@lbodor lbodor changed the title print-dev-env: stop inadvertently adding . to PATH print-dev-env: stop inadvertently adding . to PATH Mar 12, 2023
@roberth
Copy link
Member

roberth commented Mar 12, 2023

This looks uncontroversial. Please add test cases to tests/nix-shell.sh.
Looks like you'll want to change the existing test case (and your own) to run in a subshell

-source <(...
+(
+  source <(...

@lbodor
Copy link
Contributor Author

lbodor commented Mar 13, 2023

I've added a test, thanks for the pointer. Please let me know what you think.

Comment on lines 98 to 102
scratch=$(mktemp -d -t tmp.XXXXXXXXXX)
function finish {
rm -rf "$scratch"
}
trap finish EXIT
Copy link
Member

Choose a reason for hiding this comment

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

No need for a separate scratch directory, just use $TEST_ROOT instead of $scratch.

@roberth roberth enabled auto-merge March 13, 2023 17:09
@roberth roberth added the backport 2.11-maintenance Automatically creates a PR against the branch label Mar 13, 2023
@roberth roberth merged commit a387f46 into NixOS:master Mar 13, 2023
@github-actions
Copy link

Backport failed for 2.11-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin 2.11-maintenance
git worktree add -d .worktree/backport-8033-to-2.11-maintenance origin/2.11-maintenance
cd .worktree/backport-8033-to-2.11-maintenance
git checkout -b backport-8033-to-2.11-maintenance
ancref=$(git merge-base 208c8551249361d2383b44123b4ad156218c351b a3a6909bc8e33cf1bf102a315f398a252607b5e0)
git cherry-pick -x $ancref..a3a6909bc8e33cf1bf102a315f398a252607b5e0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.11-maintenance Automatically creates a PR against the branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nix print-dev-env inadvertently adds . to PATH
3 participants