Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge master into #3600 #4155

Merged
merged 2,242 commits into from Sep 15, 2021
Merged

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Oct 17, 2020

The next few conflicts are more interesting, so I stopped here for now.

@Ericson2314 Ericson2314 changed the title Merge with master before useUserNamespace Merge master into #3600 Oct 17, 2020
edolstra and others added 28 commits Jan 13, 2021
…finity

Set kern.curproc_arch_affinity=0 to escape Rosetta
Thanks @regnat and @edolstra for catching this and comming up with the
solution.

They way I had generalized those is wrong, because local settings for
non-local stores is confusing default. And due to the nature of C++
inheritance, fixing the defaults is more annoying than it should be.
Additionally, I thought we might just drop the check in the substitution
logic since `Store::addToStore` is now streaming, but @regnat rightfully
pointed out that as it downloads dependencies first, that would still be
too late, and also waste effort on possibly unneeded/unwanted
dependencies.

The simple and correct thing to do is just make a store method for the
boolean logic, keeping all the setting and key stuff the way it was
before. That new method is both used by `LocalStore::addToStore` and the
substitution goal check. Perhaps we might eventually make it fancier,
e.g. sending the ValidPathInfo to remote stores for them to validate,
but this is good enough for now.
2259 error message - "auto-call" error
While interpreting the output is fairly intuitive it would be better to
explicitly specify what a good invocation looks like.

That this isn't completely obvious (or at least causes folks to
second-guess themselves) can be seen in a couple user threads:

- https://discourse.nixos.org/t/nixos-cache-fetching-issue/3575/11
- https://discourse.nixos.org/t/newbie-question-cant-get-trivial-example-of-nixops-to-work-on-my-mac/1125/8
Document expected output of 'nix store ping'.
With the `ca-derivation` experimental features, non-ca derivations used
to have their output paths returned as unknown as long as they weren't
built (because of a mistake in the code that systematically erased the
previous value)
Fix the drv output map for non ca derivations
Prevent some `nix flake` commands to crash by trying to parse a
placeholder output as a store path
Escape filename given to nix-shell in shebang mode
…he compressed hash

This change is to simplify [Trustix](https://github.com/tweag/trustix) indexing and makes it possible to reconstruct this URL regardless of the compression used.

In particular this means that https://github.com/tweag/trustix/blob/7c2e9ca597de233846e0b265fb081626ca6c59d8/contrib/nix/nar/nar.go#L61-L71 can be removed and only the bits that are required to establish trust needs to be published in the Trustix build logs.
Changes:

* The divider lines are gone. These were in practice a bit confusing,
  in particular with --show-trace or --keep-going, since then there
  were multiple lines, suggesting a start/end which wasn't the case.

* Instead, multi-line error messages are now indented to align with
  the prefix (e.g. "error: ").

* The 'description' field is gone since we weren't really using it.

* 'hint' is renamed to 'msg' since it really wasn't a hint.

* The error is now printed *before* the location info.

* The 'name' field is no longer printed since most of the time it
  wasn't very useful since it was just the name of the exception (like
  EvalError). Ideally in the future this would be a unique, easily
  googleable error ID (like rustc).

* "trace:" is now just "…". This assumes error contexts start with
  something like "while doing X".

