Skip to content

Update npm install command to use --os flag #985

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 9, 2025
+6 −1
Merged

Update npm install command to use --os flag #985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/spicy-mangos-shop.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@opennextjs/aws": patch
---

Update npm install command to use --os flag
2 changes: 1 addition & 1 deletion packages/open-next/src/build/installDeps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function installDependencies(
: "";

const additionalArgs = installOptions.additionalArgs ?? "";
const installCommand = `npm install --platform=linux ${archOption} ${targetOption} ${libcOption} ${additionalArgs} ${installOptions.packages.join(" ")}`;
const installCommand = `npm install --os=linux ${archOption} ${targetOption} ${libcOption} ${additionalArgs} ${installOptions.packages.join(" ")}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should provide both if it does not cause any issue, not just replace it

Copy link
Contributor

@vicb vicb Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@conico974 It looks like there is no --platform option. What about updating the code to use --os=${os.platform()}

It looks like it would help with local e2e on Mac

/cc @sommeeeer

Copy link
Contributor

Choose a reason for hiding this comment

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

See https://sharp.pixelplumbing.com/install/#npm-v10 (kudos to Magnus for finding that link)

Copy link
Contributor

@sommeeeer sommeeeer Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We could introduce another optional parameter to InstallOptions that is os and defaults to linux. Then here we set it to what you suggested if the config.imageOptimization.loader is fs-dev. WDYT?

Update: However, does glibc work on macOS?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to your idea @sommeeeer

We could also detect the CPU (i.e. arm64 vs x64)

Update: However, does glibc work on macOS?

I'll test and report.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember when we added the --platform options TBH, it's a very old one.
I'm fine with adding the os option.
Autodetecting is something that we need to be able to override though, this could break some deployment otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, been busy with other things and haven't gotten round to this PR. But if I understand correctly, autodetecting would break my deployment since I usually deploy through GitHub actions but on rare occasions I've had to deploy from local to fix issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

What they suggest is only if you use the fs-dev image loader (which is likely not your case).
But even in this case I think that if the user provide some options we should respect it

execSync(installCommand, {
stdio: "pipe",
cwd: tempInstallDir,
Expand Down
Loading