feat: add always-allow-substitutes #8047

lovesegfault · 2023-03-14T14:59:46Z

Motivation

This adds a new configuration option to Nix, always-allow-substitutes,
whose effect is simple: it causes the allowSubstitutes attribute in
derivations to be ignored, and for substituters to always be used.

This option should be a good middle-ground, since it doesn't imply
rebuilding the world, such as the approach I took in

pkgs/top-level/config: add alwaysAllowSubstitutes nixpkgs#221048

Context

This is extremely valuable for users of Nix in CI, where usually
nix-build-uncached is used. There, derivations which disallow
substitutes cause headaches as the inputs for building already-cached
derivations need to be fetched to spuriously rebuild some simple text
file.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

agreed on idea
agreed on implementation strategy
tests, as appropriate
- functional tests - tests/**.sh
- unit tests - src/*/tests
- integration tests - tests/nixos/*
documentation in the manual
code and comments are self-explanatory
commit message explains why the change was made
new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

c.f. NixOS/nix#8047

lovesegfault · 2023-03-14T17:34:14Z

You can see this working here: https://github.com/lovesegfault/nix-config/actions/runs/4418436516/jobs/7745815829

Everything had already been cached by a previous run of the CI, and the subsequent run was essentially a no-op since nix-build-uncached had nothing to do.

lovesegfault · 2023-03-16T16:37:14Z

cc. @edolstra

fricklerhandwerk · 2023-03-17T03:48:41Z

@lovesegfault The maintainers team will triage this PR in one of the next meetings. Please be patient, we have a lot of open PRs to deal with, and many of them take more than one session to even decide over if they're reasonable to begin with.

lovesegfault · 2023-04-05T17:24:28Z

@fricklerhandwerk Follow-up on this? It's such a simple PR and brings huge wins for users of Nix in CI.

fricklerhandwerk · 2023-04-05T18:10:29Z

Still in the triage backlog, I suppose the team will take a look on Friday. Note that new features are currently fairly low in priority, due to many design decisions that still have to be made.

lovesegfault · 2023-04-19T12:22:52Z

Just an update, I've been running this patch on all my hosts for ~1 month without issues.

@fricklerhandwerk I assume it still hasn't been triaged?

fricklerhandwerk · 2023-04-24T05:48:30Z

Yes, sorry, we have a huge backlog, and the next two weeks will be slow due to days off.

fricklerhandwerk · 2023-04-28T17:00:53Z

Triaged in Nix team meeting:

We're not sure if this is the right solution to the underlying problem. Maybe we should instead remove allowSubstitutes altogether, or at least benchmark for the actual performance bottlenecks to determine if the feature is needed at all or when it's appropriate to use it. This needs more discussion.

Complete discussion

@edolstra: raises the question if we still want allowSubstitutes
- it was a performance hack for Hydra
- maybe we should benchmark the alternatives
  - a typical case would be NixOS closure builds with lots of config files where building is faster than substituting
  - if the cache lookup is only a very small overhead, the allowSubstitues feature may not be all that useful
@thufschmitt: it would make more of a difference with CA derivations
@edolstra: in case where trivial derivations have huge dependency closures it may be better not to set allowSubstitutes = false
- inclined to remove allowSubstitutes altogether
- @fricklerhandwerk: it seems to be a widely used feature though, even if apparently sometimes for the wrong reasons
  - @edolstra: that's actually an argument for removing it, as there is no clear criterion for using it
    - especially in Nixpkgs maintainers will make policy decisions which are not necessarily appropriate for all users
@thufschmitt: the radical alternative would be to push for CA derivations everywhere, because then a global configuration option for Nixpkgs would not break all the derivation hashes
needs more discussion

nixos-discourse · 2023-04-28T17:02:47Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-04-28-nix-team-meeting-minutes-50/27698/1

lovesegfault · 2023-04-28T17:21:46Z

My argument is that what is proposed here can improve the experience for users right now, without representing meaningful future risk. Adding this option does not restrict the solution space here, we can still choose to remove allowSubstitutes altogether in the future.

It's hard for me to see how it's better to do nothing now than to allow people to opt-out of a “broken” feature that might get removed in the future, especially given the simplicity of the patch here.

I want to emphasize how much better the experience of Nix in CI is with this, and it's a use case that's really common.

I would really appreciate if there was bias for action here, given that this is a non-invasive immediate solution to a real issue for a mainstream use-case of Nix.

cc. @domenkozar @Mic92 @edolstra

lovesegfault · 2023-04-28T17:42:33Z

A follow-up thought, this could even enable us to just flip alwaysAllowSubstitutes to true by default eventually, and then have a transition period before getting rid of allowSubstitutes altogether.

domenkozar · 2023-04-28T18:04:08Z

I fully support merging this and using it as a stepping stone towards the removal of this feature.

This adds a new configuration option to Nix, `always-allow-substitutes`, whose effect is simple: it causes the `allowSubstitutes` attribute in derivations to be ignored, and for substituters to always be used. This is extremely valuable for users of Nix in CI, where usually `nix-build-uncached` is used. There, derivations which disallow substitutes cause headaches as the inputs for building already-cached derivations need to be fetched to spuriously rebuild some simple text file. This option should be a good middle-ground, since it doesn't imply rebuilding the world, such as the approach I took in NixOS/nixpkgs#221048

domenkozar · 2023-07-18T09:42:50Z

A good example for is when local system and target systems don't match. Nix currently insists on building and thus fails.

lovesegfault · 2023-07-22T15:42:40Z

@fricklerhandwerk Is there a way to request this to be reconsidered given the comments since it was last discussed?

I'd be happy to show up to the next team meeting and talk about this PR too, though I'm not sure when/where it will happen, or whether it's open to folks popping in :)

SuperSandro2000 · 2023-07-22T16:36:54Z

I've been using this for a few days and substituting things like steam-fhs takes a long time despite the package itself only being symlinks.

thufschmitt · 2023-07-31T08:33:06Z

Discussed during the Nix team meeting: This isn't a great long-term solution (adding a layer to fix a broken layer), but it's a good-enough stop-gap. Agreement to accept this, with an explicit mention that it's a stop-gap and not a viable long-term solution

nixos-discourse · 2023-07-31T08:43:49Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-07-24-nix-team-meeting-minutes-74/31116/1

lovesegfault · 2023-08-11T14:20:46Z

Pinging @roberth for review :)

lovesegfault · 2023-10-09T18:24:54Z

Pinging @roberth again

roberth · 2023-10-13T13:29:17Z

A good example for is when local system and target systems don't match. Nix currently insists on building and thus fails.

I think you mean when the derivation system is not buildable locally. We don't have a definition for target, but that's ok.
That's a real proper bug (not just performance) and this PR would only provide a workaround, not a solution.

roberth · 2023-10-13T13:41:03Z

src/libstore/parsed-derivations.cc

@@ -122,7 +122,7 @@ bool ParsedDerivation::willBuildLocally(Store & localStore) const

 bool ParsedDerivation::substitutesAllowed() const
 {
-    return getBoolAttr("allowSubstitutes", true);
+    return settings.alwaysAllowSubstitutes ? true : getBoolAttr("allowSubstitutes", true);


This is really not a property of a "parsed derivation", but rather a property of the desired scheduling approach.
Not happy about that, but it's a structural problem, not a new thing.

feat: add always-allow-substitutes (cherry picked from commit da2b59a) Change-Id: I50481cd8fe643c673c610fec28bad84519a4d650

nixos-discourse · 2025-02-21T19:42:31Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-build-not-downloading-everything-from-cache/60680/4

lovesegfault requested review from thufschmitt and fricklerhandwerk as code owners March 14, 2023 14:59

lovesegfault added a commit to lovesegfault/nix-config that referenced this pull request Mar 14, 2023

ci: try always-allow-substitutes patched nix

3e77055

c.f. NixOS/nix#8047

fricklerhandwerk added the feature Feature request or proposal label Mar 17, 2023

fricklerhandwerk added the performance label Apr 5, 2023

lovesegfault force-pushed the always-allow-substitutes branch from 1ad8638 to bf69331 Compare May 22, 2023 18:42

thufschmitt assigned roberth Jul 31, 2023

roberth reviewed Oct 13, 2023

View reviewed changes

roberth approved these changes Oct 13, 2023

View reviewed changes

roberth merged commit da2b59a into NixOS:master Oct 13, 2023

lovesegfault deleted the always-allow-substitutes branch October 13, 2023 13:42

lovesegfault mentioned this pull request Oct 13, 2023

allowSubstitutes is an anti-feature #4442

Closed

lovesegfault mentioned this pull request Nov 21, 2023

2.19 Release Notes missing references to always-allow-substitutes #9427

Closed

2 tasks

fricklerhandwerk mentioned this pull request Dec 2, 2023

Allow overriding allowSubstitutes without modification to derivation #3686

Closed

lucperkins mentioned this pull request May 27, 2024

Set always-allow-substitutes to true in nix.conf DeterminateSystems/nix-installer#977

Merged

tebowy pushed a commit to tebowy/nix that referenced this pull request Jul 11, 2024

Merge pull request NixOS#8047 from lovesegfault/always-allow-substitutes

aeb803d

feat: add always-allow-substitutes (cherry picked from commit da2b59a) Change-Id: I50481cd8fe643c673c610fec28bad84519a4d650

marienz mentioned this pull request Oct 12, 2024

can not evaluate arm configurations on x86 marienz/nix-doom-emacs-unstraightened#32

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add always-allow-substitutes #8047

feat: add always-allow-substitutes #8047

lovesegfault commented Mar 14, 2023 •

edited by roberth

Loading

lovesegfault commented Mar 14, 2023

lovesegfault commented Mar 16, 2023

fricklerhandwerk commented Mar 17, 2023 •

edited

Loading

lovesegfault commented Apr 5, 2023

fricklerhandwerk commented Apr 5, 2023

lovesegfault commented Apr 19, 2023

fricklerhandwerk commented Apr 24, 2023

fricklerhandwerk commented Apr 28, 2023 •

edited

Loading

nixos-discourse commented Apr 28, 2023

lovesegfault commented Apr 28, 2023

lovesegfault commented Apr 28, 2023

domenkozar commented Apr 28, 2023

domenkozar commented Jul 18, 2023 •

edited

Loading

lovesegfault commented Jul 22, 2023

SuperSandro2000 commented Jul 22, 2023

thufschmitt commented Jul 31, 2023

nixos-discourse commented Jul 31, 2023

lovesegfault commented Aug 11, 2023

lovesegfault commented Oct 9, 2023

roberth commented Oct 13, 2023

roberth Oct 13, 2023

nixos-discourse commented Feb 21, 2025

feat: add always-allow-substitutes #8047

feat: add always-allow-substitutes #8047

Conversation

lovesegfault commented Mar 14, 2023 • edited by roberth Loading

Motivation

Context

Checklist for maintainers

Priorities

lovesegfault commented Mar 14, 2023

lovesegfault commented Mar 16, 2023

fricklerhandwerk commented Mar 17, 2023 • edited Loading

lovesegfault commented Apr 5, 2023

fricklerhandwerk commented Apr 5, 2023

lovesegfault commented Apr 19, 2023

fricklerhandwerk commented Apr 24, 2023

fricklerhandwerk commented Apr 28, 2023 • edited Loading

nixos-discourse commented Apr 28, 2023

lovesegfault commented Apr 28, 2023

lovesegfault commented Apr 28, 2023

domenkozar commented Apr 28, 2023

domenkozar commented Jul 18, 2023 • edited Loading

lovesegfault commented Jul 22, 2023

SuperSandro2000 commented Jul 22, 2023

thufschmitt commented Jul 31, 2023

nixos-discourse commented Jul 31, 2023

lovesegfault commented Aug 11, 2023

lovesegfault commented Oct 9, 2023

roberth commented Oct 13, 2023

roberth Oct 13, 2023

Choose a reason for hiding this comment

nixos-discourse commented Feb 21, 2025

lovesegfault commented Mar 14, 2023 •

edited by roberth

Loading

fricklerhandwerk commented Mar 17, 2023 •

edited

Loading

fricklerhandwerk commented Apr 28, 2023 •

edited

Loading

domenkozar commented Jul 18, 2023 •

edited

Loading