Skip to content

Allow meta.problems while preventing internal use of certain kinds#533376

Merged
MattSturgeon merged 6 commits into
NixOS:masterfrom
tweag:attrPathsToRemove
Jun 26, 2026
Merged

Allow meta.problems while preventing internal use of certain kinds#533376
MattSturgeon merged 6 commits into
NixOS:masterfrom
tweag:attrPathsToRemove

Conversation

@infinisil

@infinisil infinisil commented Jun 19, 2026

Copy link
Copy Markdown
Member

This allows setting meta.problems.removal (which gives a warning by default) on Nixpkgs packages, while also preventing Nixpkgs from depending on packages that have a removal or deprecated problem.

This is implemented by doing more or less the same thing that aliases.nix does: Making such packages internally unavailable from the Nixpkgs evaluation, meaning they can't be used as dependencies anymore.

Best reviewed commit-by-commit

Closes #523712

Things done

  • Tested by (packages were chosen to be in the first eval chunk, which is used with quickTest)
    • Adding meta.problems.removal.message = "..."; to pkgs.CuboCore.coregarage
    • Successfully running nix-build ci -A eval.singleSystem --arg quickTest true
    • Adding CuboCore.coregarage as a dependency to pkgs.OVMF
    • Expectedly fail to run nix-build ci -A eval.singleSystem --arg quickTest true, with the following error message:
      error: evaluation aborted with the following error message: 'CuboCore.coregarage is disallowed from being used within Nixpkgs, because it has certain meta.problems whose kinds are disallowed: {
        removal = {
          kind = "removal";
          message = "is removed";
        };
      }'
      
  • What if another problem kind were to give warnings, while internal use in Nixpkgs is allowed? What if another problem kind were internally disallowed in Nixpkgs but it doesn't give any warnings? What if we change the level of a problem in config.nix? Just think this through, might not need changes

Makes future changes easier
This ensures that external tooling can also process the problems, and
helps users with debugging.
@infinisil infinisil force-pushed the attrPathsToRemove branch 2 times, most recently from 165c2fd to 889b69f Compare June 19, 2026 19:43
@llakala

llakala commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

I'm concerned that the attribute filtering logic being run when not filtering anything will be a performance loss outside of CI. I can test this later.

Comment thread pkgs/top-level/stage.nix Outdated
@infinisil infinisil force-pushed the attrPathsToRemove branch 3 times, most recently from 22240a2 to 6f96bad Compare June 23, 2026 16:43
@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 6.topic: stdenv Standard environment 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions labels Jun 23, 2026
@infinisil infinisil force-pushed the attrPathsToRemove branch from 6f96bad to 8311277 Compare June 23, 2026 17:56
nixpkgs-branch-check[bot]

This comment was marked as outdated.

@github-project-automation github-project-automation Bot moved this to In Progress in Stdenv Jun 23, 2026
@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should target the staging branches. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jun 23, 2026
@llakala

llakala commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Got it working, but as I feared, this heavily regresses CI performance.

metric mean_before mean_after mean_diff mean_pct_change
time.cpu 21.9585 26.5902 4.6317 21.0929
time.gc 0.7130 0.7270 0.0140 1.9635
time.gcFraction 0.0325 0.0273 -0.0051 -15.7973
gc.heapSize 2315517952.0000 2483290112.0000 167772160.0000 7.2456
gc.totalBytes 3876721616.0000 4064645232.0000 187923616.0000 4.8475
envs.bytes 518874696.0000 520299576.0000 1424880.0000 0.2746
list.bytes 70970848.0000 71028248.0000 57400.0000 0.0809
sets.bytes 1766385560.0000 1951271904.0000 184886344.0000 10.4669
symbols.bytes 1749483.0000 1749703.0000 220.0000 0.0126
values.bytes 794884544.0000 796325552.0000 1441008.0000 0.1813
envs.number 23885504.0000 23971038.0000 85534.0000 0.3581
nrAvoided 34043234.0000 34114190.0000 70956.0000 0.2084
nrExprs 2151194.0000 2151376.0000 182.0000 0.0085
nrFunctionCalls 21034057.0000 21112973.0000 78916.0000 0.3752
nrLookups 10435028.0000 10507675.0000 72647.0000 0.6962
nrOpUpdateValuesCopied 55417199.0000 62834017.0000 7416818.0000 13.3836
nrOpUpdates 4846420.0000 4862352.0000 15932.0000 0.3287
nrPrimOpCalls 12974468.0000 13013902.0000 39434.0000 0.3039
nrThunks 31255864.0000 31339890.0000 84026.0000 0.2688
sets.number 7554315.0000 7563218.0000 8903.0000 0.1179
symbols.number 126350.0000 126358.0000 8.0000 0.0063
values.number 49680284.0000 49770347.0000 90063.0000 0.1813
envs.elements 40973833.0000 41066409.0000 92576.0000 0.2259
list.concats 1761698.0000 1761705.0000 7.0000 0.0004
list.elements 8871356.0000 8878531.0000 7175.0000 0.0809
sets.elements 99067625.0000 110609667.0000 11542042.0000 11.6507

