fix: resolve npm global module path on Windows#9
Conversation
On Windows, npm global installs go to <prefix>/node_modules/<pkg>, while on Linux/macOS they go to <prefix>/lib/node_modules/<pkg>. The hardcoded lib/node_modules path caused setup-utoo to fail on Windows with "package.json could not be found or read" because it looked in the wrong directory after npm install. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical cross-platform compatibility issue within the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue with resolving npm global module paths on Windows by introducing a helper function, getNpmGlobalModulePath, that generates the correct path based on the operating system. The change is well-implemented and fixes the described bug. I have one minor suggestion to improve code maintainability by reducing a small amount of code duplication.
|
|
||
| async function getUtooVersion(binPath: string): Promise<string | undefined> { | ||
| try { | ||
| const prefix = binPath.replace(/[/\\]bin$/, ""); |
There was a problem hiding this comment.
While extracting the prefix calculation into a variable improves readability here, the same logic (binPath.replace(/[\/\\]bin$/, "")) is also present in the installUtoo function on line 167. To avoid this duplication and improve maintainability, consider extracting this logic into a dedicated helper function.
For example:
function getNpmPrefixFromBinPath(binPath: string): string {
return binPath.replace(/[\/\\]bin$/, "");
}This new function could then be used both here and in installUtoo.
On Windows, npm global install puts binaries in <prefix>/ directly rather than <prefix>/bin/. Add prefix dir to PATH so ut/utoo commands are found. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
npm-generated .ps1 shims resolve #!/bin/bash shebangs to /bin/bash.exe, which doesn't exist on Windows. Copy Git's bash.exe to C:\bin\bash.exe so utoo's bin scripts can be executed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PowerShell can't resolve Unix-style /bin/bash.exe paths. Instead of trying to provide bash.exe, create .cmd wrappers that invoke utoo's JS entry point directly via node. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move shim creation to after npm install completes, and always overwrite existing .cmd files since npm's generated ones don't work. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PowerShell prioritizes .ps1 over .cmd. The npm-generated .ps1 shims use /bin/bash which doesn't exist on Windows. Delete them so PowerShell falls through to our working .cmd wrappers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
utoo's bin/utoo is a native binary (Rust), not a JS script. The .cmd shim should invoke it directly instead of through node. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
utoo's postinstall.sh uses bash which may not run on Windows.
Detect placeholder binary and copy the native one from @utoo/utoo-win32-{arch}.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GitHub Actions Windows runners have Git Bash at C:\Program Files\Git\bin\bash.exe. Use it to run utoo's postinstall.sh which installs the native binary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Log all candidate paths to help debug where npm installs the platform-specific package on Windows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Windows requires .exe extension to execute native binaries from .cmd. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
utoo executes npm scripts via bash, which can't run PE binaries. Create a bash shim (no extension) that execs the .exe binary, alongside the .cmd shim for PowerShell/cmd.exe. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract IS_WINDOWS constant, use prefixDir consistently - Rename createWindowsCmdShims → fixWindowsShims (clearer intent) - Move ensureNativeBinary into installUtoo (only needed on install) - Remove verbose debug logging - Use utoo (not ut) for setRegistry command - Simplify error handling Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove all Windows shim workarounds (ensureNativeBinary, fixWindowsShims). These are now handled by utoo's postinstall.js in the npm package itself. setup-utoo only needs to fix the npm global module path difference: - Windows: <prefix>/node_modules/<pkg> - Linux/macOS: <prefix>/lib/node_modules/<pkg> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add utoo-tgz input to install from a pre-built tgz package instead of npm registry. Useful for testing unreleased versions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
On Windows,
npm install -g --prefixinstalls packages to<prefix>/node_modules/<pkg>, while on Linux/macOS it installs to<prefix>/lib/node_modules/<pkg>.The hardcoded
lib/node_modulespath ingetUtooVersion()andnpmLibDircaused setup-utoo to fail on Windows CI with:Fix
Add
getNpmGlobalModulePath()helper that returns the correct path based onos.platform():<prefix>/node_modules/<pkg><prefix>/lib/node_modules/<pkg>Applied to both
npmLibDir(cache paths) andgetUtooVersion()(installation verification).Test plan
🤖 Generated with Claude Code