From b31d4b689ca9f82e1c4060ea41e92691d5c94ee8 Mon Sep 17 00:00:00 2001
From: aameen-tulip <98349328+aameen-tulip@users.noreply.github.com>
Date: Thu, 16 Feb 2023 18:47:45 -0600
Subject: [PATCH 01/22] Document `hasAllInfo`

If this documentation is inaccurate in any way please do not hesitate to suggest corrections.

My understanding of this function is strictly from reading the source code and some limited experience implementing fetchers.
---
 src/libfetchers/fetchers.hh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh
index 17da37f47..95c0f5974 100644
--- a/src/libfetchers/fetchers.hh
+++ b/src/libfetchers/fetchers.hh
@@ -63,6 +63,11 @@ public:
        one that contains a commit hash or content hash. */
     bool isLocked() const { return locked; }
 
+    /* Check whether the input carries all necessary info required
+       for cache insertion and substitution.
+       These fields are used to uniquely identify cached trees
+       within the "tarball TTL" window without necessarily
+       indicating that the input's origin is unchanged. */
     bool hasAllInfo() const;
 
     bool operator ==(const Input & other) const;

From 8648ebc2cc320578c8b00695d35b3f44141c7bbe Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 20 Feb 2023 20:27:50 +0100
Subject: [PATCH 02/22] Add ./configure --disable-tests option

Building without tests is useful for bootstrapping with a smaller footprint
or running the tests in a separate derivation. Otherwise, we do compile and
run them.

This isn't fine grained as to allow picking `check` but not `installcheck`
or vice versa, but it's good enough for now.

I've tried to use Nixpkgs' `checkInputs`, but those inputs weren't discovered
properly by the configure script. We can emulate its behavior very well though.
---
 Makefile            | 19 +++++++++++++------
 Makefile.config.in  |  1 +
 configure.ac        | 10 ++++++++++
 flake.nix           | 36 +++++++++++++++++++++++++-----------
 mk/disable-tests.mk | 12 ++++++++++++
 5 files changed, 61 insertions(+), 17 deletions(-)
 create mode 100644 mk/disable-tests.mk

diff --git a/Makefile b/Makefile
index 8675c9925..31e4961bc 100644
--- a/Makefile
+++ b/Makefile
@@ -2,13 +2,10 @@ makefiles = \
   mk/precompiled-headers.mk \
   local.mk \
   src/libutil/local.mk \
-  src/libutil/tests/local.mk \
   src/libstore/local.mk \
-  src/libstore/tests/local.mk \
   src/libfetchers/local.mk \
   src/libmain/local.mk \
   src/libexpr/local.mk \
-  src/libexpr/tests/local.mk \
   src/libcmd/local.mk \
   src/nix/local.mk \
   src/resolve-system-dependencies/local.mk \
@@ -19,12 +16,22 @@ makefiles = \
   misc/systemd/local.mk \
   misc/launchd/local.mk \
   misc/upstart/local.mk \
-  doc/manual/local.mk \
-  tests/local.mk \
-  tests/plugins/local.mk
+  doc/manual/local.mk
 
 -include Makefile.config
 
+ifeq ($(tests), yes)
+makefiles += \
+  src/libutil/tests/local.mk \
+  src/libstore/tests/local.mk \
+  src/libexpr/tests/local.mk \
+  tests/local.mk \
+  tests/plugins/local.mk
+else
+makefiles += \
+  mk/disable-tests.mk
+endif
+
 OPTIMIZE = 1
 
 ifeq ($(OPTIMIZE), 1)
diff --git a/Makefile.config.in b/Makefile.config.in
index 1c5405c6d..a6c84f2ad 100644
--- a/Makefile.config.in
+++ b/Makefile.config.in
@@ -45,3 +45,4 @@ sandbox_shell = @sandbox_shell@
 storedir = @storedir@
 sysconfdir = @sysconfdir@
 system = @system@
+tests = @tests@
diff --git a/configure.ac b/configure.ac
index 09b3651b9..36e119bed 100644
--- a/configure.ac
+++ b/configure.ac
@@ -145,6 +145,13 @@ if test "x$GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC" = xyes; then
     LDFLAGS="-latomic $LDFLAGS"
 fi
 
+# Building without tests is useful for bootstrapping with a smaller footprint
+# or running the tests in a separate derivation. Otherwise, we do compile and
+# run them.
+AC_ARG_ENABLE(tests, AS_HELP_STRING([--disable-tests],[Do not build the tests]),
+  tests=$enableval, tests=yes)
+AC_SUBST(tests)
+
 # LTO is currently broken with clang for unknown reasons; ld segfaults in the llvm plugin
 AC_ARG_ENABLE(lto, AS_HELP_STRING([--enable-lto],[Enable LTO (only supported with GCC) [default=no]]),
   lto=$enableval, lto=no)
@@ -270,6 +277,8 @@ if test "$gc" = yes; then
 fi
 
 
+if test "$tests" = yes; then
+
 # Look for gtest.
 PKG_CHECK_MODULES([GTEST], [gtest_main])
 
@@ -282,6 +291,7 @@ dnl No good for C++ libs with mangled symbols
 dnl AC_CHECK_LIB([rapidcheck], [])
 AC_LANG_POP(C++)
 
+fi
 
 # Look for nlohmann/json.
 PKG_CHECK_MODULES([NLOHMANN_JSON], [nlohmann_json >= 3.9])
diff --git a/flake.nix b/flake.nix
index 563a46d65..5f2b46d57 100644
--- a/flake.nix
+++ b/flake.nix
@@ -89,15 +89,16 @@
         });
 
         configureFlags =
