by-name-overlay: enable structuredAttrs, strictDeps and parallelBuilding for some directories#483445
Draft
qweered wants to merge 9 commits into
Draft
by-name-overlay: enable structuredAttrs, strictDeps and parallelBuilding for some directories#483445qweered wants to merge 9 commits into
structuredAttrs, strictDeps and parallelBuilding for some directories#483445qweered wants to merge 9 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
qweered
force-pushed
the
by-name-progressive-enhancment-test
branch
from
52f2e60 to
e6cf7c8
Compare
github-actions
Bot
dismissed
their stale review
Review dismissed automatically
qweered
force-pushed
the
by-name-progressive-enhancment-test
branch
from
e6cf7c8 to
abcec42
Compare
nixpkgs-ci
Bot
added
10.rebuild-linux: 101-500
qweered
changed the title
structuredAttrs, strictDeps and parallelBuilding for some directories
qweered
force-pushed
the
by-name-progressive-enhancment-test
branch
from
33a043b to
ec4dd07
Compare
nixpkgs-ci
Bot
added
the
6.topic: python
qweered
force-pushed
the
by-name-progressive-enhancment-test
branch
3 times, most recently
from
f19587a to
61d2dca
Compare
nixpkgs-ci
Bot
added
the
9.needs: reviewer
qweered
requested review from
Aleksanaa,
emilazy,
gepbird,
jopejoe1 and
philiptaron
4 tasks
nixpkgs-ci
Bot
removed
the
9.needs: reviewer
qweered
force-pushed
the
by-name-progressive-enhancment-test
branch
from
61d2dca to
af6e82d
Compare
13 tasks
qweered
force-pushed
the
by-name-progressive-enhancment-test
branch
from
1ef2cc1 to
af09e57
Compare
nixpkgs-ci
Bot
removed
the
6.topic: python
Contributor
Author
|
Contributor
Author
|
Failures are pre existing: |
qweered
added
the
0.kind: enhancement
qweered
force-pushed
the
by-name-progressive-enhancment-test
branch
from
af09e57 to
b84046c
Compare
Member
|
Love to see those features ( From a somewhat selfish perspective: I'm working in the same space, but I'm updating packages in a "by-subject" fashion rather than alphabetically, and I'm a bit worried if I/we will run into (even) more merge conflicts for treewide updates vs fixes for a newly enabled range. I have no actual useful review comment on the implementation though; I'll leave that to my betters. |
Contributor
Author
|
your changes will greatlly reduce amount of rebuilds and will not really conflict with mine, i'll first pickup directories with least amount of rebuilds. |
…` for some directories
fixes _64gram build
qweered
force-pushed
the
by-name-progressive-enhancment-test
branch
from
b84046c to
617ff5b
Compare
Member
This seems blocking; no rebuilds is one of the indicators that a by-name movement was done correctly. I also really don't want to have by-name movement PRs that need to go through a staging cycle. |
Contributor
|
Agree with @jopejoe1 that we shouldn't gate the by-name migration on this separate migration. I do like the various fixes to packages to modernize and enable these attributes. |
Contributor
|
nixpkgs-vet seems like the right tool to use for this kind of thing. e.g. NixOS/nixpkgs-vet#172 or a rule like "once a derivation has strictDeps set, don't allow it to be unset". |
Member
|
Yeah as @uninsane mentioned, this is something for nixpkgs-vet ratchet check |
philiptaron
requested changes
Jan 28, 2026
Contributor
philiptaron
left a comment
philiptaron
left a comment
There was a problem hiding this comment.
Requesting changes for the overlay / ratchet stuff, nothing else.
Contributor
Author
|
I aggree that check for new packages should be implemented via nixpkgs-vet, but what about existing packages? I think its a nice way to do that in overlay instead of every derivation. Yeah also we can't have rebuilds during migration to by-name, good news that there < 2000 packages to be migrated to by-name left |
Contributor
Great. Let's get that to a spot where we can set it down.
It's too arbitrary. Why would one package have one set of rules and the other not, just because of its name?! We need to enforce the real rule, which is that we want to make strictDep = true the default, and we do it by "ratcheting" packages forward. This isn't the way. |
nixpkgs-ci
Bot
added
the
2.status: merge conflict
13 tasks
Contributor
|
To be clear @qweered I would merge these commits sans the overlay change. I really appreciate the work. |
Contributor
Then what is? People have zero incentive to do this ratchet unless they are forced. While there is a movement to enable this manually for packages, this is an effort by a small amount of people. If the config option to make it the default ever has a chance to be enabled, there needs to be some way to encourage people beyond just smiling and saying "well that would be nice :)" I don't see how moving from not by-name to by-name is blocking, since you can just enable these attributes then move the packages. |
Contributor
|
Implemented ratchets for I left out |
Contributor
|
Now that the ratchet is merged, we might actually go a step further and add a feature to the auto-updater bot to try to insert these attributes for package sets where they aren't the default and not yet present in the package attr already. wdyt? |
Contributor
|
@tobim We gotta get the thing updated in a new ci/pinned -- @mdaniels5757 has been trying to make that happen but he could use a hand. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a mechanism to progressively enable modern Nix build defaults (
__structuredAttrs,strictDeps,enableParallelBuilding) for packages inpkgs/by-name, starting with directories that have minimal reverse dependencies.Changes
Adds configurable
enhancedDefaultRangestopkgs/top-level/by-name-overlay.nixto specify which shard ranges get enhanced defaults. Packages in these ranges automatically receive:__structuredAttrs = truestrictDeps = trueenableParallelBuilding = trueInitial range is
_0toa-. Multiple ranges supported so people are not limited to sequential migrationRemaining commits fix packages that broke with these new defaults.
Trade-off
This makes moving packages from
all-packages.nixtopkgs/by-nameharder for affected ranges, as it will trigger rebuilds due to the new defaults being applied.Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.