Feel free to reproduce. Tested by:

  1. checking out 1fbad9f3afcd (the commit this PR is based on)
  2. running nix-build ci -A eval.baseline --arg evalSystems '[ "x86_64-linux" ]' --arg quickTest true --out-link baseline
  3. Checking out the head commit of the PR
  4. Applying this diff to add a broken package:
diff --git a/pkgs/applications/misc/cubocore-packages/coregarage/default.nix b/pkgs/applications/misc/cubocore-packages/coregarage/default.nix
index 74b5bbd5a7cf..7d7a3d94dc2a 100644
--- a/pkgs/applications/misc/cubocore-packages/coregarage/default.nix
+++ b/pkgs/applications/misc/cubocore-packages/coregarage/default.nix
@@ -39,6 +39,7 @@ stdenv.mkDerivation (finalAttrs: {
   meta = {
     description = "Settings manager for the C Suite";
     mainProgram = "coregarage";
+    problems.removal.message = "some message";
     homepage = "https://gitlab.com/cubocore/coreapps/coregarage";
     license = lib.licenses.gpl3Plus;
     maintainers = [ ];
  1. Running nix-build ci -A eval.full --arg evalSystems '[ "x86_64-linux" ]' --arg quickTest true --arg baseline ./baseline --out-link compare
  2. Reading ./compare/step-summary.md

I'd like to try to lower this regression, as CI runs on every single PR.

@MattSturgeon

Copy link
Copy Markdown
Contributor

Thanks for testing!

I'd like to try to lower this regression, as CI runs on every single PR.

So long as total CI time (prepare → unlock) is in the same ballpark (minutes), I don't see eval being slower as a hard blocker.

Reducing the performance hit would be nice, of course, but it could be done in a follow-up if needed.

@MattSturgeon

Copy link
Copy Markdown
Contributor

So long as total CI time (prepare → unlock) is in the same ballpark (minutes), I don't see eval being slower as a hard blocker.

Looks like CI's eval is actually what determins the overall CI run time.

If we assume1 your observed 20% increase in CPU time is representative and applies directly to eval job runtime, it would add around 30s to the current ~3m job.

Slightly concerned that the "test" CI didn't run, as there are definitely CI changes that need testing here. Have you tested the CI changes in a fork?

It didn't run because I didn't touch any of the paths it triggers for, so it's not needed because the interface used by CI didn't change at all, which is nix-build ci -A eval.singleSystem, which I did test ;)

Surprisingly, we actually use the untrusted (PR branch's) CI implementation to orchestrate the eval:

nix-build nixpkgs/untrusted/ci --arg nixpkgs ./nixpkgs/untrusted-pinned -A eval.singleSystem \

So, that should mean that this PR's eval run is already using this PR's changes.

Looking at this PR's latest run, it's roughly the same overall job times as other recent PRs (example run). Both those runs have 3m20s longest eval job, and 5m40s total workflow run; no observable difference.2


I'm minded to merge this and see whether workflow/job time changes and how effectively it handles meta.problems in practice. Of course optimizations would be welcome, but they may be easier to review as dedicated PRs.

If we do observe significant slowdown, we can always revert. If we observe a small slowdown, I personally won't be too concerned.

Footnotes

  1. big assumption, there's lots of moving parts

  2. conflicting with the 20% CPU time increase you observed

@llakala llakala left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good.

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 26, 2026
@MattSturgeon MattSturgeon added this pull request to the merge queue Jun 26, 2026
Merged via the queue into NixOS:master with commit 288d25f Jun 26, 2026
31 of 33 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Stdenv Jun 26, 2026
@nixpkgs-ci

This comment was marked as resolved.

@nixpkgs-ci

This comment was marked as resolved.

@MattSturgeon

MattSturgeon commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Backporting #519600 resolves the 26.05 conflicts, but 25.11 does not have meta.problems.

For 25.11, I think we can apply 7cc4d7d as a stub config.attrPathsDisallowedForInternalUse option that has no effect, then we can backport the ci/ tree changes without backporting the other pkgs/ changes.

@MattSturgeon

Copy link
Copy Markdown
Contributor

I'm minded to merge this and see whether workflow/job time changes and how effectively it handles meta.problems in practice.

Early performance seems reasonable.

I haven't looked for PR runs where the base branch definitely includes this PR, but just selecting a few recent runs' total times at random:

* This run definitely used this PR

@MattSturgeon MattSturgeon added backport release-26.05 Backport PR automatically and removed backport release-25.11 backport release-26.05 Backport PR automatically labels Jun 26, 2026
@nixpkgs-ci

nixpkgs-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Successfully created backport PR for release-26.05:

@infinisil

infinisil commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

Follow-up to allow internally disallowed packages to depend on other such ones: #537051

@MattSturgeon

MattSturgeon commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Looking at https://github.com/NixOS/nixpkgs/actions/runs/28551752960/job/84650353939#step:7:443?pr=522364 the abort shows up e.g. when inheriting a version or other similar attribute.

We probably want more of an "abort on instantiate" behaviour, e.g. a mapAttrs where only attrs like outPath become aborts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: stdenv Standard environment 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. backport release-26.05 Backport PR automatically

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Setting meta.problems cause eval CI to fail!

3 participants