Skip to content

fix: keep PATH exports on their own line when assembling shell-env.sh - #3166

Open
Arshgill01 wants to merge 1 commit into
cachix:mainfrom
Arshgill01:cursor/fix-shell-env-path-spaces-a5d6
Open

Arshgill01 wants to merge 1 commit into
cachix:mainfrom
Arshgill01:cursor/fix-shell-env-path-spaces-a5d6

Conversation

@Arshgill01

Copy link
Copy Markdown
Contributor

Fixes #3165.

When Python venv (or any enterShell task) re-exports PATH and the parent PATH contains a directory with a space, devenv shell printed bash: export: … not a valid identifier and truncated PATH at the first space.

prepare_shell trims the Nix env script with trim_end(), which removes the newline after eval "${shellHook:-}". Task exports were then appended with no separator, so the first export PATH='…' landed on the same physical line. Outer bash consumed the quotes; eval word-split the PATH value at spaces.

Changes

  • Add push_shell_fragment, which inserts a newline before appending a non-empty fragment.
  • Use it wherever the env script is concatenated with task exports/messages (prepare_shell, PTY/reload shell-env.sh, direnv export).
  • Empty fragments remain a no-op, so projects without venv/task exports are unchanged.

Tests

  • Unit: assemble a trimmed env script plus a PATH export containing Some App, assert the export is on its own line, then bash the result and check PATH is not truncated.
  • Integration: tests/shell-path-spaces — PATH with a space + a venv-like enterShell PATH export; --shell zsh writes .devenv/shell-env.sh (the glued assembly) and devenv shell -- covers the non-interactive path.

No competing open PR for #3165.

Test plan

  • Standalone reproduction: glued eval "${shellHook:-}"export PATH='…Some App…' produces not a valid identifier and truncates PATH; with a newline separator, bash sources cleanly and PATH keeps the space-containing dir.
  • cargo nextest run -p devenv trimmed_shell_env push_shell_fragment
  • devenv-run-tests run tests --only shell-path-spaces
  • Manual: PATH="/tmp/Some App/bin:$PATH" devenv shell in a project with languages.python.venv.enable = true — no export errors, echo "$PATH" still contains Some App.

@github-actions

github-actions Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

🔍 Suggested Reviewers

Based on git blame analysis of the changed lines, the following contributors have significant experience with the modified code:

Please consider reviewing this PR as you have authored significant portions of the code being modified. Your expertise would be valuable! 🙏

This comment was automatically generated by git-blame-auto-reviewer

Last updated: 2026-09-30T04:38:50.497Z

@cursor
cursor Bot force-pushed the cursor/fix-shell-env-path-spaces-a5d6 branch from 85f7fc2 to a4c30be Compare September 8, 2026 04:06
@Arshgill01
Arshgill01 marked this pull request as ready for review September 8, 2026 04:07
@cursor
cursor Bot force-pushed the cursor/fix-shell-env-path-spaces-a5d6 branch 2 times, most recently from 05a29c5 to 1ed807d Compare September 14, 2026 04:47
@Arshgill01

Copy link
Copy Markdown
Contributor Author

Rebased onto latest cachix/devenv main (was ~13 commits behind). The push_shell_fragment / PATH-with-spaces fix is unchanged; the changelog entry now lives under 2.3.2 (unreleased) so it stays with the current unreleased notes rather than the released 2.3.1 section.

@cursor
cursor Bot force-pushed the cursor/fix-shell-env-path-spaces-a5d6 branch 2 times, most recently from 4a2ea2c to 05c3e95 Compare September 18, 2026 03:47
@Arshgill01

Copy link
Copy Markdown
Contributor Author

Still ready for review (bundled)

Gentle weekly nudge — still no maintainer feedback on the open Arshgill01 set. All MERGEABLE (rebasing onto latest main now if needed):

PR Summary
#3166 PATH spaces in shell-env.sh (Fixes #3165)
#3156 GC-root concurrent NotFound TOCTOU leftover from #3133
#3152 Reuse running process manager for process deps (Fixes #3137)
#2978 process-compose list / wait lifecycle commands

Happy to address review feedback on any of them.

@cursor
cursor Bot force-pushed the cursor/fix-shell-env-path-spaces-a5d6 branch 5 times, most recently from 923a6de to 5a93f70 Compare September 25, 2026 03:40
@cursor
cursor Bot force-pushed the cursor/fix-shell-env-path-spaces-a5d6 branch from 5a93f70 to fcd7cc4 Compare September 28, 2026 03:43
trim_end() stripped the newline after eval "${shellHook:-}", so the first
task export (for example Python venv PATH) was glued onto that line. Bash
then consumed the quotes and word-split PATH at spaces, printing
"not a valid identifier" and dropping the rest of PATH.

Fixes cachix#3165

Co-authored-by: Arshdeep singh <arshgill6120@gmail.com>
@cursor
cursor Bot force-pushed the cursor/fix-shell-env-path-spaces-a5d6 branch from fcd7cc4 to 9b9f76c Compare September 30, 2026 04:38
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.

shell-env.sh glues venv task exports onto eval "${shellHook:-}" — PATH values with spaces word-split ( bash: export: ... not a valid identifier)

2 participants