-          [
-            "CXXFLAGS=-I${lib.getDev rapidcheck}/extras/gtest/include"
-          ] ++ lib.optionals stdenv.isLinux [
+          lib.optionals stdenv.isLinux [
             "--with-boost=${boost}/lib"
             "--with-sandbox-shell=${sh}/bin/busybox"
           ]
           ++ lib.optionals (stdenv.isLinux && !(isStatic && stdenv.system == "aarch64-linux")) [
             "LDFLAGS=-fuse-ld=gold"
           ];
+        testConfigureFlags = [
+          "CXXFLAGS=-I${lib.getDev rapidcheck}/extras/gtest/include"
+        ];
 
         nativeBuildDeps =
           [
@@ -124,13 +125,16 @@
             libarchive
             boost
             lowdown-nix
-            gtest
-            rapidcheck
           ]
           ++ lib.optionals stdenv.isLinux [libseccomp]
           ++ lib.optional (stdenv.isLinux || stdenv.isDarwin) libsodium
           ++ lib.optional stdenv.hostPlatform.isx86_64 libcpuid;
 
+        checkDeps = [
+          gtest
+          rapidcheck
+        ];
+
         awsDeps = lib.optional (stdenv.isLinux || stdenv.isDarwin)
           (aws-sdk-cpp.override {
             apis = ["s3" "transfer"];
@@ -200,7 +204,7 @@
         VERSION_SUFFIX = versionSuffix;
 
         nativeBuildInputs = nativeBuildDeps;
-        buildInputs = buildDeps ++ awsDeps;
+        buildInputs = buildDeps ++ awsDeps ++ checkDeps;
         propagatedBuildInputs = propagatedDeps;
 
         enableParallelBuilding = true;
@@ -305,7 +309,7 @@
           };
           let
             canRunInstalled = currentStdenv.buildPlatform.canExecute currentStdenv.hostPlatform;
-          in currentStdenv.mkDerivation {
+          in currentStdenv.mkDerivation (finalAttrs: {
             name = "nix-${version}";
             inherit version;
 
@@ -318,7 +322,8 @@
             nativeBuildInputs = nativeBuildDeps;
             buildInputs = buildDeps
               # There have been issues building these dependencies
-              ++ lib.optionals (currentStdenv.hostPlatform == currentStdenv.buildPlatform) awsDeps;
+              ++ lib.optionals (currentStdenv.hostPlatform == currentStdenv.buildPlatform) awsDeps
+              ++ lib.optionals finalAttrs.doCheck checkDeps;
 
             propagatedBuildInputs = propagatedDeps;
 
@@ -348,6 +353,7 @@
             configureFlags = configureFlags ++
               [ "--sysconfdir=/etc" ] ++
               lib.optional stdenv.hostPlatform.isStatic "--enable-embedded-sandbox-shell" ++
+              (if finalAttrs.doCheck then testConfigureFlags else [ "--disable-tests" ]) ++
               lib.optional (!canRunInstalled) "--disable-doc-gen";
 
             enableParallelBuilding = true;
@@ -369,7 +375,7 @@
               ''}
             '';
 
-            doInstallCheck = true;
+            doInstallCheck = finalAttrs.doCheck;
             installCheckFlags = "sysconfdir=$(out)/etc";
 
             separateDebugInfo = !currentStdenv.hostPlatform.isStatic;
@@ -411,7 +417,7 @@
             });
 
             meta.platforms = lib.platforms.unix;
-          };
+          });
 
           lowdown-nix = with final; currentStdenv.mkDerivation rec {
             name = "lowdown-0.9.0";
@@ -462,6 +468,14 @@
 
         buildNoGc = forAllSystems (system: self.packages.${system}.nix.overrideAttrs (a: { configureFlags = (a.configureFlags or []) ++ ["--enable-gc=no"];}));
 
+        buildNoTests = forAllSystems (system:
+          self.packages.${system}.nix.overrideAttrs (a: {
+            doCheck =
+              assert ! a?dontCheck;
+              false;
+          })
+        );
+
         # Perl bindings for various platforms.
         perlBindings = forAllSystems (system: nixpkgsFor.${system}.native.nix.perl-bindings);
 
@@ -634,7 +648,7 @@
             nativeBuildInputs = nativeBuildDeps
                                 ++ (lib.optionals stdenv.cc.isClang [ pkgs.bear pkgs.clang-tools ]);
 
-            buildInputs = buildDeps ++ propagatedDeps ++ awsDeps;
+            buildInputs = buildDeps ++ propagatedDeps ++ awsDeps ++ checkDeps;
 
             inherit configureFlags;
 
diff --git a/mk/disable-tests.mk b/mk/disable-tests.mk
new file mode 100644
index 000000000..f72f84412
--- /dev/null
+++ b/mk/disable-tests.mk
@@ -0,0 +1,12 @@
+# This file is only active for `./configure --disable-tests`.
+# Running `make check` or `make installcheck` would indicate a mistake in the
+# caller.
+
+installcheck:
+	@echo "Tests are disabled. Configure without '--disable-tests', or avoid calling 'make installcheck'."
+	@exit 1
+
+# This currently has little effect.
+check:
+	@echo "Tests are disabled. Configure without '--disable-tests', or avoid calling 'make check'."
+	@exit 1

From c6051cac6f6258712ed64b3359004b5fe25c7d69 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Fri, 24 Feb 2023 09:59:25 +0100
Subject: [PATCH 03/22] doc: Add test dependencies to prerequisites

---
 doc/manual/src/installation/prerequisites-source.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/manual/src/installation/prerequisites-source.md b/doc/manual/src/installation/prerequisites-source.md
index 6f4eb3008..5a708f11b 100644
--- a/doc/manual/src/installation/prerequisites-source.md
+++ b/doc/manual/src/installation/prerequisites-source.md
@@ -71,3 +71,8 @@
     <http://libcpuid.sourceforge.net>.
     This is an optional dependency and can be disabled
     by providing a `--disable-cpuid` to the `configure` script.
+
+  - Unless `./configure --disable-tests` is specified, GoogleTest (GTest) and
+    RapidCheck are required, which are available at
+    <https://google.github.io/googletest/> and
+    <https://github.com/emil-e/rapidcheck> respectively.

From 3113b13df9afaab918792c725742c6bc3fcec88b Mon Sep 17 00:00:00 2001
From: Bob van der Linden <bobvanderlinden@gmail.com>
Date: Wed, 8 Feb 2023 20:03:57 +0100
Subject: [PATCH 04/22] buildenv: throw BuildEnvFileConflictError with more
 context

At the moment an Error is thrown that only holds an error message
regarding `nix-env` and `nix profile`. These tools make use of
builtins.buildEnv, but buildEnv is also used in other places. These
places are unrelated to Nix profiles, so the error shouldn't mention
these tools.

This generic error is now BuildEnvFileConflictError, which holds more
contextual information about the files that were conflicting while
building the environment.
---
 src/libstore/builtins/buildenv.cc | 12 +++++-------
 src/libstore/builtins/buildenv.hh | 26 ++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc
index b1fbda13d..7bba33fb9 100644
--- a/src/libstore/builtins/buildenv.cc
+++ b/src/libstore/builtins/buildenv.cc
@@ -92,13 +92,11 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir,
                 if (S_ISLNK(dstSt.st_mode)) {
                     auto prevPriority = state.priorities[dstFile];
                     if (prevPriority == priority)
-                        throw Error(
-                                "files '%1%' and '%2%' have the same priority %3%; "
-                                "use 'nix-env --set-flag priority NUMBER INSTALLED_PKGNAME' "
-                                "or type 'nix profile install --help' if using 'nix profile' to find out how "
-                                "to change the priority of one of the conflicting packages"
-                                " (0 being the highest priority)",
-                                srcFile, readLink(dstFile), priority);
+                        throw BuildEnvFileConflictError(
+                            readLink(dstFile),
+                            srcFile,
+                            priority
+                        );
                     if (prevPriority < priority)
                         continue;
                     if (unlink(dstFile.c_str()) == -1)
diff --git a/src/libstore/builtins/buildenv.hh b/src/libstore/builtins/buildenv.hh
index 73c0f5f7f..a018de3af 100644
--- a/src/libstore/builtins/buildenv.hh
+++ b/src/libstore/builtins/buildenv.hh
@@ -12,6 +12,32 @@ struct Package {
     Package(const Path & path, bool active, int priority) : path{path}, active{active}, priority{priority} {}
 };
 
+class BuildEnvFileConflictError : public Error
+{
+public:
+    const Path fileA;
+    const Path fileB;
+    int priority;
+
+    BuildEnvFileConflictError(
+        const Path fileA,
+        const Path fileB,
+        int priority
+    )
+        : Error(
+            "Unable to build profile. There is a conflict for the following files:\n"
+            "\n"
+            "  %1%\n"
+            "  %2%",
+            fileA,
+            fileB
+        )
+        , fileA(fileA)
+        , fileB(fileB)
+        , priority(priority)
+    {}
+};
+
 typedef std::vector<Package> Packages;
 
 void buildProfile(const Path & out, Packages && pkgs);

From 872cdb4346f72186ae683f90ac97b8a7d9bddfd4 Mon Sep 17 00:00:00 2001
From: Bob van der Linden <bobvanderlinden@gmail.com>
Date: Fri, 10 Feb 2023 14:18:27 +0100
Subject: [PATCH 05/22] nix-profile-install: show helpful error upon package
 conflict

Whenever a file conflict happens during "nix profile install" an error
is shown that was previously thrown inside builtins.buildEnv.

We catch BuildProfileConflictError here so that we can provide the user
with more useful instructions on what to do next.

Most notably, we give the user concrete commands to use with all
parameters  already filled in. This avoids the need for the user to look
up these commands in manual pages.
---
 src/nix/profile.cc | 56 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/src/nix/profile.cc b/src/nix/profile.cc
index 32364e720..3464a977d 100644
--- a/src/nix/profile.cc
+++ b/src/nix/profile.cc
@@ -329,7 +329,61 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile
             manifest.elements.push_back(std::move(element));
         }
 
-        updateProfile(manifest.build(store));
+        try {
+            updateProfile(manifest.build(store));
+        } catch (BuildEnvFileConflictError & conflictError) {
+            auto findRefByFilePath = [&]<typename Iterator>(Iterator begin, Iterator end) {
+                for (auto it = begin; it != end; it++) {
+                    auto profileElement = *it;
+                    for (auto & storePath : profileElement.storePaths) {
+                        if (conflictError.fileA.starts_with(store->printStorePath(storePath))) {
+                            return std::pair(conflictError.fileA, profileElement.source->originalRef);
+                        }
+                        if (conflictError.fileB.starts_with(store->printStorePath(storePath))) {
+                            return std::pair(conflictError.fileB, profileElement.source->originalRef);
+                        }
+                    }
+                }
+                throw conflictError;
+            };
+            // There are 2 conflicting files. We need to find out which one is from the already installed package and
+            // which one is the package that is the new package that is being installed.
+            // The first matching package is the one that was already installed (original).
+            auto [originalConflictingFilePath, originalConflictingRef] = findRefByFilePath(manifest.elements.begin(), manifest.elements.end());
+            // The last matching package is the one that was going to be installed (new).
+            auto [newConflictingFilePath, newConflictingRef] = findRefByFilePath(manifest.elements.rbegin(), manifest.elements.rend());
+
+            throw Error(
+                "An existing package already provides the following file:\n"
+                "\n"
+                "  %1%\n"
+                "\n"
+                "This is the conflicting file from the new package:\n"
+                "\n"
+                "  %2%\n"
+                "\n"
+                "To remove the existing package:\n"
+                "\n"
+                "  nix profile remove %3%\n"
+                "\n"
+                "The new package can also be installed next to the existing one by assigning a different priority.\n"
+                "The conflicting packages have a priority of %5%.\n"
+                "To prioritise the new package:\n"
+                "\n"
+                "  nix profile install %4% --priority %6%\n"
+                "\n"
+                "To prioritise the existing package:\n"
+                "\n"
+                "  nix profile install %4% --priority %7%\n",
+                originalConflictingFilePath,
+                newConflictingFilePath,
+                originalConflictingRef.to_string(),
+                newConflictingRef.to_string(),
+                conflictError.priority,
+                conflictError.priority - 1,
+                conflictError.priority + 1
+            );
+        }
     }
 };
 

