aboutsummaryrefslogtreecommitdiff
path: root/src/libexpr
AgeCommit message (Collapse)Author
2024-08-07libexpr/parser: Test experimental featurespiegames
Currently, the parser relies on the global experimental feature flags. In order to properly test conditional language features, we instead need to pass it around in the parser::State. This means that the parser cannot cache the result of isEnabled anymore, which wouldn't necessarily hurt performance if the function didn't perform a linear search on the list of enabled features on every single call. While we could simply evaluate once at the start of parsing and cache the result in the parser state, the more sustainable solution would be to fix `isEnabled` such that all callers may profit from the performance improvement. Change-Id: Ic9b9c5d882b6270e1114988b63e6064d36c25cf2
2024-08-07Merge "fix: warn and document when advanced attributes will have no impact ↵Maximilian Bosch
due to __structuredAttrs" into main
2024-08-07Merge "package: improve support for building without BDW-GC" into mainalois31
2024-08-04tree-wide: fix various lint warningsJade Lovelace
Change-Id: I0fc80718eb7e02d84cc4b5d5deec4c0f41116134
2024-08-03fix: warn and document when advanced attributes will have no impact due to ↵Tom Bereknyei
__structuredAttrs Backport of https://github.com/NixOS/nix/pull/10884. Change-Id: I82cc2794730ae9f4a9b7df0185ed0aea83efb65a
2024-08-03package: improve support for building without BDW-GCAlois Wohlschlager
Expose an option for disabling the BDW-GC build dependency entirely. Fix the place where one of its headers was included (unnecessarily) without proper guarding. Finally, use this machinery to exclude BDW-GC from the ASAN builds entirely (its usage has already been disabled due to compatibility issues anyway), to ensure this configuration is not regressed again. Change-Id: I2ebe8094abf67e7d1e99eed971de3e99d071c10b
2024-08-02Merge "Reapply "libfetchers: make attribute / URL query handling ↵Maximilian Bosch
consistent"" into main
2024-08-01Reapply "libfetchers: make attribute / URL query handling consistent"Maximilian Bosch
The original attempt at this introduced a regression; this commit reverts the revert and fixes the regression. This reverts commit 3e151d4d77b5296b9da8c3ad209932d1dfa44c68. Fix to the regression: flakeref: fix handling of `?dir=` param for flakes in subdirs As reported in #419[1], accessing a flake in a subdir of a Git repository fails with the previous commit[2] applied with the error error: unsupported Git input attribute 'dir' The problem is that the `dir`-param is inserted into the parsed URL if a flake is fetched from the subdir of a Git repository. However, for the fetching part this isn't even needed. The fix is to just pass `subdir` as second argument to `FlakeRef` (which needs a `basedir` that can be empty) and leave the parsedURL as-is. Added a regression test to make sure we don't run into this again. [1] https://git.lix.systems/lix-project/lix/issues/419 [2] e22172aaf6b6a366cecd3c025590e68fa2b91bcc, originally 3e151d4d77b5296b9da8c3ad209932d1dfa44c68 Change-Id: I2c72d5a32e406a7ca308e271730bd0af01c5d18b
2024-08-01Merge changes Icc4747ae,Id4717b5b,Ie3ddb3d0,Ic4d93a08,I00d9ff70 into mainQyriad
* changes: remove unused headers in installable-attr-path libexpr: include the type of the non-derivation value in the type error libexpr: mild cleanup to getDerivations libexpr: DrvInfo: remove unused bad-citizen constructor cleanup and slightly refactor DrvInfo::queryOutputs
2024-08-01libexpr: include the type of the non-derivation value in the type errorQyriad
Change-Id: Id4717b5b0df7c09b0dbf17e642d8713a0a3efbae
2024-08-01libexpr: mild cleanup to getDerivationsQyriad
Shuffled the logic around a bit so the shorter code paths are early returns, added comments, etc. Should be NFC. Change-Id: Ie3ddb3d0eddd614d6f8c37bf9a4d5a50282084ea
2024-08-01libexpr: DrvInfo: remove unused bad-citizen constructorQyriad
DrvInfo's constructor that only takes `EvalState` leaves everything else empty; a DrvInfo which has no iota of information about the derivation it represents is not useful, and was not used anywhere. Change-Id: Ic4d93a08cb2748b8cef9a61e41e70404834b23f9
2024-08-01cleanup and slightly refactor DrvInfo::queryOutputsQyriad
Change-Id: I00d9ff707fe61995737b86af6d2eaa1e4d8116ff
2024-07-31libexpr: implement actual constructors for nix::ValueQyriad
Change-Id: Iebc2bb4e4ea5e93045afe47677df756de4ec4d05
2024-07-22Merge "libexpr/gc-alloc: fix compilation with !HAVE_BOEHMGC" into mainPierre Bourdon
2024-07-23libexpr/gc-alloc: fix compilation with !HAVE_BOEHMGCPierre Bourdon
Fixes: 72ee25b4025257fdaab7b8e8d5d1ccc83858fdab Change-Id: Ib59386af1415a8ed4b53af24ec22a4ffa5e5877d
2024-07-22enable -Werror=suggest-overrideeldritch horrors
*accidentally* overriding a function is almost guaranteed to be an error. overriding a function without labeling it as such is merely bad style, but bad style that makes the code harder to understand. Change-Id: Ic0594f3d1604ab6b3c1a75cb5facc246effe45f0
2024-07-22libexpr: fix -Wunused-const-variable warningeldritch horrors
Change-Id: Ib986ece0ab2eff83e7abd7f1f915cd8f761827ad
2024-07-20libexpr: add a strongly typed version of gcAllocBytes()Qyriad
This commit adds a new helper template function to gc-alloc.hh (which is probably where you want to look at first, O great reviewer [custom file ordering in review diffs when]), which uses a type argument to determine the size to allocate, rather than making the caller use sizeof(). Change-Id: Ib5d138d91a28bdda304a80db24ea9fb08669ad22
2024-07-20libexpr: rename confusing makeImmutableString -> gcCopyStringIfNeededQyriad
The purpose of this function has little to do with immutability. Value's strings are never mutated, and the point of this function is to singleton empty strings. Change-Id: Ifd41dd952409d54e4d3de9ab59064e6928b0e480
2024-07-20libexpr: refactor gc-agnostic helpers into one placeQyriad
Change-Id: Icc4b367e4f670d47256f62a3a002cd248a5c2d3b
2024-07-19Merge changes Id8b3d289,Ib75ab5b8,I3792eeb3 into mainjade
* changes: Fixup a bunch of references to nixos.org manuals Add release notes for removing overflow from Nix language expr: fix a compiler warning about different signs in comparison
2024-07-18Fixup a bunch of references to nixos.org manualsJade Lovelace
(plus one reference to CppNix github) Change-Id: Id8b3d2897f3b54e286861805cfd421adc4d5de47
2024-07-18expr: fix a compiler warning about different signs in comparisonJade Lovelace
We know that variable is >=0, so we can just cast it to unsigned. Change-Id: I3792eeb3ca43e6a507cc44c1a70584d42b2acd7b
2024-07-18libexpr/print: do not show elided nested items when there are noneAlois Wohlschlager
When the configured maximum depth has been reached, attribute sets and lists are printed with ellipsis to indicate the elision of nested items. Previously, this happened even in case the structure being printed is empty, so that such items do not in fact exist. This is confusing, so stop doing it. Change-Id: I0016970dad3e42625e085dc896e6f476b21226c9
2024-07-18libexpr/print: never show empty attrsets or derivations as «repeated»Alois Wohlschlager
The repeated value detection logic exists so that the occurrence of large common substructures does not fill up the screen or the computer's memory. However, empty attribute sets and derivations (when their detection is enabled) are always cheap to print, and in practice I have observed them to make up a significant majority of the cases where I was annoyed by the repeated value detection kicking in. Furthermore, `nix-instantiate --eval` already disables this logic for empty attribute sets, and empty lists are already exempted everywhere. For these reasons, always print empty attribute sets and derivations as what they are. Change-Id: I5dac8e7739f9d726b76fd0521ec46f38af94463f
2024-07-18libexpr/print: pretty-print idempotentlyAlois Wohlschlager
When pretty-printing is enabled, previously an unforced thunk would trigger indentation, even when it subsequently does not evaluate to a nested structure. The resulting output looked inconsistent, and furthermore pretty-printing was not idempotent (since pretty-printing the same value again, which is now fully evaluated, will not trigger indentation). When strict evaluation is enabled, force the item before inspecting its type, so that it is properly known whether it contains a nested structure. Furthermore, there is no need to cause indentation for unforced thunks, since the very next operation will be printing them as `«thunk»`. This is mostly a port of https://github.com/NixOS/nix/pull/11100 , but we only force the item when it's going to be forced anyway due to strict pretty-printing, and a new test was written since the REPL testing framework in Lix is different. Co-Authored-By: Robert Hensing <robert@roberthensing.nl> Change-Id: Ib7560fe531d09e05ca6b2037a523fe21a26d9d58
2024-07-16remove boost coroutine referenceseldritch horrors
we no longer need these since sinkToSource and sourceToSink are gone. Change-Id: Ibbf440e2cf71bf3e9f3b833af2d78a21fb1b3193
2024-07-16remove sourceToSink, sinkToSource, and boehm patcheldritch horrors
Change-Id: I1379841299713175d0225b82a67f50660f9eb5e2
2024-07-14libexpr/eval.cc: remove unnecessary C string conversionNaN
Change-Id: I5b7c21df84ff8ff64cf6a1e261fc3729a06bd4f6
2024-07-13language: cleanly ban integer overflowsJade Lovelace
This also bans various sneaking of negative numbers from the language into unsuspecting builtins as was exposed while auditing the consequences of changing the Nix language integer type to a newtype. It's unlikely that this change comprehensively ensures correctness when passing integers out of the Nix language and we should probably add a checked-narrowing function or something similar, but that's out of scope for the immediate change. During the development of this I found a few fun facts about the language: - You could overflow integers by converting from unsigned JSON values. - You could overflow unsigned integers by converting negative numbers into them when going into Nix config, into fetchTree, and into flake inputs. The flake inputs and Nix config cannot actually be tested properly since they both ban thunks, however, we put in checks anyway because it's possible these could somehow be used to do such shenanigans some other way. Note that Lix has banned Nix language integer overflows since the very first public beta, but threw a SIGILL about them because we run with -fsanitize=signed-overflow -fsanitize-undefined-trap-on-error in production builds. Since the Nix language uses signed integers, overflow was simply undefined behaviour, and since we defined that to trap, it did. Trapping on it was a bad UX, but we didn't even entirely notice that we had done this at all until it was reported as a bug a couple of months later (which is, to be fair, that flag working as intended), and it's got enough production time that, aside from code that is IMHO buggy (and which is, in any case, not in nixpkgs) such as https://git.lix.systems/lix-project/lix/issues/445, we don't think anyone doing anything reasonable actually depends on wrapping overflow. Even for weird use cases such as doing funny bit crimes, it doesn't make sense IMO to have wrapping behaviour, since two's complement arithmetic overflow behaviour is so *aggressively* not what you want for *any* kind of mathematics/algorithms. The Nix language exists for package management, a domain where bit crimes are already only dubiously in scope to begin with, and it makes a lot more sense for that domain for the integers to never lose precision, either by throwing errors if they would, or by being arbitrary-precision. This change will be ported to CppNix as well, to maintain language consistency. Fixes: https://git.lix.systems/lix-project/lix/issues/423 Change-Id: I51f253840c4af2ea5422b8a420aa5fafbf8fae75
2024-07-12Use std::strong_ordering for version comparisonJade Lovelace
The actual motive here is the avoidance of integer overflow if we were to make these use checked NixInts and retain the subtraction. However, the actual *intent* of this code is a three-way comparison, which can be done with operator<=>, so we should just do *that* instead. Change-Id: I7f9a7da1f3176424b528af6d1b4f1591e4ab26bf
2024-07-08libexpr: stop lying about DrvInfo's constnessQyriad
DrvInfo's query methods all use mutable fields to cache, but like. that's basically the entire interface for DrvInfo. Not only that, but these formerly-const-marked functions can even throw due to eval errors! Changing this only required removing some `const` markers in nix-env, and changing a single inline `queryInstalled()` call to be an lvalue instead. Change-Id: I796807118f3b35b0e93668b5e28210d9e521b2ae
2024-07-04mildly cleanup libexpr/eval.hhQyriad
Change-Id: I40d01a8f8b7fb101279c6f88ebdf1f0969d9d7f0
2024-07-04distinguish between throws & errors during throwQyriad
Turns errors like this: let throwMsg = a: throw (a + " invalid bar"); in throwMsg "bullshit" error: … from call site at «string»:3:4: 2| throwMsg = a: throw (a + " invalid bar"); 3| in throwMsg "bullshit" | ^ … while calling 'throwMsg' at «string»:2:14: 1| let 2| throwMsg = a: throw (a + " invalid bar"); | ^ 3| in throwMsg "bullshit" … while calling the 'throw' builtin at «string»:2:17: 1| let 2| throwMsg = a: throw (a + " invalid bar"); | ^ 3| in throwMsg "bullshit" error: bullshit invalid bar into errors like this: let throwMsg = a: throw (a + " invalid bar"); in throwMsg "bullshit" error: … from call site at «string»:3:4: 2| throwMsg = a: throw (a + " invalid bar"); 3| in throwMsg "bullshit" | ^ … while calling 'throwMsg' at «string»:2:14: 1| let 2| throwMsg = a: throw (a + " invalid bar"); | ^ 3| in throwMsg "bullshit" … caused by explicit throw at «string»:2:17: 1| let 2| throwMsg = a: throw (a + " invalid bar"); | ^ 3| in throwMsg "bullshit" error: bullshit invalid bar Change-Id: I593688928ece20f97999d1bf03b2b46d9ac338cb
2024-07-04trace which part of `foo.bar.baz` errorsQyriad
Turns errors like: let somepkg.src = throw "invalid foobar"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta … while calling the 'throw' builtin at «string»:2:17: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta error: invalid foobar into errors like: let somepkg.src = throw "invalid foobar"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta … while evaluating 'somepkg.src' to select 'meta' on it at «string»:3:4: 2| somepkg.src = throw "invalid foobar"; 3| in somepkg.src.meta | ^ … while calling the 'throw' builtin at «string»:2:17: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta error: invalid foobar And for type errors, from: let somepkg.src = "I'm not an attrset"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = "I'm not an attrset"; | ^ 3| in somepkg.src.meta … while selecting an attribute at «string»:3:4: 2| somepkg.src = "I'm not an attrset"; 3| in somepkg.src.meta | ^ error: expected a set but found a string: "I'm not an attrset" into: let somepkg.src = "I'm not an attrset"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = "I'm not an attrset"; | ^ 3| in somepkg.src.meta … while selecting 'meta' on 'somepkg.src' at «string»:3:4: 2| somepkg.src = "I'm not an attrset"; 3| in somepkg.src.meta | ^ error: expected a set but found a string: "I'm not an attrset" For the low price of an enumerate() and a lambda you too can have the incorrect line of code actually show up in the trace! Change-Id: Ic1491c86e33c167891bdac9adad6224784760bd6
2024-07-04trace when the `foo` part of `foo.bar.baz` errorsQyriad
Turns errors like: let errpkg = throw "invalid foobar"; in errpkg.meta error: … while calling the 'throw' builtin at «string»:2:12: 1| let 2| errpkg = throw "invalid foobar"; | ^ 3| in errpkg.meta error: invalid foobar into errors like: let errpkg = throw "invalid foobar"; in errpkg.meta error: … while evaluating 'errpkg' to select 'meta' on it at «string»:3:4: 2| errpkg = throw "invalid foobar"; 3| in errpkg.meta | ^ … while calling the 'throw' builtin at «string»:2:12: 1| let 2| errpkg = throw "invalid foobar"; | ^ 3| in errpkg.meta error: invalid foobar For the low price of one try/catch, you too can have the incorrect line of code actually show up in the trace! Change-Id: If8d6200ec1567706669d405c34adcd7e2d2cd29d
2024-07-04add an ExprPrinter class, like ValuePrinterQyriad
To be used Shortly Change-Id: I9def7975aa55f251eb8486391677771f7352d7ce
2024-07-04add an impl of Expr::show for ExprInheritFrom that doesn't crashQyriad
ExprVar::show() assumes it has a name. dynamic inherits do not necessarily (ever?) have a name. Change-Id: If10893188e307431da17f0c1bd0787adc74f7141
2024-07-04give ExprInheritFrom a handle to what its standing in forQyriad
Change-Id: I12088e0b618407e5432523bbc97be63c8d6fce62
2024-07-02Merge "Reject fully-qualified URLs in 'from' argument of `nix registry add`" ↵Delan Azabani
into main
2024-06-30libexpr/flake: allow automatic rejection of configuration options from flakesAlois Wohlschlager
The `allow-flake-configuration` option allows the user to control whether to accept configuration options supplied by flakes. Unfortunately, setting this to false really meant "ask each time" (with an option to remember the choice for each specific option encountered). Let no mean no, and introduce (and default to) a separate value for the "ask each time" behaviour. Co-Authored-By: Jade Lovelace <lix@jade.fyi> Change-Id: I7ccd67a95bfc92cffc1ebdc972d243f5191cc1b4
2024-06-29Reject fully-qualified URLs in 'from' argument of `nix registry add`Delan Azabani
We previously allowed you to map any flake URL to any other flake URL, including shorthand flakerefs, indirect flake URLs like `flake:nixpkgs`, direct flake URLs like `github:NixOS/nixpkgs`, or local paths. But flake registry entries mapping from direct flake URLs often come from swapping the 'from' and 'to' arguments by accident, and even when created intentionally, they may not actually work correctly. This patch rejects those URLs (and fully-qualified flake: URLs), making it harder to swap the arguments by accident. Fixes #181. Change-Id: I24713643a534166c052719b8770a4edfcfdb8cf3
2024-06-25libexpr: rewrite the parser with pegtl instead of flex/bisoneldritch horrors
this gives about 20% performance improvements on pure parsing. obviously it will be less on full eval, but depending on how much parsing is to be done (e.g. including hackage-packages.nix or not) it's more like 4%-10%. this has been tested (with thousands of core hours of fuzzing) to ensure that the ASTs produced by the new parser are exactly the same as the old one would have produced. error messages will change (sometimes by a lot) and are not yet perfect, but we would rather leave this as is for later. test results for running only the parser (excluding the variable binding code) in a tight loop with inputs and parameters as given are promising: - 40% faster on lix's package.nix at 10000 iterations - 1.3% faster on nixpkgs all-packages.nix at 1000 iterations - equivalent on all of nixpkgs concatenated at 100 iterations (excluding invalid files, each file surrounded with parens) more realistic benchmarks are somewhere in between the extremes, parsing once again getting the largest uplift. other realistic workloads improve by a few percentage points as well, notably system builds are 4% faster. Benchmarks summary (from ./bench/summarize.jq bench/bench-*.json) old/bin/nix --extra-experimental-features 'nix-command flakes' eval -f bench/nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix mean: 0.408s ± 0.025s user: 0.355s | system: 0.033s median: 0.389s range: 0.388s ... 0.442s relative: 1 new/bin/nix --extra-experimental-features 'nix-command flakes' eval -f bench/nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix mean: 0.332s ± 0.024s user: 0.279s | system: 0.033s median: 0.314s range: 0.313s ... 0.361s relative: 0.814 --- old/bin/nix --extra-experimental-features 'nix-command flakes' eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system' mean: 6.133s ± 0.022s user: 5.395s | system: 0.437s median: 6.128s range: 6.099s ... 6.183s relative: 1 new/bin/nix --extra-experimental-features 'nix-command flakes' eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system' mean: 5.925s ± 0.025s user: 5.176s | system: 0.456s median: 5.934s range: 5.861s ... 5.943s relative: 0.966 --- GC_INITIAL_HEAP_SIZE=10g old/bin/nix eval --extra-experimental-features 'nix-command flakes' --raw --impure --expr 'with import <nixpkgs/nixos> {}; system' mean: 4.503s ± 0.027s user: 3.731s | system: 0.547s median: 4.499s range: 4.478s ... 4.541s relative: 1 GC_INITIAL_HEAP_SIZE=10g new/bin/nix eval --extra-experimental-features 'nix-command flakes' --raw --impure --expr 'with import <nixpkgs/nixos> {}; system' mean: 4.285s ± 0.031s user: 3.504s | system: 0.571s median: 4.281s range: 4.221s ... 4.328s relative: 0.951 --- old/bin/nix --extra-experimental-features 'nix-command flakes' search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello mean: 16.475s ± 0.07s user: 14.088s | system: 1.572s median: 16.495s range: 16.351s ... 16.536s relative: 1 new/bin/nix --extra-experimental-features 'nix-command flakes' search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello mean: 15.973s ± 0.013s user: 13.558s | system: 1.615s median: 15.973s range: 15.946s ... 15.99s relative: 0.97 --- Change-Id: Ie66ec2d045dec964632c6541e25f8f0797319ee2
2024-06-25Merge "Revert "libfetchers: make attribute / URL query handling consistent"" ↵jade
into main
2024-06-24Revert "libfetchers: make attribute / URL query handling consistent"jade
This reverts commit 35eec921af1043fc6322edc0ad88c872d41623b8. Reason for revert: Regressed nix-eval-jobs, and it appears to be this change is buggy/missing a case. It just needs another pass. Code causing the problem in n-e-j, when invoked with `nix-eval-jobs --flake '.#hydraJobs'`: ``` n-e-j/tests/assets » ../../build/src/nix-eval-jobs --meta --workers 1 --flake .#hydraJobs warning: unknown setting 'trusted-users' warning: `--gc-roots-dir' not specified error: unsupported Git input attribute 'dir' error: worker error: error: unsupported Git input attribute 'dir' ``` ``` nix::Value *vRoot = [&]() { if (args.flake) { auto [flakeRef, fragment, outputSpec] = nix::parseFlakeRefWithFragmentAndExtendedOutputsSpec( args.releaseExpr, nix::absPath(".")); nix::InstallableFlake flake{ {}, state, std::move(flakeRef), fragment, outputSpec, {}, {}, args.lockFlags}; return flake.toValue(*state).first; } else { return releaseExprTopLevelValue(*state, autoArgs, args); } }(); ``` Inspecting the program behaviour reveals that `dir` was in fact set in the URL going into the fetcher. This is in turn because unlike in the case changed in this commit, it was not erased before handing it to libfetchers, which is probably just a mistake. ``` (rr) up 3 0x00007ffff60262ae in nix::fetchers::Input::fromURL (url=..., requireTree=requireTree@entry=true) at src/libfetchers/fetchers.cc:39 warning: Source file is more recent than executable. 39 auto res = inputScheme->inputFromURL(url, requireTree); (rr) p url $1 = (const nix::ParsedURL &) @0x7fffdc874190: {url = "git+file:///home/jade/lix/nix-eval-jobs", base = "git+file:///home/jade/lix/nix-eval-jobs", scheme = "git+file", authority = std::optional<std::string> = {[contained value] = ""}, path = "/home/jade/lix/nix-eval-jobs", query = std::map with 1 element = {["dir"] = "tests/assets"}, fragment = ""} (rr) up 4 0x00007ffff789d904 in nix::parseFlakeRefWithFragment (url=".#hydraJobs", baseDir=std::optional<std::string> = {...}, allowMissing=allowMissing@entry=false, isFlake=isFlake@entry=true) at src/libexpr/flake/flakeref.cc:179 warning: Source file is more recent than executable. 179 FlakeRef(Input::fromURL(parsedURL, isFlake), getOr(parsedURL.query, "dir", "")), (rr) p parsedURL $2 = {url = "git+file:///home/jade/lix/nix-eval-jobs", base = "git+file:///home/jade/lix/nix-eval-jobs", scheme = "git+file", authority = std::optional<std::string> = {[contained value] = ""}, path = "/home/jade/lix/nix-eval-jobs", query = std::map with 1 element = { ["dir"] = "tests/assets"}, fragment = ""} (rr) list 174 175 if (pathExists(flakeRoot + "/.git/shallow")) 176 parsedURL.query.insert_or_assign("shallow", "1"); 177 178 return std::make_pair( 179 FlakeRef(Input::fromURL(parsedURL, isFlake), getOr(parsedURL.query, "dir", "")), 180 fragment); 181 } ``` Change-Id: Ib55a882eaeb3e59228857761dc1e3b2e366b0f5e
2024-06-23Merge "libfetchers: make attribute / URL query handling consistent" into mainMaximilian Bosch
2024-06-22mildly cleanup ExprSelect::evalQyriad
Better variable names, some comments, and a slight logic rearrange. Change-Id: I9685ae252f83217aa85f06432234159c9ad19d1c
2024-06-22doc-comment ExprSelect's fieldsQyriad
Change-Id: I63e79991a4bab93421266785e9258e0f5bb89b8f
2024-06-22libfetchers: make attribute / URL query handling consistentMaximilian Bosch
The original idea was to fix lix#174, but for a user friendly solution, I figured that we'd need more consistency: * Invalid query params will cause an error, just like invalid attributes. This has the following two consequences: * The `?dir=`-param from flakes will be removed before the URL to be fetched is passed to libfetchers. * The tarball fetcher doesn't allow URLs with custom query params anymore. I think this was questionable anyways given that an arbitrary set of query params was silently removed from the URL you wanted to fetch. The correct way is to use an attribute-set with a key `url` that contains the tarball URL to fetch. * Same for the git & mercurial fetchers: in that case it doesn't even matter though: both fetchers added unused query params to the URL that's passed from the input scheme to the fetcher (`url2` in the code). It turns out that this was never used since the query parameters were erased again in `getActualUrl`. * Validation happens for both attributes and URLs. Previously, a lot of fetchers validated e.g. refs/revs only when specified in a URL and the validity of attribute names only in `inputFromAttrs`. Now, all the validation is done in `inputFromAttrs` and `inputFromURL` constructs attributes that will be passed to `inputFromAttrs`. * Accept all attributes as URL query parameters. That also includes lesser used ones such as `narHash`. And "output" attributes like `lastModified`: these could be declared already when declaring inputs as attribute rather than URL. Now the behavior is at least consistent. Personally, I think we should differentiate in the future between "fetched input" (basically the attr-set that ends up in the lock-file) and "unfetched input" earlier: both inputFrom{Attrs,URL} entrypoints are probably OK for unfetched inputs, but for locked/fetched inputs a custom entrypoint should be used. Then, the current entrypoints wouldn't have to allow these attributes anymore. Change-Id: I1be1992249f7af8287cfc37891ab505ddaa2e8cd