Improve lazy trees backward compatibility#56
Merged
Conversation
When you apply `builtins.toString` to a path value representing a path in the Nix store (as is the case with flake inputs), historically you got a string without context (e.g. `/nix/store/...-source`). This is broken, since it allows you to pass a store path to a derivation/toFile without a proper store reference. This is especially a problem with lazy trees, since the store path is a virtual path that doesn't exist and can be different every time. For backwards compatibility, and to warn users about this unsafe use of `toString`, we now keep track of such strings as a special type of context.
This is already done by consumers of builtins.toJSON (like builtins.toFile or builtins.derivation), so we can delay this until it's actually needed.
grahamc
reviewed
May 8, 2025
grahamc
reviewed
May 8, 2025
Co-authored-by: Graham Christensen <graham@grahamc.com>
edolstra
added
the
flake-regression-test
grahamc
approved these changes
May 8, 2025
The warning added seems to also trigger when intentionally removing context using * and anywhere else the usual NixOS options-doc pattern is used |
2 tasks
|
@grahamc @edolstra please see the comment above, related to nix-community/home-manager#7935 and NixOS/nixpkgs#482766 |
friedenberg
added a commit
to amarbel-llc/eng
that referenced
this pull request
May 1, 2026
Make the home-manager default (`manual.manpages.enable = true`) explicit so the warning seen during `just build-home` has a documented anchor. The warning fires inside nixpkgs' `make-options-doc/default.nix`, where `optionsJSON` does `builtins.unsafeDiscardStringContext (builtins.toJSON optionsNix)` to keep the manual's closure from dragging in every nixpkgs module file referenced by `options.<...>.declarations`. `disallowedReferences` is the actual correctness guard; the warning is cosmetic and Determinate-Nix-specific (added in DeterminateSystems/nix-src#56; upstream Nix does not emit it). Tracking upstream: - NixOS/nixpkgs#485682 - nix-community/home-manager#7935 - NixOS/nix#14011 If/when this becomes unacceptable, flip the line to `false` (loses `man home-configuration.nix` but eliminates the trigger end-to-end). :clown: https://github.com/amarbel-llc/clown
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
4 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.
Motivation
When you apply
builtins.toStringto a path value representing a path in the Nix store (as is the case with flake inputs), historically you got a string without context (e.g./nix/store/...-source). This is broken, since it allows you to pass a store path to a derivation/toFile without a proper store reference. This is especially a problem with lazy trees, since the store path is a virtual path that doesn't exist and can be different every time.For backwards compatibility, and to warn users about this unsafe use of
toString, we now keep track of such strings as a special type of context.(The original lazy trees branch had something similar, but there the context was keeping track of
SourcePathsrather than virtualStorePaths.)This makes Nix with
--lazy-treespass the entire flake-regressions test suite.Context