From 3efa476c5439f8f6c1968a6ba20a31d1239c2f04 Mon Sep 17 00:00:00 2001
From: Bob van der Linden <bobvanderlinden@gmail.com>
Date: Fri, 24 Feb 2023 21:26:36 +0100
Subject: [PATCH 06/22] tests: nix-profile: test install error message upon
 conflicting files

---
 tests/nix-profile.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tests/nix-profile.sh b/tests/nix-profile.sh
index 266dc9e49..d4beaa190 100644
--- a/tests/nix-profile.sh
+++ b/tests/nix-profile.sh
@@ -140,6 +140,32 @@ printf World2 > $flake2Dir/who
 
 nix profile install $flake1Dir
 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]]
+expect 1 nix profile install $flake2Dir
+diff -u <(nix profile install $flake2Dir 2>&1 1> /dev/null || true) <(cat << EOF
+error: An existing package already provides the following file:
+
+         $(nix build --no-link --print-out-paths ${flake1Dir}"#default.out")/bin/hello
+
+       This is the conflicting file from the new package:
+
+         $(nix build --no-link --print-out-paths ${flake2Dir}"#default.out")/bin/hello
+
+       To remove the existing package:
+
+         nix profile remove path:${flake1Dir}
+
+       The new package can also be installed next to the existing one by assigning a different priority.
+       The conflicting packages have a priority of 5.
+       To prioritise the new package:
+
+         nix profile install path:${flake2Dir} --priority 4
+
+       To prioritise the existing package:
+
+         nix profile install path:${flake2Dir} --priority 6
+EOF
+)
+[[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]]
 nix profile install $flake2Dir --priority 100
 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]]
 nix profile install $flake2Dir --priority 0

From 0167862e8e3faddeab6fb40a08c05a63865fe96b Mon Sep 17 00:00:00 2001
From: Bob van der Linden <bobvanderlinden@gmail.com>
Date: Tue, 28 Feb 2023 10:53:42 +0100
Subject: [PATCH 07/22] fixup! tests: nix-profile: test install error message
 upon conflicting files

---
 tests/nix-profile.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/nix-profile.sh b/tests/nix-profile.sh
index d4beaa190..652e8a8f2 100644
--- a/tests/nix-profile.sh
+++ b/tests/nix-profile.sh
@@ -141,7 +141,11 @@ printf World2 > $flake2Dir/who
 nix profile install $flake1Dir
 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]]
 expect 1 nix profile install $flake2Dir
