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.

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

fetchers: uniformly support the hash attribute #342173

Merged
merged 15 commits into from
Sep 30, 2024
Merged

fetchers: uniformly support the hash attribute #342173

merged 15 commits into from
Sep 30, 2024

Conversation

nbraud
Copy link
Contributor

@nbraud nbraud commented Sep 15, 2024 •
edited
Loading

Description of changes

Went through pkgs/buildsupport/fetch* and adapted fetchers as necessary, to support the hash attribute; see #325892.
Also updated some packages to use the hash attribute, as working examples.

This is a follow-up on #342072 and includes its commits.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
  • Tested compilation of all affected packages using nixpkgs-review
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: fetch 6.topic: lib The Nixpkgs function library labels Sep 15, 2024
@nbraud
Copy link
Contributor Author

nbraud commented Sep 15, 2024 •
edited
Loading

Note to self: give fetchipfs the same treatment, its logic fails to reject { hash = lib.fakeHash; sha256 = lib.fakeSha256; }
Done

@nbraud nbraud added the 2.status: blocked by pr/issue Another PR or issue is preventing this from being completed label Sep 15, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Sep 16, 2024
@nbraud nbraud force-pushed the sha-to-SRI/fetchests branch from ae68fc7 to 3ee66db Compare September 16, 2024 18:17
@nbraud
Copy link
Contributor Author

nbraud commented Sep 16, 2024

Result of nixpkgs-review pr 342173 run on aarch64-darwin 1

1 package built:
  • nixpkgs-manual

@nbraud
Copy link
Contributor Author

nbraud commented Sep 17, 2024

Also updated the corresponding nix-prefetch-* commands, to output SRI hashes by default

@nbraud nbraud force-pushed the sha-to-SRI/fetchests branch 2 times, most recently from 3cdc7b7 to c584b28 Compare September 17, 2024 08:47
@nbraud
Copy link
Contributor Author

nbraud commented Sep 17, 2024

Forgot to update this PR after renaming the lib.fetchers functions

nbraud added 15 commits September 17, 2024 15:00
@nbraud nbraud marked this pull request as ready for review September 17, 2024 15:00
@nbraud nbraud force-pushed the sha-to-SRI/fetchests branch from c584b28 to 7c19bb3 Compare September 17, 2024 15:00
@nbraud nbraud requested a review from philiptaron as a code owner September 17, 2024 15:00
@nbraud nbraud removed the 2.status: blocked by pr/issue Another PR or issue is preventing this from being completed label Sep 17, 2024
@github-actions github-actions bot removed the 6.topic: lib The Nixpkgs function library label Sep 17, 2024
@nbraud
Copy link
Contributor Author

nbraud commented Sep 17, 2024

Result of nixpkgs-review pr 342173 run on aarch64-darwin 1

7 packages built:
  • cabal2nix
  • luarocks-packages-updater
  • nix-prefetch-bzr
  • nix-prefetch-cvs
  • nix-prefetch-scripts
  • nix-update-source
  • nix-update-source.dist

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Sep 17, 2024
@nbraud
Copy link
Contributor Author

nbraud commented Sep 18, 2024

Result of nixpkgs-review pr 342173 run on x86_64-linux 1

8 packages built:
  • cabal2nix
  • haskellPackages.cabal2nix-unstable
  • luarocks-packages-updater
  • nix-prefetch-bzr
  • nix-prefetch-cvs
  • nix-prefetch-scripts
  • nix-update-source
  • nix-update-source.dist

@nbraud nbraud added the needs_reviewer (old Marvin label, do not use) label Sep 18, 2024
philiptaron
philiptaron approved these changes Sep 24, 2024
View reviewed changes
Copy link
Contributor

@philiptaron philiptaron left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A couple One curious question.

pkgs/build-support/fetchsvnssh/default.nix Show resolved Hide resolved
pkgs/build-support/fetchipfs/default.nix
Comment on lines +9 to +10
, outputHash
, outputHashAlgo
Copy link
Contributor

Choose a reason for hiding this comment

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

Others have defaults of lib.fakeHash and null here; why did this one get no default?

Copy link
Contributor Author

@nbraud nbraud Dec 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Because the fetcher was throwing without a hash set, so I preserved that behaviour.

PS: Sorry for getting back to you so late, I missed the “new message in issue/PR” thing as I was using a view filtered for open ones

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 25, 2024
@philiptaron philiptaron merged commit d590835 into NixOS:master Sep 30, 2024
28 of 29 checks passed
@nbraud nbraud deleted the sha-to-SRI/fetchests branch December 30, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philiptaron philiptaron philiptaron approved these changes

Assignees
No one assigned
Labels
6.topic: fetch 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12.approvals: 1 This PR was reviewed and approved by one reputable person needs_reviewer (old Marvin label, do not use)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nbraud @philiptaron @wegank