-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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.
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
pnpm.fetchDeps: ensure consistent hashes by setting file permissions after fetching #350063
base: master
Are you sure you want to change the base?
pnpm.fetchDeps: ensure consistent hashes by setting file permissions after fetching #350063
Conversation
A related bug in pnpm recently got fixed in pnpm/pnpm#8625, and got released in pnpm 9.12.2 4 days ago. Were you using the same pnpm version on the 22.04 and 24.04 ubuntu machines? Would you mind testing it with pnpm 9.12.2? (The merge conflict was caused by a formatting change, nothing to worry about when rebasing) |
Thanks for the hint!
Yes, we were using the same version in all cases, but it was 9.12.1. I will check if the inconsistency still occurs with 9.12.2 and report back. |
Finally got around to check: |
For reasons not yet completely understood, `pnpm` might create dependency files with inconsistent file permissions. Since file permissions are stored in the NAR-archive used to derive the hash of a fixed output derivation, this leads to inconsistencies depending on where a derivation is built. Hence, we ensure a consistent file permission scheme: * All files with `-exec` suffix have 555. * All other files have 444. * All folders have 555. This schema was chosen because it as already upheld in most environments we tested.
aa29fd1
to
8bfd975
Compare
@obreitwi There were many changes in pnpm 10, and since not many packages are using it yet, we can afford to change the permissions and therefore the hashes if needed. Would you mind checking whether this bug is reproducible with pnpm 10? |
@gepbird Just double checked: With
Unfortunately, I did not have time to investigate what causes these wrong permissions in the runner any further. |
Interesting that it still happens with pnpm 10.
That's fine, you already spent a lot of time with this problem. I suggest making these permission changes only for pnpm 10 and mentioning it in the release notes that |
For reasons not yet completely understood,
pnpm
might create dependency files with inconsistent file permissions. Since file permissions influence the resulting hash, this PR attempts a workaround by ensuring consistency after the fact.NOTE: I am unsure that this should be merged prior to understanding what causes
pnpm
to generate inconsistent file permissions in the first place.Maybe this PR already helps others facing the same issue.
More details:
We tried to build a small pnpm-based application locally versus within Github actions.
Here we observed build failures due to differing hashes for
pnpmDeps
. After investigation of the actual derivation contents we discovered that on a local, Ubuntu 24.04.1 LTS-based laptops with multi-user nix installs, all files in the derivation either had444
or555
, the same derivation within theubuntu-24.04
-based runner (with single user install via nix-quick-install) had644
/755
as permissions and in different quantities.To make things even more confusing: The
ubuntu-22.04
agent runner showed the same behavior as our local environment.Detailed statistics.
ubuntu-22.04
Github actions runnerubuntu-22.04
Github actions runnerubuntu-24.04
Github actions runnerubuntu-24.04
Github actions runnerSince file permissions are stored in the NAR-archives used to derive the hash of a fixed output derivation, this leads to inconsistencies depending on where a derivation is built.
Hence, we ensure a consistent file permission schema:
-exec
suffix have 555.This schema was chosen because it as already upheld in most environments we tested (i.e. local multi user installs on Ubuntu 24.04.1 LTS and single user install via nix-quick-install in
ubuntu-22.04
-based Github action runners).Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.