Skip to content

lib.extendDerivation': init to fix overriding issues caused by lib.extendDerivation#269785

Draft
ShamrockLee wants to merge 2 commits intoNixOS:masterfrom
ShamrockLee:lib-extend-derivation-fix
Draft

lib.extendDerivation': init to fix overriding issues caused by lib.extendDerivation#269785
ShamrockLee wants to merge 2 commits intoNixOS:masterfrom
ShamrockLee:lib-extend-derivation-fix

Conversation

@ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Nov 25, 2023 •
edited
Loading

Description of changes

Add function lib.extendDerivation' as an alternative to lib.extendDerivation that is aware of existing overriders (existing attributes that performs overriding), and use it to fix overriding problem caused by lib.extendDerivation when applying to packages build with stdenv.mkDerivaiton.

Centinuation of #266232

Things done

  • Built on platform(s)
    • x86_64-linux (No derivations are rebuilt)
    • 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/)
  • 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.

Priorities

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 6.topic: lib The Nixpkgs function library labels Nov 25, 2023
msfjarvis
msfjarvis reviewed Nov 25, 2023
View reviewed changes
@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch 2 times, most recently from ea059de to 32f25b1 Compare November 25, 2023 15:38
@github-actions github-actions bot added the 6.topic: kernel The Linux kernel label Nov 25, 2023
@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch 2 times, most recently from 9a5e5d3 to c16d830 Compare November 25, 2023 16:39
@ofborg ofborg bot requested a review from abathur November 25, 2023 21:56
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Nov 25, 2023
@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch from c16d830 to bf4e383 Compare November 26, 2023 10:45
@ShamrockLee ShamrockLee changed the title lib.extendDerivation: fix overriding lib.extendDerivation': init Nov 26, 2023
@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch 3 times, most recently from 1bdd5ec to 45b8e33 Compare November 26, 2023 12:07
@ShamrockLee ShamrockLee mentioned this pull request Nov 26, 2023
Open
13 tasks
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Nov 27, 2023

Rebase to resolve merge conflicts.

@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch from 45b8e33 to 24badd8 Compare November 27, 2023 16:04
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Nov 27, 2023

Result of nixpkgs-review run on x86_64-linux 1

1 package blacklisted:
  • nixos-install-tools

@ShamrockLee ShamrockLee marked this pull request as ready for review November 27, 2023 16:16
@ShamrockLee ShamrockLee changed the title lib.extendDerivation': init lib.extendDerivation': init to fix overriding issues caused by lib.extendDerivation Nov 27, 2023
@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 27, 2023
@ofborg ofborg bot removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Nov 27, 2023
@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch from 24badd8 to 545bef2 Compare November 30, 2023 14:25
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Nov 30, 2023

Rebase onto master to resolve release note merge conflict.

@ShamrockLee ShamrockLee mentioned this pull request Nov 30, 2023
Draft
13 tasks
@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch from 545bef2 to 2a53205 Compare November 30, 2023 15:20
@github-actions github-actions bot removed the 6.topic: kernel The Linux kernel label Nov 30, 2023
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Nov 30, 2023

Split the application of lib.extendDerivation' into another PR to make this one shorter.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 30, 2023
@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch from 2a53205 to 0325c2c Compare December 1, 2023 00:06
infinisil
infinisil reviewed Dec 1, 2023
View reviewed changes
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

This seems really complicated, I don't think I can review this as is unless it's simplified a bunch.

@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch from 0325c2c to 773cf37 Compare December 1, 2023 08:56
@github-actions github-actions bot removed the 6.topic: python Python is a high-level, general-purpose programming language. label Dec 1, 2023
ShamrockLee and others added 2 commits December 1, 2023 16:58
@ShamrockLee
test.overriding: escape test names as shell arguments
a727811
@ShamrockLee @msfjarvis
lib.extendDerivation': init
87c9a09 
Add lib.extendDerivaton' as an overriding-aware alternative to
`lib.extendDerivation`.

Co-authored-by: Harsh Shandilya <[email protected]>
@ShamrockLee ShamrockLee force-pushed the lib-extend-derivation-fix branch from 773cf37 to 87c9a09 Compare December 1, 2023 09:26
@ShamrockLee ShamrockLee mentioned this pull request Dec 1, 2023
Draft
13 tasks
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Dec 1, 2023 •
edited
Loading

Just split the application part into #271391 and #271213, and only focus on the definition and testing of lib.extendDerivation' in this PR. Hope this helps.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 1, 2023
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Dec 1, 2023

@ofborg build tests.overriding

@ShamrockLee ShamrockLee requested a review from infinisil December 1, 2023 18:26
@ShamrockLee ShamrockLee mentioned this pull request Dec 14, 2023
Merged
12 tasks
@ShamrockLee ShamrockLee mentioned this pull request Feb 11, 2024
Open
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Aug 15, 2024

The extendDerivation' implementation is indeed too complex.

Instead of adding another version of the low-level extendDerivation that plays well with high-level overriding, we could encourage developers to use existing functionality such as the passthru attribute and the meta.broken attribute.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 15, 2024
@ShamrockLee ShamrockLee marked this pull request as draft August 15, 2024 14:38
@ShamrockLee ShamrockLee mentioned this pull request Aug 16, 2024
Open
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abathur abathur Awaiting requested review from abathur

@FRidh FRidh Awaiting requested review from FRidh

@infinisil infinisil Awaiting requested review from infinisil

1 more reviewer

@msfjarvis msfjarvis msfjarvis left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: lib The Nixpkgs function library 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ShamrockLee @msfjarvis @infinisil @wegank