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

nc4nix: 0-unstable-2024-03-01 -> 0-unstable-2024-08-01 #331507

Merged
merged 5 commits into from
Aug 2, 2024

Conversation

dotlambda
Copy link
Member

@dotlambda dotlambda commented Aug 1, 2024

Description of changes

Here's a bit of a rationale for this change:
In #193075 fetchNextcloudApp was changed to use fetchzip in order to aid reproducability and in helsinki-systems/nc4nix#3 nc4nix followed suit.
In helsinki-systems/nc4nix@3c35a0e nc4nix changed back to hashing the tarballs rather than their content. That's why #326442 left us in a broken state.
The homebrewed bash script that replaced nc4nix in #326947 has its own problems (see that PR), so we should really be using nc4nix again.
As explained in helsinki-systems/nc4nix#3 (comment) I doubt using fetchurl is still an issue (or ever was).
Hence let's use nc4nix again and switch back to fetchurl and be happy :)

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
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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
  • Fits CONTRIBUTING.md.

closes #330967


Add a 👍 reaction to pull requests you find important.

dotlambda and others added 5 commits August 1, 2024 03:48
Nc4nix now generates SRI hashes instead of sha256 hashes.
It also no longer unpacks the tarballs before hashing.

Co-authored-by: Pyrox <[email protected]>
We no longer use fetchzip because nc4nix no longer unpacks the tarball
before computing the hash.
@@ -8,19 +8,12 @@
, patches ? [ ]
, description ? null
, homepage ? null
, unpack ? true # whether to use fetchzip rather than fetchurl
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this is a non-breaking change until we set

Suggested change
, unpack ? true # whether to use fetchzip rather than fetchurl
, unpack ? false # whether to use fetchzip rather than fetchurl

by default. I suggest we do so, accompanied by release notes, before NixOS 24.11.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good 👍
Do you want to file a PR for that?

@dotlambda dotlambda requested a review from onny August 1, 2024 11:42
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Aug 1, 2024
@dotlambda
Copy link
Member Author

I'd prefer to merge this very soon because it contains an important fix of nextcloud/calendar#6114.

@Ma27
Copy link
Member

Ma27 commented Aug 2, 2024

Thanks a lot for taking care of this @dotlambda !

I'm very sorry that I didn't manage to do that myself, this week was just too full so far. Doing a review now.

@Ma27 Ma27 merged commit b663d72 into NixOS:master Aug 2, 2024
25 of 27 checks passed
Copy link
Contributor

github-actions bot commented Aug 2, 2024

Backport failed for release-24.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.05
git worktree add -d .worktree/backport-331507-to-release-24.05 origin/release-24.05
cd .worktree/backport-331507-to-release-24.05
git switch --create backport-331507-to-release-24.05
git cherry-pick -x 1ad15c918a99581f285f447956fa6e099ea0a889 b8a80e15e5a952b85322ab742d9dc823e9c800b3 73eb537d17631266173c86d40af040c325c79498 2c1d58e90c06d14674c59f7813023b4b71494b32 5c812046c3b38d1c4775fed2e116e0b4537be800

@Ma27
Copy link
Member

Ma27 commented Aug 2, 2024

Expected that. Filing a backport now.

@dotlambda dotlambda deleted the nextcloud branch August 2, 2024 08:33
Ma27 added a commit that referenced this pull request Aug 2, 2024
[24.05]  nc4nix: 0-unstable-2024-03-01 -> 0-unstable-2024-08-01 #331507, update nextcloud apps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: fetch 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 backport release-24.05 Backport PR automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants