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

{ruby,neovim}: fix build with content-addressed derivations #258705

Merged
merged 2 commits into from
Mar 24, 2024

Conversation

tie
Copy link
Member

@tie tie commented Oct 3, 2023

Description of changes

This PR fixes some packages that were broken after #211783 with contentAddressedByDefault. See #211783 (comment), #211783 (comment), and #214044 (comment)

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.11 Release Notes (or backporting 23.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.

@tie
Copy link
Member Author

tie commented Oct 3, 2023

Wrt #211783, I think this should be a separate parameter for mkDerivation instead of overriding the meaning of (dis)allowed{References,Requisites} by discarding the context (breaking content-addressed derivations along the way).

@Artturin
Copy link
Member

Artturin commented Oct 5, 2023

Should target staging, follow contributing.md to rebase. Undraft once done.

@Artturin Artturin marked this pull request as draft October 5, 2023 04:30
@tie
Copy link
Member Author

tie commented Oct 5, 2023

I don’t think a rebase is necessary.

$ git rebase --onto upstream/staging... upstream/master
Current branch fix-ca-derivations-disallowed is up to date.

@tie tie changed the base branch from master to staging October 5, 2023 06:15
@tie tie marked this pull request as ready for review October 5, 2023 06:16
DarkKirb added a commit to DarkKirb/nix-packages that referenced this pull request Nov 7, 2023
@Artturin Artturin requested a review from r-vdp November 7, 2023 23:02
@ConnorBaker
Copy link
Contributor

This is kind of difficult to review due to the number of rebuilds. Any suggestions for a more limited way to run nixpkgs-review which verifies that this 1) fixes the problem and 2) doesn't break anything else?

@r-vdp
Copy link
Contributor

r-vdp commented Feb 8, 2024

Why do we have stdenv.cc for neovim and stdenv.cc.cc for ruby, in disallowedRequisites ?

@pillowtrucker
Copy link
Contributor

fwiw I've been testing the ruby side of this for 2 days and haven't had any issues

Copy link
Contributor

@marsam marsam left a comment

Choose a reason for hiding this comment

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

Thank you!

@marsam marsam merged commit 28e51c9 into NixOS:staging Mar 24, 2024
27 checks passed
@tie tie deleted the fix-ca-derivations-disallowed branch May 3, 2024 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants