Skip to content

Pin shell in installer-generated env setup (Schniz#1525) #204

Open
Dargon789 wants to merge 10 commits intoDargon789:Dargon789-patch-9from
Schniz:master
Open

Pin shell in installer-generated env setup (Schniz#1525) #204
Dargon789 wants to merge 10 commits intoDargon789:Dargon789-patch-9from
Schniz:master

Conversation

@Dargon789
Copy link
Copy Markdown
Owner

@Dargon789 Dargon789 commented Apr 5, 2026

Pin shell in installer-generated env setup (https://github.com/Schniz/fnm/pull/1525[)](https://github.com/Dargon789/fnm/commit/bfb186034978b1ddaf87501eb1633bdc42a5c0a6)

  • perf(install): pin shell in generated env setup

  • ci: pin docker platform for ARM installer tests

  • chore: add changeset for installer shell update

Summary by Sourcery

Optimize fnm env --use-on-cd shell setup, harden shell hooks across platforms, and update installer, architecture support, and docs accordingly.

New Features:

  • Add support for the x64-glibc-217 architecture via the --arch x64-glibc-217 option in fnm env.

Bug Fixes:

  • Ensure Windows CMD --use-on-cd hooks work with paths containing spaces and handle drive changes correctly.
  • Avoid duplicating the zsh --use-on-cd hook when shell configuration is re-sourced.
  • Fix installer-generated Windows CMD macro so the cd hook path is correctly quoted.

Enhancements:

  • Reduce shell startup overhead for fnm env --use-on-cd by applying the current directory Node version during fnm env execution instead of via an extra fnm use subprocess.
  • Prefer explicit --shell flags in installer-generated shell setup to avoid runtime shell inference and process tree detection.
  • Route informational fnm use output to stderr when invoked internally from fnm env to keep stdout clean for shell evaluation.
  • Improve the fnm use interactive missing-version prompt by prefixing the message with fnm for clearer attribution.
  • Make configuration and directory helper types clonable to support reuse when computing multishell paths.

Build:

  • Bump crate and npm package versions from 1.38.1 to 1.39.0 and add changesets describing the new architecture support and installer shell changes.

CI:

  • Pin Docker --platform for ARM installer tests in the installation script CI workflow and normalize YAML formatting.

Documentation:

  • Document the recommendation to pass an explicit --shell to fnm env and update examples in CLI docs and README accordingly.
  • Update changelog for version 1.39.0 with the new features and fixes.

Tests:

  • Add end-to-end tests for immediate version application and re-sourcing behavior of --use-on-cd env setup across shells.
  • Add unit tests to verify zsh hook de-duplication and Windows CMD hook quoting for paths with spaces.

Chores:

  • Add changeset entries for shell behavior updates, architecture support, and clarified prompts, and remove now-obsolete changesets.

Schniz and others added 6 commits March 5, 2026 15:47
* fix(zsh): dedupe use-on-cd hook on re-source

* fix(wincmd): support spaces and drive switches in use-on-cd

* test(e2e): cover use-on-cd after re-sourcing env

* chore: add changeset for use-on-cd shell fixes
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* feat: support x64-glibc-217

* add changeset

---------

Co-authored-by: Gal Schlezinger <[email protected]>
* fix(use): clarify interactive install prompt source

* chore: add changeset for prompt clarification
* perf(install): pin shell in generated env setup

* ci: pin docker platform for ARM installer tests

* chore: add changeset for installer shell update
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 5, 2026

Reviewer's Guide

Pins the shell used by the installer-generated env setup, optimizes env --use-on-cd by running fnm use once during fnm env, improves shell hooks (especially zsh and Windows cmd), adds x64-glibc-217 arch support, and adjusts CI and docs accordingly for the new behavior.

Sequence diagram for fnm env --use-on-cd applying initial version

sequenceDiagram
    actor User
    participant Shell
    participant Fnmbin as fnm_binary
    participant EnvCmd as Env_command
    participant UseCmd as Use_command
    participant OS as OS_environment

    User->>Shell: evaluate env setup (fnm env --use-on-cd --shell <shell>)
    Shell->>Fnmbin: invoke Env subcommand
    Fnmbin->>EnvCmd: run Env.apply(config)

    EnvCmd->>OS: compute multishell_path
    EnvCmd->>OS: set_path_for_multishell(multishell_path)
    OS-->>EnvCmd: PATH updated to include multishell bin

    EnvCmd->>EnvCmd: config_with_multishell = config.with_multishell_path(multishell_path)
    EnvCmd->>UseCmd: construct Use_command(info_to_stderr = true, install_if_missing = false, silent_if_unchanged = true)

    EnvCmd->>OS: check NO_COLOR, stdout, stderr
    alt should_force_stderr_color
        EnvCmd->>OS: enable colored override
    end

    EnvCmd->>UseCmd: Use.apply(config_with_multishell)
    UseCmd->>OS: read version file and select Node version
    UseCmd->>OS: update PATH for selected Node version
    UseCmd-->>EnvCmd: result (errors ignored)

    alt should_force_stderr_color
        EnvCmd->>OS: disable colored override
    end

    EnvCmd->>Shell: print shell hook script (shell.use_on_cd(config))
    Shell-->>User: hook installed without extra fnm use subprocess on first cd
Loading

Updated class diagram for FnmConfig, Use, Arch, and related types

classDiagram
    class FnmConfig {
        <<clap::Parser, Clone>>
        +mirror Url
        +arch Arch
        +multishell_path Option~PathBuf~
        +with_base_dir(base_dir Option~PathBuf~) FnmConfig
        +with_multishell_path(multishell_path PathBuf) FnmConfig
    }

    class Directories {
        <<Clone>>
        +base BaseStrategy
        +new() Directories
        +cache_dir() PathBuf
    }

    class Use {
        +version Option~UserVersion~
        +install_if_missing bool
        +silent_if_unchanged bool
        +info_to_stderr bool
        +apply(config &FnmConfig) Result~(), Error~
    }

    class Arch {
        <<enum>>
        X86
        X64
        X64Musl
        X64Glibc217
        Arm64
        Armv7l
        Ppc64le
        Arm64Musl
        +to_string() &str
        +from_str(s &str) Result~Arch, Error~
    }

    class EnvCommand {
        +use_on_cd bool
        +apply(config &FnmConfig) Result~(), Error~
        -set_path_for_multishell(multishell_path &Path)
    }

    class Shell {
        <<trait>>
        +env(config &FnmConfig) String
        +use_on_cd(config &FnmConfig) Result~String, Error~
        +rehash() Option~String~
    }

    class Zsh {
        +use_on_cd(config &FnmConfig) Result~String, Error~
        -add_zsh_hook_remove_existing()
    }

    class Bash {
        +use_on_cd(config &FnmConfig) Result~String, Error~
    }

    class Fish {
        +use_on_cd(config &FnmConfig) Result~String, Error~
    }

    class PowerShell {
        +use_on_cd(config &FnmConfig) Result~String, Error~
    }

    class WindowsCmd {
        +use_on_cd(config &FnmConfig) Result~String, Error~
        +create_cd_file_at(path &Path) Result~(), IoError~
    }

    FnmConfig *-- Directories
    FnmConfig o-- Arch
    EnvCommand ..> Use : constructs
    EnvCommand ..> FnmConfig : reads and clones
    Use ..> FnmConfig : uses
    Zsh ..|> Shell
    Bash ..|> Shell
    Fish ..|> Shell
    PowerShell ..|> Shell
    WindowsCmd ..|> Shell
Loading

File-Level Changes

Change Details Files
Apply the initial Node version during fnm env --use-on-cd instead of deferring to a separate fnm use subprocess, and route its informational output to stderr when invoked internally.
  • Add helper to prepend the multishell bin directory to PATH before running Use so the correct Node is available in-process.
  • Clone FnmConfig with an explicit multishell path via new with_multishell_path to reuse existing config when running Use internally.
  • Extend Use command with an info_to_stderr flag and switch logging to stderr when set, used by env to avoid contaminating stdout shell code.
  • Update internal fnm use call sites (install/use) to set info_to_stderr explicitly to maintain previous behavior.
  • Clarify interactive missing-version error message to prefix with fnm in the prompt text.
src/commands/env.rs
src/commands/use.rs
src/commands/install.rs
Make shell hooks more robust and more precisely pinned, particularly for zsh, fish, bash, PowerShell, and Windows cmd, and update installer script and tests accordingly.
  • Change zsh use-on-cd hook generation to first remove any existing _fnm_autoload_hook (add-zsh-hook -D) before re-adding it, and add a unit test to assert this behavior.
  • Remove eager invocation of the use-on-cd hook (_fnm_autoload_hook / __fnm_use_if_file_found / Set-FnmOnLoad) from fish, bash, and PowerShell snippets so the initial version is now applied from fnm env instead.
  • Modify cd.cmd to use cd /d %* so drive-letter changes work in Windows cmd, and adjust the WindowsCmd use_on_cd macro to quote the path in the doskey definition for paths with spaces, including a unit test.
  • Update e2e tests around use-on-cd to assert immediate use of the current directory version after env setup and correct behavior when sourcing env multiple times across shells.
src/shell/zsh.rs
src/shell/fish.rs
src/shell/bash.rs
src/shell/powershell.rs
src/shell/windows_cmd/mod.rs
src/shell/windows_cmd/cd.cmd
e2e/use-on-cd.test.ts
Pin explicit shells in installer-generated setup and adjust documentation to recommend explicit --shell usage.
  • Update .ci/install.sh to call fnm env with explicit --shell flags for zsh, fish, and bash in the generated shell configuration.
  • Change CLI and docs examples (README, docs/commands.md) to use fnm env --shell <shell> instead of relying on shell inference.
  • Tweak README formatting and links around Windows cmd instructions for consistency.
.ci/install.sh
src/cli.rs
README.md
docs/commands.md
Improve architecture handling and bump project version for release, including changeset metadata.
  • Introduce Arch::X64Glibc217 variant with proper string mapping and parsing to support --arch x64-glibc-217 in fnm env.
  • Derive Clone for FnmConfig and Directories to support config cloning needed by the new env behavior.
  • Bump crate and package versions from 1.38.1 to 1.39.0 and add changelog entries summarizing the new features and fixes.
  • Add changeset files documenting the shell pinning change, the new arch support, and the clarified fnm use prompt; remove obsolete changesets.
src/arch.rs
src/config.rs
src/directories.rs
Cargo.toml
package.json
CHANGELOG.md
.changeset/brave-timers-move.md
.changeset/cuddly-cars-ring.md
.changeset/fair-carrots-greet.md
.changeset/grumpy-dingos-turn.md
.changeset/odd-mayflies-poke.md
.changeset/twelve-badgers-happen.md
Adjust installer CI to pin ARM Docker platforms and make minor workflow/style updates.
  • Change the ARM installation workflow matrix to include both Docker image and explicit --platform for arm64v8 and arm32v7 runs.
  • Pass the selected platform into docker run for ARM tests to avoid host-architecture ambiguity.
  • Normalize YAML quoting/spacing in the installation_script workflow for consistency.
.github/workflows/installation_script.yml

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented Apr 5, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 5, 2026

⚠️ The sha of the head commit of this PR conflicts with #199. Mergify cannot evaluate rules on this PR. Once #199 is merged or closed, Mergify will resume processing this PR. ⚠️

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In set_path_for_multishell, the unsafe block around std::env::set_var is unnecessary since set_var is safe; you can drop the unsafe and simplify the function.
  • In windows_cmd tests, consider using a tempfile::TempDir (or similar RAII temp dir) and avoiding unwrap() on cleanup calls so tests don't panic or leave stray directories/files behind if cd.cmd isn't created as expected.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `set_path_for_multishell`, the `unsafe` block around `std::env::set_var` is unnecessary since `set_var` is safe; you can drop the `unsafe` and simplify the function.
- In `windows_cmd` tests, consider using a `tempfile::TempDir` (or similar RAII temp dir) and avoiding `unwrap()` on cleanup calls so tests don't panic or leave stray directories/files behind if `cd.cmd` isn't created as expected.

## Individual Comments

### Comment 1
<location path="docs/commands.md" line_range="383" />
<code_context>
 This command generates a series of shell commands that should be evaluated by your shell to create a fnm-ready environment.

-Each shell has its own syntax of evaluating a dynamic expression. For example, evaluating fnm on Bash and Zsh would look like `eval "$(fnm env)"`. In Fish, evaluating would look like `fnm env | source`
+Each shell has its own syntax of evaluating a dynamic expression. For example, evaluating fnm on Bash and Zsh would look like `eval "$(fnm env --shell bash)"`. In Fish, evaluating would look like `fnm env --shell fish | source`

 Usage: fnm env [OPTIONS]
</code_context>
<issue_to_address>
**suggestion (typo):** Use more idiomatic phrasing "syntax for evaluating" instead of "syntax of evaluating".

Please change the sentence to: "Each shell has its own syntax for evaluating a dynamic expression."

```suggestion
Each shell has its own syntax for evaluating a dynamic expression. For example, evaluating fnm on Bash and Zsh would look like `eval "$(fnm env --shell bash)"`. In Fish, evaluating would look like `fnm env --shell fish | source`
```
</issue_to_address>

Fix all in Cursor


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread docs/commands.md
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates fnm to version 1.39.0, adding support for the x64-glibc-217 architecture and optimizing shell startup for env --use-on-cd by applying the initial version internally. It also improves shell hook robustness for Zsh and Windows CMD, adds a use flag to the install command, and clarifies the missing-version prompt. Feedback suggests adding safety documentation for the unsafe environment variable modification and refactoring the output utility to handle stderr redirection more cleanly than repurposing the Error log level.

Comment thread src/commands/env.rs
Comment thread src/commands/use.rs
@Dargon789 Dargon789 enabled auto-merge (squash) April 5, 2026 14:24
@Dargon789 Dargon789 linked an issue Apr 5, 2026 that may be closed by this pull request
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
auto-merge was automatically disabled April 17, 2026 22:23

Head branch was pushed to by a user without write access

@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 17, 2026

⚠️ The sha of the head commit of this PR conflicts with #199. Mergify cannot evaluate rules on this PR. Once #199 is merged or closed, Mergify will resume processing this PR. ⚠️

* feat: add possible values to the arch help docs.

* add a changeset

---------

Co-authored-by: Gal Schlezinger <[email protected]>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 17, 2026

@theborowski is attempting to deploy a commit to the Foundry development Team on Vercel.

A member of the Team first needs to authorize it.

@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 17, 2026

⚠️ The sha of the head commit of this PR conflicts with #199. Mergify cannot evaluate rules on this PR. Once #199 is merged or closed, Mergify will resume processing this PR. ⚠️

* docs: add e2e testing guide for agents

* docs: trim AGENTS e2e section

* docs: refine AGENTS guidance for e2e and changesets
@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 17, 2026

⚠️ The sha of the head commit of this PR conflicts with #199. Mergify cannot evaluate rules on this PR. Once #199 is merged or closed, Mergify will resume processing this PR. ⚠️

* Add random number to multishell symlinks

This protects against collisions when multiple shells are started in parallel
inside Bubblewrap sandboxes, as each shell will have the same timestamp and the
same low PID (e.g. 2), since Bubblewrap sandboxes have isolated PID spaces.

* Add a changeset

---------

Co-authored-by: Gal Schlezinger <[email protected]>
@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 17, 2026

⚠️ The sha of the head commit of this PR conflicts with #199. Mergify cannot evaluate rules on this PR. Once #199 is merged or closed, Mergify will resume processing this PR. ⚠️

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.

#### Flow diagram for CircleCI job execution steps

5 participants