When the common pattern for store config constructors is to accept
an arbitrary string it's unclear whether it needs pct-decoding or not.
Prior to c436b7a32a the string passed to
the constructors was a mix of encoded authority and decoded path concatenated
with a `/`. After that commit it accidentally started accepting pct-encoded
result of renderAuthorityAndPath, but only in some code paths. This lead to
file:///tmp/a+b to be created on disk in /tmp/a%2Bb directory. Similar issue
affected the less-known variant with local:///tmp/a+b. Regular store references
that are local paths were not affected.
This patch changes the constructors to accept different types to signify what
is actually needed to let the factory method handle this in a consistent way:
- std::filesystem::path - local binary cache store and local store
- ParsedURL::Authority - for ssh stores
- ParsedURL - for http stores
(Some MinGW build fixes by Amaan Qureshi <git@amaanq.com>)
- Convert `no-url-literals` from experimental feature to `Diagnose`
setting
Replace `Xp::NoUrlLiterals` with a new `lint-url-literals` setting
that accepts `ignore`, `warn`, or `fatal`. This provides more
flexibility than the binary experimental feature.
- Convert `warn-short-path-literals` to use new lint infra
We now have `lint-short-path-literals = ignore | warn | fatal`
instead.
- Convert some of the tests to lang tests
Fix#10048
Progress on #10281
This takes the `std::filesystem::path` migration from the CLI layer into the core libraries (libutil, libstore, libexpr, libfetchers, libflake), converting function signatures, settings fields, and locals throughout `file-system.hh`, `archive.hh`, `configuration.hh`, and their implementations. `PathSetting` becomes `Setting<std::filesystem::path>` in `local-overlay-store.hh` and `local-store.hh` with `.get()` calls at use sites, and several `writeFile` overloads collapse now that the `Path`-based wrappers in `file-system.hh` are gone.
Co-authored-by: Amaan Qureshi <git@amaanq.com>
This commit reverts to using `getFSSourceAccessor()` so absolute symlink targets resolve correctly, since `makeFSSourceAccessor(path)` roots the accessor at `path` and can't follow symlinks that escape it.
In the multithreaded evaluator, it's possible for multiple threads to
wait on the same thunk. If evaluation of the thunk results in an
exception, the waiting threads shouldn't try to re-force the thunk.
Instead, they should rethrow the same exception, without duplicating
any work.
Therefore, there is now a new value type `tFailed` that stores an
std::exception_ptr. If evaluation of a thunk/app results in an
exception, `forceValue()` overwrites the value with a `tFailed`. If
`forceValue()` encounters a `tFailed`, it rethrows the exception. So
you normally never need to check for failed values (since forcing them
causes a rethrow).
Co-authored-by: Robert Hensing <robert@roberthensing.nl>
This reverts commit 4f91e9599f.
This broke:
when I applied this pr, `--print-out-paths` wouldn't print anything:
```
shell-for-nix-env % nix build --print-out-paths .#legacyPackages.aarch64-darwin.homeConfigurations.macos.activationPackage
```
After dropping the patches from my fork, it does again.
```
shell-for-nix-env % nix build --print-out-paths .#legacyPackages.aarch64-darwin.homeConfigurations.macos.activationPackage
/nix/store/s8mlcalszdml0v8172w4hwqnx0m6477r-home-manager-generation
```
When `max-jobs = 0` and no remote builders are available, Nix reported
"required system or feature not available" even though the system and
features matched fine. The `canBuildLocally` lambda returned a plain
`bool`, conflating a configuration knob (`max-jobs = 0`) with actual
incompatibility (wrong platform, missing features). It also short-circuited
on the first failing check, so a user with both a platform mismatch and
missing features would only see one of the two.
This commit replaces the bool with a `LocalBuildRejection` struct whose
`WrongLocalStore` variant collects all applicable failures into
`badPlatform`, `missingFeatures`, and an orthogonal `maxJobsZero` flag.
Platform mismatch and missing features now produce separate error
paragraphs, and all applicable reasons appear in a single message.
The local-build capability check also now returns
`std::variant<LocalBuildCapability, LocalBuildRejection>`, bundling
the `LocalStore &` and optional `ExternalBuilder *` together.
The external-builders test expands `$PATH` into a heredoc without quotes,
so any `PATH` entry containing spaces causes bash to parse the line as a
command instead of an assignment, failing the test.
The NIX_BUILD_TOP test used regex matching with an unquoted path
variable. When the path contains `+` (or other regex operators),
the test fails because `+` is interpreted as a quantifier rather than
a literal character. Glob matching handles these characters correctly.
`DirectoryIterator` is dereferenced without an end check, which segfaults
when unpacking an empty or zero-sized archive. This commit adds an
emptiness check before the dereference in both `prefetchFile` and
`builtinUnpackChannel`, throwing a descriptive error instead.
Fixes#15116.
Fix#11897
As described in the issue, this makes for a simpler and much more
intuitive notion of a realisation key. This is better for pedagogy, and
interoperability between more tools.
The way the issue was written was that we would switch to only having
shallow realisations first, and then do this. But going to only shallow
realisations is more complex change, and it turns out we weren't even
testing for the benefits that derivation hashes (modulo FODs) provided
in the deep realisation case, so I now just want to do this first.
Doing this gets the binary cache data structures in order, which will
unblock the Hydra fixed-output-derivation tracking work. I don't want to
delay that work while I figure out the changes needed for
shallow-realisations only.
This reverts commit bab1cda0e6.
Co-authored-by: Amaan Qureshi <git@amaanq.com>
Fixes abort on :ll if nothing has been loaded yet. Also gets rid of
redundant openStore() calls that were dead code (store can be extracted
from EvalState already) and arguably openStore is a layer violation.
Also catches EPIPE in case the pager gets interrupted to avoid superfluous
error messages.
This test insisted on placing profiles in NIX_STATE_DIR, but all
packages were removed from the profile immediately after so they did not
act as garbage collector roots. Switch to directly calling nix-build,
allowing the test to run in VMs without NIX_STATE_DIR.
Due to a typo in quoteRegexChars, finding runtime garbage collection roots
was failing on paths that contained a dot, or any other regex chars that would
have to be replaced.
When fixing that error, also add tests to make sure gc continues to
work.
Fixes#15053
This adds support for inherit statements and multiple bindings per
line in nix repl:
nix-repl> a = { x = 1; y = 2; }
nix-repl> inherit (a) x y
nix-repl> x + y
3
nix-repl> p = 1; q = 2;
nix-repl> p + q
3
Design
------
The REPL now uses a parser-based approach instead of ad-hoc regex
matching. It tries parsing input in order:
1. As an expression (using the regular parser entry point)
2. As bindings (using a new REPL_BINDINGS entry point)
3. As bindings with an appended semicolon (for convenience)
The third attempt allows users to omit the trailing semicolon for
single statements like "inherit (a) x" or "foo = 1".
A separate REPL_BINDINGS parser entry point is required because
combining expressions and bindings in the same production causes
reduce/reduce conflicts: "ID ." is ambiguous between attribute access
(a.b) and the start of a nested binding path (a.b = 1).
Alternatives considered
-----------------------
GLR parsing: Would resolve the ambiguity but risks parsing
performance regression for all Nix code, not just the REPL.
Duplicated grammar: A REPL-specific repl_binds production that
duplicates binds1 rules. Rejected to avoid maintenance burden and
grammar drift.
Single binding per line: Simpler but less useful. The current
approach reuses binds1 directly, getting multiple bindings and
nested attribute paths for free.
Tradeoffs
---------
Binding input may be parsed twice (once as expression, then as
bindings). This is acceptable for interactive REPL use where input is
typically small and latency from parsing is negligible.
Error messages preserve the original input even when the semicolon-
appended parse is attempted, so users see "inherit (a) y" in errors
rather than "inherit (a) y;".
Currently, tests fail when the host system has `commit.gpgsign` or
`tag.gpgsign` enabled at the system level (in my case, a custom path
located at `/etc/git/config`), since the signing key is unavailable in
the test sandbox.
The tests set `HOME=$TEST_HOME` to isolate themselves, which bypasses
the user-level git config (`~/.gitconfig`). However, if a user sets the
system-level config via `GIT_CONFIG_GLOBAL` or `GIT_CONFIG_SYSTEM`, it
still applies, causing commits to fail when signing is enabled there.
In this PR, I explicitly set `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`
to `/dev/null` so that the user's system config is never read from or
written to. I've also replaced `git config --global protocol.file.allow
always` with `GIT_CONFIG_*` environment variables to avoid writing to
`/dev/null`.
The test was checking for `$stdenv` but the `fixed` derivation doesn't
actually have stdenv, it just has `FOO`. I've updated it to check the
value of `FOO` instead.
The previous implementation double-quoted the _args variable by escaping
each argument individually and then wrapping them all in single quotes,
producing output like: _args=''-e' 'arg1' 'arg2''
This fix concatenates all arguments into a single string first, then
escapes that string once, producing correct output like:
_args='-e arg1 arg2'
This prevents potential command injection issues when the output is
sourced in shell scripts.
Fixes#14327
Don't add the whole store to sandbox-paths unconditionally. Exposing
the entire store defeats the purpose of sandboxing, and when the test
store is the same as the system store (NixOS VM), it causes an obscure
"Permission denied" error.
Only add sandbox-paths when not on NixOS, indicating a separate test
store that needs access to system store build tools.
This will once and for all get rid of all double-quoting issues. On windows the quoting
is doubly bad because it escaped all \ to \\, which is very bad for error messages. In
order to prevent future regression std::filesystem::path formatting now must use a special
type PathFmt (like Magenta). In the future we could even change how we render filesystem paths.
Change "cannot build missing derivation" to "failed to obtain derivation of"
since the path (e.g. '...drv^out') is a derivation output, not a derivation.
The message could be improved further to resolve ambiguity when multiple
outputOf links are involved, but for now we err on the side of brevity
since this message is already merged into larger error messages with
other context from the Worker and CLI.
When !keepGoing and a goal fails, other goals are cancelled and
remain with exitCode == ecBusy. These cancelled goals have a default
BuildResult::Failure{} with empty errorMsg.
Previously, buildPathsWithResults would return these cancelled goals,
and throwBuildErrors would report them as failures. When only one such
cancelled goal was present, it would throw an error with an empty
message like:
error: build of '/nix/store/...drv^*' failed:
Now we skip goals with ecBusy since their state is indeterminate.
Cancelled goals could be reported, but this keeps the output relevant.
Other indeterminate goal states were already not being reported, for
instance: derivations that weren't started for being blocked on a
concurrency limit, or blocked on a currently building dependency.
When keepGoing=false and a build fails, other goals are cancelled.
Previously, these cancelled goals were reported in the "build of ...
failed" error message alongside actual failures. This was misleading
since cancelled goals didn't actually fail - they were never tried.
Update the test to expect only the actual failure (hash mismatch) to
be reported, not the cancelled goals.
DerivationTrampolineGoal is the top-level goal whose buildResult is
returned by buildPathsWithResults. When it failed without setting
buildResult.inner, buildPathsWithResults would return failures with
empty errorMsg, producing error messages like:
error: failed to build attribute 'checks.x86_64-linux.foo',
build of '/nix/store/...drv^*' failed:
(note the empty message after "failed:")
Use the new doneFailure helper to ensure buildResult is populated
with meaningful error information.
Relative paths like `file:./foo.tar.gz` have never worked for tarballs
because curl rejects relative file: URLs. Previously this resulted in
cryptic curl errors. Now we reject them early with a clear message
explaining that relative paths are not supported because there is no
defined base directory to resolve them against.
See https://github.com/NixOS/nix/issues/12281
