Skip to content
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

prefetch-npm-deps: fix reproducibility #214454

Merged

Conversation

winterqt
Copy link
Member

@winterqt winterqt commented Feb 4, 2023

Description of changes

v1 lockfiles can contain multiple references to the same version of a
package, and these references can contain different integrity values,
such as one having SHA-1 and SHA-512, while another just has SHA-512.

Given that HashMap iteration order isn't defined, this causes
reproducibility issues, as a different integrity value could be chosen
each time.

Thanks to @lilyinstarlight for discovering this issue originally, as well
as the idea for the sorting-based implementation.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

v1 lockfiles can contain multiple references to the same version of a
package, and these references can contain different `integrity` values,
such as one having SHA-1 and SHA-512, while another just has SHA-512.

Given that HashMap iteration order isn't defined, this causes
reproducibility issues, as a different integrity value could be chosen
each time.

Thanks to @lilyinstarlight for discovering this issue originally, as well
as the idea for the sorting-based implementation.
@lilyinstarlight lilyinstarlight force-pushed the prefetch-npm-deps-reproducibility branch 4 times, most recently from f042929 to c7db220 Compare April 28, 2023 23:25
@lilyinstarlight lilyinstarlight marked this pull request as ready for review April 28, 2023 23:27
@lilyinstarlight lilyinstarlight force-pushed the prefetch-npm-deps-reproducibility branch from c7db220 to ac35d7e Compare April 30, 2023 14:30
@lilyinstarlight
Copy link
Member

By the way, I've now confirmed all 46 npm FODs in nixpkgs still reproduce to the same hash after this change (and now some like open-stage-control.npmDeps won't randomly get a different hash sometimes)

FODs checked, by attr (required some local changes not in nixpkgs to get all of them exposed via attrs):
ansible-language-server.npmDeps
audiobookshelf.npmDeps
authelia.web.npmDeps
bitwarden.npmDeps
cdxgen.npmDeps
deltachat-desktop.npmDeps
docker-compose-language-service.npmDeps
dot-language-server.npmDeps
element-desktop.keytar.npmDeps
emscripten.nodeModules.npmDeps
evcc.npmDeps
forgejo.frontend.npmDeps
geph.gui.gephgui.npmDeps
hred.npmDeps
iosevka-comfy.comfy-duo.npmDeps
iosevka-comfy.comfy-fixed.npmDeps
iosevka-comfy.comfy-motion-duo.npmDeps
iosevka-comfy.comfy-motion-fixed.npmDeps
iosevka-comfy.comfy-motion.npmDeps
iosevka-comfy.comfy-wide-duo.npmDeps
iosevka-comfy.comfy-wide-fixed.npmDeps
iosevka-comfy.comfy-wide.npmDeps
iosevka-comfy.comfy.npmDeps
iosevka.npmDeps
kaufkauflist.npmDeps
lv_img_conv.npmDeps
matrix-appservice-irc.npmDeps
mongosh.npmDeps
mpvScripts.webtorrent-mpv-hook.npmDeps
open-stage-control.npmDeps
paperless-ngx.frontend.npmDeps
photoprism.frontend.npmDeps
pnpm-lock-export.npmDeps
sharing.npmDeps
sunshine.ui.npmDeps
terminal-stocks.npmDeps
torq.web.npmDeps
twspace-crawler.npmDeps
uptime-kuma.npmDeps
vaultwarden.webvault.npmDeps
vieb.npmDeps
vsce.npmDeps
webcord-vencord.vencord.npmDeps
webcord.npmDeps
zigbee2mqtt.npmDeps
zinc.webui.npmDeps

Copy link
Member

@RaitoBezarius RaitoBezarius left a comment

Choose a reason for hiding this comment

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

LGTM

@lilyinstarlight lilyinstarlight merged commit 6a6f328 into NixOS:master May 4, 2023
@CMCDragonkai
Copy link
Member

This is not in 23.05 atm right?

@lilyinstarlight
Copy link
Member

lilyinstarlight commented Oct 16, 2023

This is not in 23.05 atm right?

This was merged before the 23.05 branch-off, so it definitely is in 23.05. Are you having an issue still?

Feel free to open a new issue on nixpkgs if you are and ping me on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

buildNodePackage produces ENOTCACHED when referring to the same package via different hashes (?)
4 participants