-diff -u <(nix profile install $flake2Dir 2>&1 1> /dev/null || true) <(cat << EOF
+diff -u <(
+    nix --offline profile install $flake2Dir 2>&1 1> /dev/null \
+        | grep -vE "^warning: " \
+        || true
+) <(cat << EOF
 error: An existing package already provides the following file:
 
          $(nix build --no-link --print-out-paths ${flake1Dir}"#default.out")/bin/hello

From 892d46adbbb284501c00c7bfe0914d633845bcd2 Mon Sep 17 00:00:00 2001
From: Robert Hensing <roberth@users.noreply.github.com>
Date: Tue, 28 Feb 2023 15:56:46 +0100
Subject: [PATCH 08/22] flake.nix: Force the ./configure tests setting

This always forces the setting, rather than relying on its default, and cleans up the code a bit.

Co-authored-by: John Ericson <git@JohnEricson.me>
---
 flake.nix | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/flake.nix b/flake.nix
index 5f2b46d57..2000c2d73 100644
--- a/flake.nix
+++ b/flake.nix
@@ -353,7 +353,8 @@
             configureFlags = configureFlags ++
               [ "--sysconfdir=/etc" ] ++
               lib.optional stdenv.hostPlatform.isStatic "--enable-embedded-sandbox-shell" ++
-              (if finalAttrs.doCheck then testConfigureFlags else [ "--disable-tests" ]) ++
+              [ (lib.enableFeature finalAttrs.doCheck "tests") ] ++
+              lib.optionals finalAttrs.doCheck testConfigureFlags ++
               lib.optional (!canRunInstalled) "--disable-doc-gen";
 
             enableParallelBuilding = true;

From ea0adfc582e8c0d233cec03da9258caf718678b7 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 15 Dec 2022 22:44:14 -0500
Subject: [PATCH 09/22] Get rid of `.drv` special-casing for store path
 installables

The release notes document the change in behavior, I don't include it
here so there is no risk to it getting out of sync.

> Motivation

>> Plumbing CLI should be simple

Store derivation installations are intended as "plumbing": very simple
utilities for advanced users and scripts, and not what regular users
interact with. (Similarly, regular Git users will use branch and tag
names not explicit hashes for most things.)

The plumbing CLI should prize simplicity over convenience; that is its
raison d'etre. If the user provides a path, we should treat it the same
way not caring what sort of path it is.

>> Scripting

This is especially important for the scripting use-case. when arbitrary
paths are sent to e.g. `nix copy` and the script author wants consistent
behavior regardless of what those store paths are. Otherwise the script
author needs to be careful to filter out `.drv` ones, and then run `nix
copy` again with those paths and `--derivation`. That is not good!

>> Surprisingly low impact

Only two lines in the tests need changing, showing that the impact of
this is pretty light.

Many command, like `nix log` will continue to work with just the
derivation passed as before. This because we used to:

- Special case the drv path and replace it with it's outputs (what this
  gets rid of).

- Turn those output path *back* into the original drv path.

Now we just skip that entire round trip!

> Context

Issue #7261 lays out a broader vision for getting rid of `--derivation`,
and has this as one of its dependencies. But we can do this with or
without that.

`Installable::toDerivations` is changed to handle the case of a
`DerivedPath::Opaque` ending in `.drv`, which is new: it simply doesn't
need to do any extra work in that case. On this basis, commands like
`nix {show-derivation,log} /nix/store/...-foo.drv` still work as before,
as described above.

When testing older daemons, the post-build-hook will be run against the
old CLI, so we need the old version of the post-build-hook to support
that use-case.

Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
---
 doc/manual/src/release-notes/rl-next.md | 16 ++++++++++++
 src/libcmd/installable-derived-path.cc  | 33 +++++++++++--------------
 src/libcmd/installables.cc              |  9 ++++---
 tests/binary-cache.sh                   |  2 +-
 tests/ca/substitute.sh                  | 10 ++++++--
 tests/post-hook.sh                      |  8 +++++-
 tests/push-to-store-old.sh              | 10 ++++++++
 tests/push-to-store.sh                  |  2 +-
 8 files changed, 64 insertions(+), 26 deletions(-)
 create mode 100755 tests/push-to-store-old.sh

diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md
index 78ae99f4b..091d12b7e 100644
--- a/doc/manual/src/release-notes/rl-next.md
+++ b/doc/manual/src/release-notes/rl-next.md
@@ -1,2 +1,18 @@
 # Release X.Y (202?-??-??)
 
+* The special handling of an [installable](../command-ref/new-cli/nix.md#installables) with `.drv` suffix being interpreted as all of the given [store derivation](../glossary.md#gloss-store-derivation)'s output paths is removed, and instead taken as the literal store path that it represents.
+
+  The new `^` syntax for store paths introduced in Nix 2.13 allows explicitly referencing output paths of a derivation.
+  Using this is better and more clear than relying on the now-removed `.drv` special handling.
+
+  For example,
+  ```shell-session
+  $ nix path-info /nix/store/gzaflydcr6sb3567hap9q6srzx8ggdgg-glibc-2.33-78.drv
+  ```
+
+  now gives info about the derivation itself, while
+
+  ```shell-session
+  $ nix path-info /nix/store/gzaflydcr6sb3567hap9q6srzx8ggdgg-glibc-2.33-78.drv^*
+  ```
+  provides information about each of its outputs.
diff --git a/src/libcmd/installable-derived-path.cc b/src/libcmd/installable-derived-path.cc
index a9921b901..729dc7d31 100644
--- a/src/libcmd/installable-derived-path.cc
+++ b/src/libcmd/installable-derived-path.cc
@@ -31,27 +31,24 @@ InstallableDerivedPath InstallableDerivedPath::parse(
     ExtendedOutputsSpec extendedOutputsSpec)
 {
     auto derivedPath = std::visit(overloaded {
-        // If the user did not use ^, we treat the output more liberally.
+        // If the user did not use ^, we treat the output more
+        // liberally: we accept a symlink chain or an actual
+        // store path.
         [&](const ExtendedOutputsSpec::Default &) -> DerivedPath {
-            // First, we accept a symlink chain or an actual store path.
             auto storePath = store->followLinksToStorePath(prefix);
-            // Second, we see if the store path ends in `.drv` to decide what sort
-            // of derived path they want.
-            //
-            // This handling predates the `^` syntax. The `^*` in
-            // `/nix/store/hash-foo.drv^*` unambiguously means "do the
-            // `DerivedPath::Built` case", so plain `/nix/store/hash-foo.drv` could
-            // also unambiguously mean "do the DerivedPath::Opaque` case".
-            //
-            // Issue #7261 tracks reconsidering this `.drv` dispatching.
-            return storePath.isDerivation()
-                ? (DerivedPath) DerivedPath::Built {
-                    .drvPath = std::move(storePath),
-                    .outputs = OutputsSpec::All {},
-                }
-                : (DerivedPath) DerivedPath::Opaque {
-                    .path = std::move(storePath),
+            // Remove this prior to stabilizing the new CLI.
+            if (storePath.isDerivation()) {
+                auto oldDerivedPath = DerivedPath::Built {
+                    .drvPath = storePath,
+                    .outputs = OutputsSpec::All { },
                 };
+                warn(
+                    "The interpretation of store paths arguments ending in `.drv` recently changed. If this command is now failing try again with '%s'",
+                    oldDerivedPath.to_string(*store));
+            };
+            return DerivedPath::Opaque {
+                .path = std::move(storePath),
+            };
         },
         // If the user did use ^, we just do exactly what is written.
         [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> DerivedPath {
diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc
index 00c6f9516..90f001902 100644
--- a/src/libcmd/installables.cc
+++ b/src/libcmd/installables.cc
@@ -677,9 +677,12 @@ StorePathSet Installable::toDerivations(
         for (const auto & b : i->toDerivedPaths())
             std::visit(overloaded {
                 [&](const DerivedPath::Opaque & bo) {
-                    if (!useDeriver)
-                        throw Error("argument '%s' did not evaluate to a derivation", i->what());
-                    drvPaths.insert(getDeriver(store, *i, bo.path));
+                    drvPaths.insert(
+                        bo.path.isDerivation()
+                            ? bo.path
+                        : useDeriver
+                            ? getDeriver(store, *i, bo.path)
+                        : throw Error("argument '%s' did not evaluate to a derivation", i->what()));
                 },
                 [&](const DerivedPath::Built & bfd) {
                     drvPaths.insert(bfd.drvPath);
diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh
index 0361ac6a8..b38db8a15 100644
--- a/tests/binary-cache.sh
+++ b/tests/binary-cache.sh
@@ -23,7 +23,7 @@ nix log $outPath 2>&1 | grep 'is not available'
 nix log --substituters file://$cacheDir $outPath | grep FOO
 
 # Test copying build logs from the binary cache.
-nix store copy-log --from file://$cacheDir $(nix-store -qd $outPath)
+nix store copy-log --from file://$cacheDir $(nix-store -qd $outPath)^'*'
 nix log $outPath | grep FOO
 
 basicDownloadTests() {
diff --git a/tests/ca/substitute.sh b/tests/ca/substitute.sh
index 819f3fd85..ea981adc4 100644
--- a/tests/ca/substitute.sh
+++ b/tests/ca/substitute.sh
@@ -28,6 +28,12 @@ nix realisation info --file ./content-addressed.nix transitivelyDependentCA
 nix realisation info --file ./content-addressed.nix dependentCA
 # nix realisation info --file ./content-addressed.nix rootCA --outputs out
 
+if isDaemonNewer "2.13"; then
+    pushToStore="../push-to-store.sh"
+else
+    pushToStore="../push-to-store-old.sh"
+fi
+
 # Same thing, but
 # 1. With non-ca derivations
 # 2. Erasing the realisations on the remote store
@@ -37,7 +43,7 @@ nix realisation info --file ./content-addressed.nix dependentCA
 #
 # Regression test for #4725
 clearStore
-nix build --file ../simple.nix -L --no-link --post-build-hook ../push-to-store.sh
+nix build --file ../simple.nix -L --no-link --post-build-hook "$pushToStore"
 clearStore
 rm -r "$REMOTE_STORE_DIR/realisations"
 nix build --file ../simple.nix -L --no-link --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0
@@ -52,7 +58,7 @@ if [[ -z "$(ls "$REMOTE_STORE_DIR/realisations")" ]]; then
 fi
 
 # Test the local realisation disk cache
-buildDrvs --post-build-hook ../push-to-store.sh
+buildDrvs --post-build-hook "$pushToStore"
 clearStore
 # Add the realisations of rootCA to the cachecache
 clearCacheCache
diff --git a/tests/post-hook.sh b/tests/post-hook.sh
index 4eff5f511..0266eb15d 100644
--- a/tests/post-hook.sh
+++ b/tests/post-hook.sh
@@ -9,8 +9,14 @@ echo 'require-sigs = false' >> $NIX_CONF_DIR/nix.conf
 
 restartDaemon
 
+if isDaemonNewer "2.13"; then
+    pushToStore="$PWD/push-to-store.sh"
+else
+    pushToStore="$PWD/push-to-store-old.sh"
+fi
+
 # Build the dependencies and push them to the remote store.
-nix-build -o $TEST_ROOT/result dependencies.nix --post-build-hook $PWD/push-to-store.sh
+nix-build -o $TEST_ROOT/result dependencies.nix --post-build-hook "$pushToStore"
 
 clearStore
 
diff --git a/tests/push-to-store-old.sh b/tests/push-to-store-old.sh
new file mode 100755
index 000000000..b1495c9e2
--- /dev/null
+++ b/tests/push-to-store-old.sh
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+set -x
+set -e
+
+[ -n "$OUT_PATHS" ]
+[ -n "$DRV_PATH" ]
+
+echo Pushing "$OUT_PATHS" to "$REMOTE_STORE"
+printf "%s" "$DRV_PATH" | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
diff --git a/tests/push-to-store.sh b/tests/push-to-store.sh
index b1495c9e2..0b090e1b3 100755
--- a/tests/push-to-store.sh
+++ b/tests/push-to-store.sh
@@ -7,4 +7,4 @@ set -e
 [ -n "$DRV_PATH" ]
 
 echo Pushing "$OUT_PATHS" to "$REMOTE_STORE"
-printf "%s" "$DRV_PATH" | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
+printf "%s" "$DRV_PATH"^'*' | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs

From f86f2b973f3b07134736fd96c1eff1da184dc1af Mon Sep 17 00:00:00 2001
From: Matthew Kenigsberg <matthewkenigsberg@gmail.com>
Date: Tue, 28 Feb 2023 16:04:17 -0700
Subject: [PATCH 10/22] Always set valid in path-info --json output

Currently the valid key is only present when the path is invalid, which
makes checking path validity more complex than it should be. With this
change, the valid key can always be used to check if a path is valid
---
 src/libstore/store-api.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 601efa1cc..f32c2d30c 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -855,6 +855,7 @@ json Store::pathInfoToJSON(const StorePathSet & storePaths,
             auto info = queryPathInfo(storePath);
 
             jsonPath["path"] = printStorePath(info->path);
+            jsonPath["valid"] = true;
             jsonPath["narHash"] = info->narHash.to_string(hashBase, true);
             jsonPath["narSize"] = info->narSize;
 

From 2b801d6e3c3a3be6feb6fa2d9a0b009fa9261b45 Mon Sep 17 00:00:00 2001
From: Cole Helbling <cole.e.helbling@outlook.com>
Date: Tue, 28 Feb 2023 16:10:42 -0800
Subject: [PATCH 11/22] Check if NIX_LINK_NEW exists instead of checking that
 NIX_LINK doesn't exist

For brand new installations, neither NIX_LINK_NEW
(`$XDG_STATE_HOME/nix/profile` or `~/.local/state/nix/profile`), nor
NIX_LINK (`~/.nix-profile`) will exist.

This restores functionality to nix-env, which is relied upon by GitHub
Actions such as https://github.com/cachix/cachix-action and the Nixpkgs
EditorConfig (and other) CI.
---
 scripts/nix-profile-daemon.sh.in | 2 +-
 scripts/nix-profile.sh.in        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/nix-profile-daemon.sh.in b/scripts/nix-profile-daemon.sh.in
index 235536c65..e07e5b349 100644
--- a/scripts/nix-profile-daemon.sh.in
+++ b/scripts/nix-profile-daemon.sh.in
@@ -8,7 +8,7 @@ if [ -n "$XDG_STATE_HOME" ]; then
 else
     NIX_LINK_NEW=$HOME/.local/state/nix/profile
 fi
-if ! [ -e "$NIX_LINK" ]; then
+if [ -e "$NIX_LINK_NEW" ]; then
     NIX_LINK="$NIX_LINK_NEW"
 else
     if [ -t 2 ] && [ -e "$NIX_LINK_NEW" ]; then
diff --git a/scripts/nix-profile.sh.in b/scripts/nix-profile.sh.in
index 264d9a8e2..a9a83559b 100644
--- a/scripts/nix-profile.sh.in
+++ b/scripts/nix-profile.sh.in
@@ -8,7 +8,7 @@ if [ -n "$HOME" ] && [ -n "$USER" ]; then
     else
         NIX_LINK_NEW="$HOME/.local/state/nix/profile"
     fi
-    if ! [ -e "$NIX_LINK" ]; then
+    if [ -e "$NIX_LINK_NEW" ]; then
         NIX_LINK="$NIX_LINK_NEW"
     else
         if [ -t 2 ] && [ -e "$NIX_LINK_NEW" ]; then

From 990fbb8fefd4c2f229e4153b0be5fa5ca8dd175e Mon Sep 17 00:00:00 2001
From: Cole Helbling <cole.e.helbling@outlook.com>
Date: Tue, 28 Feb 2023 16:19:11 -0800
Subject: [PATCH 12/22] WIP: pin cachix actions to 2.13.3 to see if CI will
 pass

---
 .github/workflows/ci.yml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 325579a5b..fe0976228 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -20,6 +20,8 @@ jobs:
       with:
         fetch-depth: 0
     - uses: cachix/install-nix-action@v19
+      with:
+        install_url: https://releases.nixos.org/nix/nix-2.13.3/install
     - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV
     - uses: cachix/cachix-action@v12
       if: needs.check_secrets.outputs.cachix == 'true'
@@ -59,6 +61,8 @@ jobs:
         fetch-depth: 0
     - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV
     - uses: cachix/install-nix-action@v19
+      with:
+        install_url: https://releases.nixos.org/nix/nix-2.13.3/install
     - uses: cachix/cachix-action@v12
       with:
         name: '${{ env.CACHIX_NAME }}'
@@ -103,6 +107,8 @@ jobs:
       with:
         fetch-depth: 0
     - uses: cachix/install-nix-action@v19
+      with:
+        install_url: https://releases.nixos.org/nix/nix-2.13.3/install
     - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV
     - run: echo NIX_VERSION="$(nix --experimental-features 'nix-command flakes' eval .\#default.version | tr -d \")" >> $GITHUB_ENV
     - uses: cachix/cachix-action@v12

From 12538605fdc99be846b736080558567edbb8a393 Mon Sep 17 00:00:00 2001
From: Bob van der Linden <bobvanderlinden@gmail.com>
Date: Wed, 1 Mar 2023 07:40:44 +0100
Subject: [PATCH 13/22] nix-profile: add FIXME about using C++20 std::ranges

---
 src/nix/profile.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/nix/profile.cc b/src/nix/profile.cc
index 3464a977d..3d5c0c8a3 100644
--- a/src/nix/profile.cc
+++ b/src/nix/profile.cc
@@ -332,6 +332,8 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile
         try {
             updateProfile(manifest.build(store));
         } catch (BuildEnvFileConflictError & conflictError) {
+            // FIXME use C++20 std::ranges once macOS has it
+            //       See https://github.com/NixOS/nix/compare/3efa476c5439f8f6c1968a6ba20a31d1239c2f04..1fe5d172ece51a619e879c4b86f603d9495cc102
             auto findRefByFilePath = [&]<typename Iterator>(Iterator begin, Iterator end) {
                 for (auto it = begin; it != end; it++) {
                     auto profileElement = *it;

From b7f01a82a9b0b2c66bb5d58e1518a113d20e79c5 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 1 Mar 2023 11:09:47 -0500
Subject: [PATCH 14/22] Remove dead code `RemoteStore::sameMachine`

It has been dead code since 9747ea84b43c75f719d719673b9d0a7cb50d34e5.
---
 src/libstore/remote-store.hh     | 2 --
 src/libstore/ssh-store.cc        | 3 ---
 src/libstore/uds-remote-store.hh | 3 ---
 3 files changed, 8 deletions(-)

diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index 11d089cd2..8cd7cc822 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -38,8 +38,6 @@ class RemoteStore : public virtual RemoteStoreConfig,
 {
 public:
 
-    virtual bool sameMachine() = 0;
-
     RemoteStore(const Params & params);
 
     /* Implementations of abstract store API methods. */
diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc
index a1d4daafd..cfa063803 100644
--- a/src/libstore/ssh-store.cc
+++ b/src/libstore/ssh-store.cc
@@ -49,9 +49,6 @@ public:
         return *uriSchemes().begin() + "://" + host;
     }
 
-    bool sameMachine() override
-    { return false; }
-
     // FIXME extend daemon protocol, move implementation to RemoteStore
     std::optional<std::string> getBuildLogExact(const StorePath & path) override
     { unsupported("getBuildLogExact"); }
diff --git a/src/libstore/uds-remote-store.hh b/src/libstore/uds-remote-store.hh
index f8dfcca70..d31a4d592 100644
--- a/src/libstore/uds-remote-store.hh
+++ b/src/libstore/uds-remote-store.hh
@@ -29,9 +29,6 @@ public:
     static std::set<std::string> uriSchemes()
     { return {"unix"}; }
 
-    bool sameMachine() override
-    { return true; }
-
     ref<FSAccessor> getFSAccessor() override
     { return LocalFSStore::getFSAccessor(); }
 

From f9443143aee547df59295d9b3202a148998c5621 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 1 Mar 2023 18:02:26 -0500
Subject: [PATCH 15/22] Remove needless `--experimental-feature` in a CA drvs
 test

This is already blanket enabled for these tests
---
 tests/ca/build.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/ca/build.sh b/tests/ca/build.sh
index cc225c6c8..8783525e7 100644
--- a/tests/ca/build.sh
+++ b/tests/ca/build.sh
@@ -2,14 +2,14 @@
 
 source common.sh
 
-drv=$(nix-instantiate --experimental-features ca-derivations ./content-addressed.nix -A rootCA --arg seed 1)
-nix --experimental-features 'nix-command ca-derivations' show-derivation "$drv" --arg seed 1
+drv=$(nix-instantiate ./content-addressed.nix -A rootCA --arg seed 1)
+nix show-derivation "$drv" --arg seed 1
 
 buildAttr () {
     local derivationPath=$1
     local seedValue=$2
     shift; shift
-    local args=("--experimental-features" "ca-derivations" "./content-addressed.nix" "-A" "$derivationPath" --arg seed "$seedValue" "--no-out-link")
+    local args=("./content-addressed.nix" "-A" "$derivationPath" --arg seed "$seedValue" "--no-out-link")
     args+=("$@")
     nix-build "${args[@]}"
 }
@@ -46,17 +46,17 @@ testCutoff () {
 }
 
 testGC () {
-    nix-instantiate --experimental-features ca-derivations ./content-addressed.nix -A rootCA --arg seed 5
-    nix-collect-garbage --experimental-features ca-derivations --option keep-derivations true
+    nix-instantiate ./content-addressed.nix -A rootCA --arg seed 5
+    nix-collect-garbage --option keep-derivations true
     clearStore
     buildAttr rootCA 1 --out-link $TEST_ROOT/rootCA
-    nix-collect-garbage --experimental-features ca-derivations
+    nix-collect-garbage
     buildAttr rootCA 1 -j0
 }
 
 testNixCommand () {
     clearStore
-    nix build --experimental-features 'nix-command ca-derivations' --file ./content-addressed.nix --no-link
+    nix build --file ./content-addressed.nix --no-link
 }
 
 # Regression test for https://github.com/NixOS/nix/issues/4775

From 378508a877386c6a12a2778fb7abd93a687556e5 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Tue, 28 Feb 2023 18:16:07 +0100
Subject: [PATCH 16/22] Bump version

---
 .version                       | 2 +-
 maintainers/release-process.md | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/.version b/.version
index 575a07b9f..c910885a0 100644
--- a/.version
+++ b/.version
@@ -1 +1 @@
-2.14.0
\ No newline at end of file
+2.15.0
\ No newline at end of file
diff --git a/maintainers/release-process.md b/maintainers/release-process.md
index b52d218f5..ec9e96489 100644
--- a/maintainers/release-process.md
+++ b/maintainers/release-process.md
@@ -123,6 +123,7 @@ release:
   `/home/eelco/Dev/nixpkgs-pristine`.
 
   TODO: trigger nixos.org netlify: https://docs.netlify.com/configure-builds/build-hooks/
+
 * Prepare for the next point release by editing `.version` to
   e.g.
 
@@ -152,7 +153,7 @@ release:
   from the previous milestone, and close the previous milestone. Set
   the date for the next milestone 6 weeks from now.
 
-* Create a backport label
+* Create a backport label.
 
 * Post an [announcement on Discourse](https://discourse.nixos.org/c/announcements/8), including the contents of
   `rl-$VERSION.md`.

From c13cbd20ab97e46eeb39939494d137a2f831b5e1 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 2 Mar 2023 14:46:28 +0100
Subject: [PATCH 17/22] Logger::writeToStdout(): Use writeFull()

This ensures that write errors do not get ignored.
---
 src/libutil/logging.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc
index 904ba6ebe..56bdaf87a 100644
--- a/src/libutil/logging.cc
+++ b/src/libutil/logging.cc
@@ -32,7 +32,8 @@ void Logger::warn(const std::string & msg)
 
 void Logger::writeToStdout(std::string_view s)
 {
-    std::cout << s << "\n";
+    writeFull(STDOUT_FILENO, s);
+    writeFull(STDOUT_FILENO, "\n");
 }
 
 class SimpleLogger : public Logger
@@ -84,7 +85,7 @@ public:
 
     void startActivity(ActivityId act, Verbosity lvl, ActivityType type,
         const std::string & s, const Fields & fields, ActivityId parent)
-    override
+        override
     {
         if (lvl <= verbosity && !s.empty())
             log(lvl, s + "...");

From 989b823ac5265a7738396566fa0792f3cb55160e Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 2 Mar 2023 14:51:14 +0100
Subject: [PATCH 18/22] nix store cat: Use writeFull()

Fixes #7939.
---
 src/nix/cat.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nix/cat.cc b/src/nix/cat.cc
index 6420a0f79..60aa66ce0 100644
--- a/src/nix/cat.cc
+++ b/src/nix/cat.cc
@@ -17,7 +17,7 @@ struct MixCat : virtual Args
         if (st.type != FSAccessor::Type::tRegular)
             throw Error("path '%1%' is not a regular file", path);
 
-        std::cout << accessor->readFile(path);
+        writeFull(STDOUT_FILENO, accessor->readFile(path));
     }
 };
 

From 09f5975c6a0d8706b22f15e4c5998018e56484ae Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 2 Mar 2023 14:52:37 +0100
Subject: [PATCH 19/22] Logger::cout: Use fmt()

This ensures that in cout(s), 's' does not get interpreted as a format
string.
---
 src/libutil/logging.hh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh
index 4642c49f7..a68edd15a 100644
--- a/src/libutil/logging.hh
+++ b/src/libutil/logging.hh
@@ -102,11 +102,9 @@ public:
     virtual void writeToStdout(std::string_view s);
 
     template<typename... Args>
-    inline void cout(const std::string & fs, const Args & ... args)
+    inline void cout(const Args & ... args)
     {
-        boost::format f(fs);
-        formatHelper(f, args...);
-        writeToStdout(f.str());
+        writeToStdout(fmt(args...));
     }
 
     virtual std::optional<char> ask(std::string_view s)

From 1a825b6919e9a3a98caec58192bb89dfb4e5bcfa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?=
 <theophane.hufschmitt@tweag.io>
Date: Thu, 2 Mar 2023 14:59:15 +0100
Subject: [PATCH 20/22] Log the decompressed body in case of http errors

Don't show the users the raw (possibly compressed) error message as
everyone isn't able to decompress brotli in their brain.
---
 src/libstore/filetransfer.cc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc
index 1c8676a59..6a4778d1f 100644
--- a/src/libstore/filetransfer.cc
+++ b/src/libstore/filetransfer.cc
@@ -88,6 +88,10 @@ struct curlFileTransfer : public FileTransfer
                 {request.uri}, request.parentAct)
             , callback(std::move(callback))
             , finalSink([this](std::string_view data) {
+                if (errorSink) {
+                    (*errorSink)(data);
+                }
+
                 if (this->request.dataCallback) {
                     auto httpStatus = getHTTPStatus();
 
@@ -163,8 +167,6 @@ struct curlFileTransfer : public FileTransfer
                     }
                 }
 
-                if (errorSink)
-                    (*errorSink)({(char *) contents, realSize});
                 (*decompressionSink)({(char *) contents, realSize});
 
                 return realSize;

From b69a73a2305aaee2a4da73da601dbde3f1ddd7a6 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 2 Mar 2023 15:02:24 +0100
Subject: [PATCH 21/22] Get rid of some unchecked calls to std::cout

---
 src/nix/build.cc                  |  4 ++--
 src/nix/describe-stores.cc        |  2 +-
 src/nix/diff-closures.cc          |  2 +-
 src/nix/eval.cc                   |  4 ++--
 src/nix/flake.cc                  |  2 +-
 src/nix/log.cc                    |  2 +-
 src/nix/ls.cc                     |  2 +-
 src/nix/main.cc                   | 12 ++++++------
 src/nix/make-content-addressed.cc |  2 +-
 src/nix/prefetch.cc               |  4 ++--
 src/nix/profile.cc                | 16 ++++++++--------
 src/nix/realisation.cc            | 12 +++++-------
 src/nix/search.cc                 |  5 ++---
 src/nix/show-derivation.cc        |  2 +-
 src/nix/sigs.cc                   |  4 ++--
 15 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/src/nix/build.cc b/src/nix/build.cc
index 12b22d999..f4f2ec81d 100644
--- a/src/nix/build.cc
+++ b/src/nix/build.cc
@@ -139,11 +139,11 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile
             for (auto & buildable : buildables) {
                 std::visit(overloaded {
                     [&](const BuiltPath::Opaque & bo) {
-                        std::cout << store->printStorePath(bo.path) << std::endl;
+                        logger->cout(store->printStorePath(bo.path));
                     },
                     [&](const BuiltPath::Built & bfd) {
                         for (auto & output : bfd.outputs) {
-                            std::cout << store->printStorePath(output.second) << std::endl;
+                            logger->cout(store->printStorePath(output.second));
                         }
                     },
                 }, buildable.path.raw());
diff --git a/src/nix/describe-stores.cc b/src/nix/describe-stores.cc
index 1dd384c0e..eafcedd1f 100644
--- a/src/nix/describe-stores.cc
+++ b/src/nix/describe-stores.cc
@@ -25,7 +25,7 @@ struct CmdDescribeStores : Command, MixJSON
             res[storeName] = storeConfig->toJSON();
         }
         if (json) {
-            std::cout << res;
+            logger->cout("%s", res);
         } else {
             for (auto & [storeName, storeConfig] : res.items()) {
                 std::cout << "## " << storeName << std::endl << std::endl;
diff --git a/src/nix/diff-closures.cc b/src/nix/diff-closures.cc
index 3489cc132..c7c37b66f 100644
--- a/src/nix/diff-closures.cc
+++ b/src/nix/diff-closures.cc
@@ -97,7 +97,7 @@ void printClosureDiff(
                 items.push_back(fmt("%s → %s", showVersions(removed), showVersions(added)));
             if (showDelta)
                 items.push_back(fmt("%s%+.1f KiB" ANSI_NORMAL, sizeDelta > 0 ? ANSI_RED : ANSI_GREEN, sizeDelta / 1024.0));
-            std::cout << fmt("%s%s: %s\n", indent, name, concatStringsSep(", ", items));
+            logger->cout("%s%s: %s", indent, name, concatStringsSep(", ", items));
         }
     }
 }
diff --git a/src/nix/eval.cc b/src/nix/eval.cc
index a579213fd..209fd3ed2 100644
--- a/src/nix/eval.cc
+++ b/src/nix/eval.cc
@@ -112,11 +112,11 @@ struct CmdEval : MixJSON, InstallableCommand, MixReadOnlyOption
 
         else if (raw) {
             stopProgressBar();
-            std::cout << *state->coerceToString(noPos, *v, context, "while generating the eval command output");
+            writeFull(STDOUT_FILENO, *state->coerceToString(noPos, *v, context, "while generating the eval command output"));
         }
 
         else if (json) {
-            std::cout << printValueAsJSON(*state, true, *v, pos, context, false).dump() << std::endl;
+            logger->cout("%s", printValueAsJSON(*state, true, *v, pos, context, false));
         }
 
         else {
diff --git a/src/nix/flake.cc b/src/nix/flake.cc
index 053a9c9e1..3fe093fc7 100644
--- a/src/nix/flake.cc
+++ b/src/nix/flake.cc
@@ -952,7 +952,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun
                 {"path", store->printStorePath(flake.flake.sourceInfo->storePath)},
                 {"inputs", traverse(*flake.lockFile.root)},
             };
-            std::cout << jsonRoot.dump() << std::endl;
+            logger->cout("%s", jsonRoot);
         } else {
             traverse(*flake.lockFile.root);
         }
diff --git a/src/nix/log.cc b/src/nix/log.cc
index a0598ca13..0c9f778f0 100644
--- a/src/nix/log.cc
+++ b/src/nix/log.cc
@@ -53,7 +53,7 @@ struct CmdLog : InstallableCommand
             if (!log) continue;
             stopProgressBar();
             printInfo("got build log for '%s' from '%s'", installable->what(), logSub.getUri());
-            std::cout << *log;
+            writeFull(STDOUT_FILENO, *log);
             return;
         }
 
diff --git a/src/nix/ls.cc b/src/nix/ls.cc
index e964b01b3..c990a303c 100644
--- a/src/nix/ls.cc
+++ b/src/nix/ls.cc
@@ -93,7 +93,7 @@ struct MixLs : virtual Args, MixJSON
         if (json) {
             if (showDirectory)
                 throw UsageError("'--directory' is useless with '--json'");
-            std::cout << listNar(accessor, path, recursive);
+            logger->cout("%s", listNar(accessor, path, recursive));
         } else
             listText(accessor);
     }
diff --git a/src/nix/main.cc b/src/nix/main.cc
index d3d2f5b16..53bf649d4 100644
--- a/src/nix/main.cc
+++ b/src/nix/main.cc
@@ -292,7 +292,7 @@ void mainWrapped(int argc, char * * argv)
     NixArgs args;
 
     if (argc == 2 && std::string(argv[1]) == "__dump-args") {
-        std::cout << args.toJSON().dump() << "\n";
+        logger->cout("%s", args.toJSON());
         return;
     }
 
@@ -312,7 +312,7 @@ void mainWrapped(int argc, char * * argv)
             b["doc"] = trim(stripIndentation(primOp->doc));
             res[state.symbols[builtin.name]] = std::move(b);
         }
-        std::cout << res.dump() << "\n";
+        logger->cout("%s", res);
         return;
     }
 
@@ -321,14 +321,14 @@ void mainWrapped(int argc, char * * argv)
         if (completions) {
             switch (completionType) {
             case ctNormal:
-                std::cout << "normal\n"; break;
+                logger->cout("normal"); break;
             case ctFilenames:
-                std::cout << "filenames\n"; break;
+                logger->cout("filenames"); break;
             case ctAttrs:
-                std::cout << "attrs\n"; break;
+                logger->cout("attrs"); break;
             }
             for (auto & s : *completions)
-                std::cout << s.completion << "\t" << trim(s.description) << "\n";
+                logger->cout(s.completion + "\t" + trim(s.description));
         }
     });
 
diff --git a/src/nix/make-content-addressed.cc b/src/nix/make-content-addressed.cc
index d86b90fc7..6693c55ac 100644
--- a/src/nix/make-content-addressed.cc
+++ b/src/nix/make-content-addressed.cc
@@ -45,7 +45,7 @@ struct CmdMakeContentAddressed : virtual CopyCommand, virtual StorePathsCommand,
             }
             auto json = json::object();
             json["rewrites"] = jsonRewrites;
-            std::cout << json.dump();
+            logger->cout("%s", json);
         } else {
             for (auto & path : storePaths) {
                 auto i = remappings.find(path);
diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc
index fc3823406..51c8a3319 100644
--- a/src/nix/prefetch.cc
+++ b/src/nix/prefetch.cc
@@ -234,9 +234,9 @@ static int main_nix_prefetch_url(int argc, char * * argv)
         if (!printPath)
             printInfo("path is '%s'", store->printStorePath(storePath));
 
-        std::cout << printHash16or32(hash) << std::endl;
+        logger->cout(printHash16or32(hash));
         if (printPath)
-            std::cout << store->printStorePath(storePath) << std::endl;
+            logger->cout(store->printStorePath(storePath));
 
         return 0;
     }
diff --git a/src/nix/profile.cc b/src/nix/profile.cc
index 711fbe2f0..eef33b3d9 100644
--- a/src/nix/profile.cc
+++ b/src/nix/profile.cc
@@ -228,12 +228,12 @@ struct ProfileManifest
 
         while (i != prevElems.end() || j != curElems.end()) {
             if (j != curElems.end() && (i == prevElems.end() || i->describe() > j->describe())) {
-                std::cout << fmt("%s%s: ∅ -> %s\n", indent, j->describe(), j->versions());
+                logger->cout("%s%s: ∅ -> %s", indent, j->describe(), j->versions());
                 changes = true;
                 ++j;
             }
             else if (i != prevElems.end() && (j == curElems.end() || i->describe() < j->describe())) {
-                std::cout << fmt("%s%s: %s -> ∅\n", indent, i->describe(), i->versions());
+                logger->cout("%s%s: %s -> ∅", indent, i->describe(), i->versions());
                 changes = true;
                 ++i;
             }
@@ -241,7 +241,7 @@ struct ProfileManifest
                 auto v1 = i->versions();
                 auto v2 = j->versions();
                 if (v1 != v2) {
-                    std::cout << fmt("%s%s: %s -> %s\n", indent, i->describe(), v1, v2);
+                    logger->cout("%s%s: %s -> %s", indent, i->describe(), v1, v2);
                     changes = true;
                 }
                 ++i;
@@ -250,7 +250,7 @@ struct ProfileManifest
         }
 
         if (!changes)
-            std::cout << fmt("%sNo changes.\n", indent);
+            logger->cout("%sNo changes.", indent);
     }
 };
 
@@ -640,9 +640,9 @@ struct CmdProfileDiffClosures : virtual StoreCommand, MixDefaultProfile
 
         for (auto & gen : gens) {
             if (prevGen) {
-                if (!first) std::cout << "\n";
+                if (!first) logger->cout("");
                 first = false;
-                std::cout << fmt("Version %d -> %d:\n", prevGen->number, gen.number);
+                logger->cout("Version %d -> %d:", prevGen->number, gen.number);
                 printClosureDiff(store,
                     store->followLinksToStorePath(prevGen->path),
                     store->followLinksToStorePath(gen.path),
@@ -678,10 +678,10 @@ struct CmdProfileHistory : virtual StoreCommand, EvalCommand, MixDefaultProfile
         for (auto & gen : gens) {
             ProfileManifest manifest(*getEvalState(), gen.path);
 
-            if (!first) std::cout << "\n";
+            if (!first) logger->cout("");
             first = false;
 
-            std::cout << fmt("Version %s%d" ANSI_NORMAL " (%s)%s:\n",
+            logger->cout("Version %s%d" ANSI_NORMAL " (%s)%s:",
                 gen.number == curGen ? ANSI_GREEN : ANSI_BOLD,
                 gen.number,
                 std::put_time(std::gmtime(&gen.creationTime), "%Y-%m-%d"),
diff --git a/src/nix/realisation.cc b/src/nix/realisation.cc
index c9a7157cd..0d3466515 100644
--- a/src/nix/realisation.cc
+++ b/src/nix/realisation.cc
@@ -65,18 +65,16 @@ struct CmdRealisationInfo : BuiltPathsCommand, MixJSON
 
                 res.push_back(currentPath);
             }
-            std::cout << res.dump();
+            logger->cout("%s", res);
         }
         else {
             for (auto & path : realisations) {
                 if (auto realisation = std::get_if<Realisation>(&path.raw)) {
-                    std::cout <<
-                        realisation->id.to_string() << " " <<
-                        store->printStorePath(realisation->outPath);
+                    logger->cout("%s %s",
+                        realisation->id.to_string(),
+                        store->printStorePath(realisation->outPath));
                 } else
-                    std::cout << store->printStorePath(path.path());
-
-                std::cout << std::endl;
+                    logger->cout("%s", store->printStorePath(path.path()));
             }
         }
     }
diff --git a/src/nix/search.cc b/src/nix/search.cc
index 4fa1e7837..2e38f7e4b 100644
--- a/src/nix/search.cc
+++ b/src/nix/search.cc
@@ -196,9 +196,8 @@ struct CmdSearch : InstallableCommand, MixJSON
         for (auto & cursor : installable->getCursors(*state))
             visit(*cursor, cursor->getAttrPath(), true);
 
-        if (json) {
-            std::cout << jsonOut->dump() << std::endl;
-        }
+        if (json)
+            logger->cout("%s", *jsonOut);
 
         if (!json && !results)
             throw Error("no results for the given search term(s)!");
diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc
index d1a516cad..520e8b1ce 100644
--- a/src/nix/show-derivation.cc
+++ b/src/nix/show-derivation.cc
@@ -57,7 +57,7 @@ struct CmdShowDerivation : InstallablesCommand
             jsonRoot[store->printStorePath(drvPath)] =
                 store->readDerivation(drvPath).toJSON(*store);
         }
-        std::cout << jsonRoot.dump(2) << std::endl;
+        logger->cout(jsonRoot.dump(2));
     }
 };
 
diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc
index 3d659d6d2..ee27e3725 100644
--- a/src/nix/sigs.cc
+++ b/src/nix/sigs.cc
@@ -173,7 +173,7 @@ struct CmdKeyGenerateSecret : Command
         if (!keyName)
             throw UsageError("required argument '--key-name' is missing");
 
-        std::cout << SecretKey::generate(*keyName).to_string();
+        writeFull(STDOUT_FILENO, SecretKey::generate(*keyName).to_string());
     }
 };
 
@@ -194,7 +194,7 @@ struct CmdKeyConvertSecretToPublic : Command
     void run() override
     {
         SecretKey secretKey(drainFD(STDIN_FILENO));
-        std::cout << secretKey.toPublicKey().to_string();
+        writeFull(STDOUT_FILENO, secretKey.toPublicKey().to_string());
     }
 };
 

From 9c79ce353d1714f948ba116f3fc696639aaacbf6 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 2 Mar 2023 16:11:49 +0100
Subject: [PATCH 22/22] Fix 'make check' inside 'nix develop'

---
 flake.nix | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/flake.nix b/flake.nix
index 2000c2d73..5e0504a59 100644
--- a/flake.nix
+++ b/flake.nix
@@ -96,6 +96,7 @@
           ++ lib.optionals (stdenv.isLinux && !(isStatic && stdenv.system == "aarch64-linux")) [
             "LDFLAGS=-fuse-ld=gold"
           ];
+
         testConfigureFlags = [
           "CXXFLAGS=-I${lib.getDev rapidcheck}/extras/gtest/include"
         ];
@@ -651,7 +652,7 @@
 
             buildInputs = buildDeps ++ propagatedDeps ++ awsDeps ++ checkDeps;
 
-            inherit configureFlags;
+            configureFlags = configureFlags ++ testConfigureFlags;
 
             enableParallelBuilding = true;