Skip to content

Using pnpm on Windows binaries as well #2267

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

Jump to bottom
Merged
malice00 merged 1 commit into from
Sep 9, 2025
Merged

Using pnpm on Windows binaries as well #2267

malice00 merged 1 commit into from
Sep 9, 2025

Conversation

@malice00
Copy link
Collaborator

@malice00 malice00 commented Sep 9, 2025

The issue with not being able to use pnpm, was that pnpm links dependencies to its central store, which on Windows is done with 'junctions' -- which are full paths. This means that the packaged binary doesn't work, because it is linking to non-existing files! Hoisting the packages creates a flat node_modules folder without any links.

Also, the package 'encoding' can't be optional on MacOS and Windows.

@malice00 malice00 requested a review from prabhu as a code owner September 9, 2025 11:16
@malice00
Using pnpm on Windows binaries as well
fb5d587 
The issue with not being able to use pnpm, was that pnpm links dependencies to its central store,
which on Windows is done with 'junctions' -- which are full paths.
This means that the packages binary doesn't work, because it is linking to non-existing files!
Hoisting the packages creates a flat node_modules folder without any links.

Also, the package 'encoding' can't be optional on MacOS and Windows.

Signed-off-by: Roland Asmann <roland.asmann@gmail.com>
@prabhu
Copy link
Collaborator

prabhu commented Sep 9, 2025

Which package needs encoding? Can we bring it in rather than use it as a dependency, since it is just a single file?

https://github.com/andris9/encoding/blob/master/lib/encoding.js

@malice00
Copy link
Collaborator Author

malice00 commented Sep 9, 2025

It is pulled in by @npmcli/arborist:

@npmcli/arborist 9.1.4
├─┬ @npmcli/metavuln-calculator 9.0.1
│ └─┬ pacote 21.0.0
│   ├─┬ @npmcli/run-script 9.1.0
│   │ └─┬ node-gyp 11.4.2
│   │   └─┬ make-fetch-happen 15.0.1
│   │     └─┬ minipass-fetch 4.0.1
│   │       └── encoding 0.1.13
│   ├─┬ npm-registry-fetch 18.0.2
│   │ ├─┬ make-fetch-happen 15.0.1
│   │ │ └─┬ minipass-fetch 4.0.1
│   │ │   └── encoding 0.1.13
│   │ └─┬ minipass-fetch 4.0.1
│   │   └── encoding 0.1.13
│   └─┬ sigstore 3.1.0
│     ├─┬ @sigstore/sign 3.1.0
│     │ └─┬ make-fetch-happen 15.0.1
│     │   └─┬ minipass-fetch 4.0.1
│     │     └── encoding 0.1.13
│     └─┬ @sigstore/tuf 3.1.1
│       └─┬ tuf-js 3.1.0
│         └─┬ make-fetch-happen 15.0.1
│           └─┬ minipass-fetch 4.0.1
│             └── encoding 0.1.13
├─┬ @npmcli/run-script 9.1.0
│ └─┬ node-gyp 11.4.2
│   └─┬ make-fetch-happen 15.0.1
│     └─┬ minipass-fetch 4.0.1
│       └── encoding 0.1.13
├─┬ npm-registry-fetch 18.0.2
│ ├─┬ make-fetch-happen 15.0.1
│ │ └─┬ minipass-fetch 4.0.1
│ │   └── encoding 0.1.13
│ └─┬ minipass-fetch 4.0.1
│   └── encoding 0.1.13
└─┬ pacote 21.0.0
  ├─┬ @npmcli/run-script 9.1.0
  │ └─┬ node-gyp 11.4.2
  │   └─┬ make-fetch-happen 15.0.1
  │     └─┬ minipass-fetch 4.0.1
  │       └── encoding 0.1.13
  └─┬ npm-registry-fetch 18.0.2
    └─┬ make-fetch-happen 15.0.1
      └─┬ minipass-fetch 4.0.1
        └── encoding 0.1.13

The issue occurs when I try to install dependencies without optional (for slim binaries):

> pnpm install --config.strict-dep-builds=true --frozen-lockfile --package-import-method copy --prod --config.node-linker=hoisted --no-optional

Lockfile is up to date, resolution step is skipped
 ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY  Broken lockfile: no entry for 'encoding@0.1.13' in pnpm-lock.yaml

This issue is probably caused by a badly resolved merge conflict.
To fix the lockfile, run 'pnpm install --no-frozen-lockfile'.
 ELIFECYCLE  Command failed with exit code 1.

If I leave out the --no-optional (eg non-slim), it works. If I do not use --config.node-linker, it also works.

I'm not sure how much it helps to include it, because we probably need to somehow tell all the above packages with references that we include it as a file?

@prabhu
Copy link
Collaborator

prabhu commented Sep 9, 2025

Ah, ok. I am working on a branch to bring Arborist (lite version) into the repo.

@malice00 malice00 merged commit 9404ce3 into master Sep 9, 2025
87 of 90 checks passed
@malice00 malice00 deleted the fix/binary-builds branch September 9, 2025 13:25
@malice00
Copy link
Collaborator Author

malice00 commented Sep 9, 2025

Please rebase your branch on this then, so we can figure our if we still need those dependencies!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@prabhu prabhu Awaiting requested review from prabhu prabhu is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@malice00 @prabhu