Example before:

  error: --- AssertionError ---------------------------------------------------------------------------------------- nix
  at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

       6|
       7|   x = assert false; 1;
        |       ^
       8|

  assertion 'false' failed
  ----------------------------------------------------- show-trace -----------------------------------------------------
  trace: while evaluating the attribute 'x' of the derivation 'hello-2.10'
  at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix

     191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
     192|           name = "${attrs.pname}-${attrs.version}";
        |           ^
     193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {

Example after:

  error: assertion 'false' failed

         at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

              6|
              7|   x = assert false; 1;
               |       ^
              8|

         … while evaluating the attribute 'x' of the derivation 'hello-2.10'

         at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix

            191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
            192|           name = "${attrs.pname}-${attrs.version}";
               |           ^
            193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {
It's now

  at /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix:7:7:

instead of

  at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

The new format is more standard and clickable.
bjornfor and others added 21 commits Mar 30, 2021
This fixes builtins.fetchGit { url = ...; ref = "HEAD"; }, that works in
stable nix (v2.3.10), but is broken in nix master:

  $ ./result/bin/nix repl
  Welcome to Nix version 2.4pre19700101_dd77f71. Type :? for help.

  nix-repl> builtins.fetchGit { url = "https://github.com/NixOS/nix"; ref = "HEAD"; }
  fetching Git repository 'https://github.com/NixOS/nix'fatal: couldn't find remote ref refs/heads/HEAD
  error: program 'git' failed with exit code 128

The documentation for builtins.fetchGit says ref = "HEAD" is the
default, so it should also be supported to explicitly pass it.

I came across this issue because poetry2nix can use ref = "HEAD" in some
situations.

Fixes NixOS#4674.
fetchGit: don't prefix "refs/heads/" on ref = "HEAD"
[prerequisites]: add JSON lib dependency
In the following commits it will become less prevalent.
These are by no means part of the notion of a store, but rather are
things that happen to use stores. (Or put another way, there's no way
we'd make them virtual methods any time soon.) It's better to move them
out of that too-big class then.

Also, this helps us remove StorePathWithOutputs from the Store interface
altogether next commit.
This avoids an ambiguity where the `StorePathWithOutputs { drvPath, {}
}` could mean "build `brvPath`" or "substitute `drvPath`" depending on
context.

It also brings the internals closer in line to the new CLI, by
generalizing the `Buildable` type is used there and makes that
distinction already.

In doing so, relegate `StorePathWithOutputs` to being a type just for
backwards compatibility (CLI and RPC).
This makes for better types errors and allows us to give it methods.
This allows us to namespace its constructors under it.
Use `DerivedPath` for `buildPaths` and `ensurePath`
If there were many top-level goals (which are not destroyed until the
very end), commands like

  $ nix copy --to 'ssh://localhost?remote-store=/tmp/nix' \
    /run/current-system --no-check-sigs --substitute-on-destination

could fail with "Too many open files". So now we do some explicit
cleanup from amDone(). It would be cleaner to separate goals from
their temporary internal state, but that would be a bigger refactor.
nixos-discourse
Copy link

nixos-discourse commented on 77f5d17 Apr 7, 2021

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

https://discourse.nixos.org/t/tweag-nix-dev-update-9/12357/1

@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Apr 12, 2021

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

https://discourse.nixos.org/t/contributor-retention/12412/21

Ma27
Copy link
Member

Ma27 commented on 255d145 May 1, 2021

@Ericson2314 @edolstra since this revision I get the following issue when opening a nix-shell:

λ ma27 [~/Projects/nixpkgs] at  update-nix-unstable ?
→ ./result/bin/nix-shell -A hello                                                           [78effd79f10]
error: derivation '/nix/store/zipc1j00q8i30xaylg0fz64lf2ypx752-bash-interactive-4.4-p23.drv' does not have wanted outputs '*'
Ma27
Copy link
Member

Ma27 commented on 255d145 May 1, 2021

AFAIU the check shouldn't throw an error if a wildcard is used for outputs. Currently testing the following patch locally:

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 9100d3333..116fc46db 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -1292,10 +1292,13 @@ void DerivationGoal::checkPathValidity()
     // If we requested all the outputs via the empty set, we are always fine.
     // If we requested specific elements, the loop above removes all the valid
     // ones, so any that are left must be invalid.
-    if (!wantedOutputsLeft.empty())
-        throw Error("derivation '%s' does not have wanted outputs %s",
-            worker.store.printStorePath(drvPath),
-            concatStringsSep(", ", quoteStrings(wantedOutputsLeft)));
+    if (!wantedOutputsLeft.empty()) {
+        if (!(wantedOutputsLeft.size() == 1 && wantedOutputs.find("*") != wantedOutputs.end())) {
+            throw Error("derivation '%s' does not have wanted outputs %s",
+                worker.store.printStorePath(drvPath),
+                concatStringsSep(", ", quoteStrings(wantedOutputsLeft)));
+        }
+    }
 }
Ericson2314
Copy link
Member

Ericson2314 commented on 255d145 May 2, 2021

@Ma27 This is what's supposed to handle the *.

@mkenigs mkenigs mentioned this pull request Sep 3, 2021
@edolstra edolstra merged commit 3b82c1a into NixOS:auto-uid-allocation Sep 15, 2021
6 checks passed
@edolstra
Copy link
Member

@edolstra edolstra commented Sep 15, 2021

Closing due to #5212 which updates this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet