From 7130f0a3a6455cf27863f0bbccf0974cfcc82fa2 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 17 Jun 2020 04:04:41 +0000
Subject: [PATCH 01/13] Don't need abstract `struct Derivation` in local-store

---
 src/libstore/local-store.hh | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index e17cc45ae..872500957 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -23,9 +23,6 @@ namespace nix {
 const int nixSchemaVersion = 10;
 
 
-struct Derivation;
-
-
 struct OptimiseStats
 {
     unsigned long filesLinked = 0;

From 18493fd9c48676ab26854739db67ac5d76ff9347 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 17 Jun 2020 03:56:48 +0000
Subject: [PATCH 02/13] Move some Store functions from derivations.cc to
 store-api.cc

This further continues with the dependency inverstion. Also I just went
ahead and exposed `parseDerivation`: it seems like the more proper
building block, and not a bad thing to expose if we are trying to be
less wedded to drv files on disk anywas.
---
 src/libexpr/primops.cc      |  2 +-
 src/libstore/derivations.cc | 31 +------------------------------
 src/libstore/derivations.hh |  2 +-
 src/libstore/store-api.cc   | 20 +++++++++++++++++++-
 src/nix/repl.cc             | 28 ++++++++++++++++------------
 5 files changed, 38 insertions(+), 45 deletions(-)

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 907f15246..0c2371c43 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -103,7 +103,7 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args
 
     // FIXME
     if (state.store->isStorePath(path) && state.store->isValidPath(state.store->parseStorePath(path)) && isDerivation(path)) {
-        Derivation drv = readDerivation(*state.store, realPath);
+        Derivation drv = state.store->readDerivation(state.store->parseStorePath(path));
         Value & w = *state.allocValue();
         state.mkAttrs(w, 3 + drv.outputs.size());
         Value * v2 = state.allocAttr(w, state.sDrvPath);
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index b95f7bfdc..ea30813fa 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -3,7 +3,6 @@
 #include "globals.hh"
 #include "util.hh"
 #include "worker-protocol.hh"
-#include "fs-accessor.hh"
 #include "istringstream_nocopy.hh"
 
 namespace nix {
@@ -120,7 +119,7 @@ static StringSet parseStrings(std::istream & str, bool arePaths)
 }
 
 
-static Derivation parseDerivation(const Store & store, const string & s)
+Derivation parseDerivation(const Store & store, const string & s)
 {
     Derivation drv;
     istringstream_nocopy str(s);
@@ -173,34 +172,6 @@ static Derivation parseDerivation(const Store & store, const string & s)
 }
 
 
-Derivation readDerivation(const Store & store, const Path & drvPath)
-{
-    try {
-        return parseDerivation(store, readFile(drvPath));
-    } catch (FormatError & e) {
-        throw Error("error parsing derivation '%1%': %2%", drvPath, e.msg());
-    }
-}
-
-
-Derivation Store::derivationFromPath(const StorePath & drvPath)
-{
-    ensurePath(drvPath);
-    return readDerivation(drvPath);
-}
-
-
-Derivation Store::readDerivation(const StorePath & drvPath)
-{
-    auto accessor = getFSAccessor();
-    try {
-        return parseDerivation(*this, accessor->readFile(printStorePath(drvPath)));
-    } catch (FormatError & e) {
-        throw Error("error parsing derivation '%s': %s", printStorePath(drvPath), e.msg());
-    }
-}
-
-
 static void printString(string & res, std::string_view s)
 {
     char buf[s.size() * 2 + 2];
diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
index 856aae59f..2adbf6b94 100644
--- a/src/libstore/derivations.hh
+++ b/src/libstore/derivations.hh
@@ -77,7 +77,7 @@ StorePath writeDerivation(ref<Store> store,
     const Derivation & drv, std::string_view name, RepairFlag repair = NoRepair);
 
 /* Read a derivation from a file. */
-Derivation readDerivation(const Store & store, const Path & drvPath);
+Derivation parseDerivation(const Store & store, const string & s);
 
 // FIXME: remove
 bool isDerivation(const string & fileName);
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index aae227bae..0147445ff 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -1,11 +1,11 @@
 #include "crypto.hh"
+#include "fs-accessor.hh"
 #include "globals.hh"
 #include "store-api.hh"
 #include "util.hh"
 #include "nar-info-disk-cache.hh"
 #include "thread-pool.hh"
 #include "json.hh"
-#include "derivations.hh"
 #include "url.hh"
 
 #include <future>
@@ -821,6 +821,24 @@ std::string makeFixedOutputCA(FileIngestionMethod recursive, const Hash & hash)
 }
 
 
+Derivation Store::derivationFromPath(const StorePath & drvPath)
+{
+    ensurePath(drvPath);
+    return readDerivation(drvPath);
+}
+
+
+Derivation Store::readDerivation(const StorePath & drvPath)
+{
+    auto accessor = getFSAccessor();
+    try {
+        return parseDerivation(*this, accessor->readFile(printStorePath(drvPath)));
+    } catch (FormatError & e) {
+        throw Error("error parsing derivation '%s': %s", printStorePath(drvPath), e.msg());
+    }
+}
+
+
 }
 
 
diff --git a/src/nix/repl.cc b/src/nix/repl.cc
index 4bcaaeebf..b46f7015f 100644
--- a/src/nix/repl.cc
+++ b/src/nix/repl.cc
@@ -66,7 +66,7 @@ struct NixRepl : gc
     void mainLoop(const std::vector<std::string> & files);
     StringSet completePrefix(string prefix);
     bool getLine(string & input, const std::string &prompt);
-    Path getDerivationPath(Value & v);
+    StorePath getDerivationPath(Value & v);
     bool processLine(string line);
     void loadFile(const Path & path);
     void initEnv();
@@ -377,13 +377,16 @@ bool isVarName(const string & s)
 }
 
 
-Path NixRepl::getDerivationPath(Value & v) {
+StorePath NixRepl::getDerivationPath(Value & v) {
     auto drvInfo = getDerivation(*state, v, false);
     if (!drvInfo)
         throw Error("expression does not evaluate to a derivation, so I can't build it");
-    Path drvPath = drvInfo->queryDrvPath();
-    if (drvPath == "" || !state->store->isValidPath(state->store->parseStorePath(drvPath)))
-        throw Error("expression did not evaluate to a valid derivation");
+    Path drvPathRaw = drvInfo->queryDrvPath();
+    if (drvPathRaw == "")
+        throw Error("expression did not evaluate to a valid derivation (no drv path)");
+    StorePath drvPath = state->store->parseStorePath(drvPathRaw);
+    if (!state->store->isValidPath(drvPath))
+        throw Error("expression did not evaluate to a valid derivation (invalid drv path)");
     return drvPath;
 }
 
@@ -476,29 +479,30 @@ bool NixRepl::processLine(string line)
         evalString("drv: (import <nixpkgs> {}).runCommand \"shell\" { buildInputs = [ drv ]; } \"\"", f);
         state->callFunction(f, v, result, Pos());
 
-        Path drvPath = getDerivationPath(result);
-        runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPath});
+        StorePath drvPath = getDerivationPath(result);
+        runProgram(settings.nixBinDir + "/nix-shell", Strings{state->store->printStorePath(drvPath)});
     }
 
     else if (command == ":b" || command == ":i" || command == ":s") {
         Value v;
         evalString(arg, v);
-        Path drvPath = getDerivationPath(v);
+        StorePath drvPath = getDerivationPath(v);
+        Path drvPathRaw = state->store->printStorePath(drvPath);
 
         if (command == ":b") {
             /* We could do the build in this process using buildPaths(),
                but doing it in a child makes it easier to recover from
                problems / SIGINT. */
-            if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPath}) == 0) {
-                auto drv = readDerivation(*state->store, drvPath);
+            if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPathRaw}) == 0) {
+                auto drv = state->store->readDerivation(drvPath);
                 std::cout << std::endl << "this derivation produced the following outputs:" << std::endl;
                 for (auto & i : drv.outputs)
                     std::cout << fmt("  %s -> %s\n", i.first, state->store->printStorePath(i.second.path));
             }
         } else if (command == ":i") {
-            runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPath});
+            runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPathRaw});
         } else {
-            runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPath});
+            runProgram(settings.nixBinDir + "/nix-shell", Strings{drvPathRaw});
         }
     }
 

From f1c7746eb407258d77b2f7fffec0e5d4facf516a Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Sun, 5 Jul 2020 21:39:04 +0000
Subject: [PATCH 03/13] See if setting -std=c++17 for perl bindings helps

---
 perl/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Makefile b/perl/Makefile
index 7ddb0cf69..259ed7dc3 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -1,6 +1,6 @@
 makefiles = local.mk
 
-GLOBAL_CXXFLAGS += -g -Wall
+GLOBAL_CXXFLAGS += -g -Wall -std=c++17
 
 -include Makefile.config
 

From e3b394b6e8420795c1976195184921df2c2b3e83 Mon Sep 17 00:00:00 2001
From: Carlo Nucera <carlo.nucera@protonmail.com>
Date: Thu, 16 Jul 2020 09:36:02 -0400
Subject: [PATCH 04/13] Small namespace fix

---
 src/libstore/derivations.hh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
index d32793b6a..d170d7223 100644
--- a/src/libstore/derivations.hh
+++ b/src/libstore/derivations.hh
@@ -76,7 +76,7 @@ StorePath writeDerivation(ref<Store> store,
     const Derivation & drv, std::string_view name, RepairFlag repair = NoRepair);
 
 /* Read a derivation from a file. */
-Derivation parseDerivation(const Store & store, string && s);
+Derivation parseDerivation(const Store & store, std::string && s);
 
 // FIXME: remove
 bool isDerivation(const string & fileName);

From 6ccfdb79c7779f9eedeea73a21df694551c9893e Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Tue, 21 Jul 2020 23:38:18 +0200
Subject: [PATCH 05/13] libutil/logging: extend `internal-json` logger to make
 it more machine-readable

The new error-format is pretty nice from a UX point-of-view, however
it's fairly hard to parse the output e.g. for editor plugins such as
vim-ale[1] that use `nix-instantiate --parse` to determine syntax errors in
Nix expression files.

This patch extends the `internal-json` logger by adding the fields
`line`, `column` and `file` to easily locate an error in a file and the
field `raw_msg` which contains the error-message itself without
code-lines and additional helpers.

An exemplary output may look like this:

```
[nix-shell]$ ./inst/bin/nix-instantiate ~/test.nix --log-format minimal
{"action":"msg","column":1,"file":"/home/ma27/test.nix","level":0,"line":4,"raw_msg":"syntax error, unexpected IF, expecting $end","msg":"<full error-msg with code-lines etc>"}
```

[1] https://github.com/dense-analysis/ale
---
 doc/manual/command-ref/opt-common.xml |  9 ++++++++-
 src/libutil/logging.cc                | 27 +++++++++++++++++++++++++++
 src/libutil/tests/logging.cc          | 18 ++++++++++++++++++
 3 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/doc/manual/command-ref/opt-common.xml b/doc/manual/command-ref/opt-common.xml
index a68eef1d0..093bc2526 100644
--- a/doc/manual/command-ref/opt-common.xml
+++ b/doc/manual/command-ref/opt-common.xml
@@ -106,7 +106,14 @@
     </varlistentry>
 
     <varlistentry><term>internal-json</term>
-    <listitem><para>Outputs the logs in a structured manner. NOTE: the json schema is not guarantees to be stable between releases.</para></listitem>
+     <listitem>
+      <para>Outputs the logs in a structured manner.</para>
+      <warning>
+       <para>
+        While the schema itself is relatively stable, the format of the error-messages (namely of the <literal>msg</literal>-field) can change between several releases.
+       </para>
+      </warning>
+     </listitem>
     </varlistentry>
 
     <varlistentry><term>bar</term>
diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc
index 832aee783..cbbf64395 100644
--- a/src/libutil/logging.cc
+++ b/src/libutil/logging.cc
@@ -184,6 +184,33 @@ struct JSONLogger : Logger {
         json["action"] = "msg";
         json["level"] = ei.level;
         json["msg"] = oss.str();
+        json["raw_msg"] = ei.hint->str();
+
+        if (ei.errPos.has_value() && (*ei.errPos)) {
+            json["line"] = ei.errPos->line;
+            json["column"] = ei.errPos->column;
+            json["file"] = ei.errPos->file;
+        } else {
+            json["line"] = nullptr;
+            json["column"] = nullptr;
+            json["file"] = nullptr;
+        }
+
+        if (loggerSettings.showTrace.get() && !ei.traces.empty()) {
+            nlohmann::json traces = nlohmann::json::array();
+            for (auto iter = ei.traces.rbegin(); iter != ei.traces.rend(); ++iter) {
+                nlohmann::json stackFrame;
+                stackFrame["raw_msg"] = iter->hint.str();
+                if (iter->pos.has_value() && (*iter->pos)) {
+                    stackFrame["line"] = iter->pos->line;
+                    stackFrame["column"] = iter->pos->column;
+                    stackFrame["file"] = iter->pos->file;
+                }
+                traces.push_back(stackFrame);
+            }
+
+            json["trace"] = traces;
+        }
 
         write(json);
     }
diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc
index ad588055f..7e53f17c6 100644
--- a/src/libutil/tests/logging.cc
+++ b/src/libutil/tests/logging.cc
@@ -34,6 +34,24 @@ namespace nix {
         }
     }
 
+    TEST(logEI, jsonOutput) {
+        SymbolTable testTable;
+        auto problem_file = testTable.create("random.nix");
+        testing::internal::CaptureStderr();
+
+        makeJSONLogger(*logger)->logEI({
+                .name = "error name",
+                .description = "error without any code lines.",
+                .hint = hintfmt("this hint has %1% templated %2%!!",
+                    "yellow",
+                    "values"),
+                .errPos = Pos(foFile, problem_file, 02, 13)
+            });
+
+        auto str = testing::internal::GetCapturedStderr();
+        ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nopening file '\x1B[33;1mrandom.nix\x1B[0m': \x1B[33;1mNo such file or directory\x1B[0m\n@nix {\"action\":\"msg\",\"column\":13,\"file\":\"random.nix\",\"level\":0,\"line\":2,\"msg\":\"\\u001b[31;1merror:\\u001b[0m\\u001b[34;1m --- error name --- error-unit-test\\u001b[0m\\n\\u001b[34;1mat: \\u001b[33;1m(2:13)\\u001b[34;1m in file: \\u001b[0mrandom.nix\\n\\nerror without any code lines.\\n\\nthis hint has \\u001b[33;1myellow\\u001b[0m templated \\u001b[33;1mvalues\\u001b[0m!!\",\"raw_msg\":\"this hint has \\u001b[33;1myellow\\u001b[0m templated \\u001b[33;1mvalues\\u001b[0m!!\"}\n");
+    }
+
     TEST(logEI, appendingHintsToPreviousError) {
 
         MakeError(TestError, Error);

From 8ce88adad9728142e1f11f7e00d2f31925f8fdd1 Mon Sep 17 00:00:00 2001
From: Rok Garbas <rok@garbas.si>
Date: Thu, 20 Aug 2020 13:21:22 +0200
Subject: [PATCH 06/13] set Content-Type to "text/plain" for install script

fixes #3947
---
 maintainers/upload-release.pl | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/maintainers/upload-release.pl b/maintainers/upload-release.pl
index 91ae896d6..6f3882a12 100755
--- a/maintainers/upload-release.pl
+++ b/maintainers/upload-release.pl
@@ -117,7 +117,16 @@ for my $fn (glob "$tmpDir/*") {
     my $dstKey = "$releaseDir/" . $name;
     unless (defined $releasesBucket->head_key($dstKey)) {
         print STDERR "uploading $fn to s3://$releasesBucketName/$dstKey...\n";
-        $releasesBucket->add_key_filename($dstKey, $fn)
+
+        my $configuration = ();
+        $configuration->{content_type} = "application/octet-stream";
+
+        if ($fn =~ /.sha256|.asc|install/) {
+            # Text files
+            $configuration->{content_type} = "text/plain";
+        }
+
+        $releasesBucket->add_key_filename($dstKey, $fn, $configuration)
             or die $releasesBucket->err . ": " . $releasesBucket->errstr;
     }
 }

From 422affe1027058c7807adec0dac1aee09b65ec4c Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Fri, 21 Aug 2020 19:19:14 +0000
Subject: [PATCH 07/13] tabs -> spaces

Sorry I let the tab sneak in there in the first place.
---
 src/libstore/remote-store.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index c2c6eff66..adba17c5e 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -74,7 +74,7 @@ void write(const Store & store, Sink & out, const StorePath & storePath)
 template<>
 std::optional<StorePath> read(const Store & store, Source & from, Phantom<std::optional<StorePath>> _)
 {
-	auto s = readString(from);
+    auto s = readString(from);
     return s == "" ? std::optional<StorePath> {} : store.parseStorePath(s);
 }
 

From a0f19d9f3a009961869a077922f15986ce05738e Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 27 Aug 2020 14:48:08 +0200
Subject: [PATCH 08/13] RemoteStore::addToStore(): Fix race between
 stderrThread and NAR writer

As pointed out by @B4dM4n, the call to to.flush() on stderrThread is
unsafe because the NAR writer thread is also writing to 'to'.

Fixes #3943.
---
 src/libstore/remote-store.cc | 13 ++++++++-----
 src/libstore/remote-store.hh |  2 +-
 src/libutil/serialise.hh     |  6 ++++--
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index adba17c5e..e4a4ef5af 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -284,9 +284,9 @@ struct ConnectionHandle
 
     RemoteStore::Connection * operator -> () { return &*handle; }
 
-    void processStderr(Sink * sink = 0, Source * source = 0)
+    void processStderr(Sink * sink = 0, Source * source = 0, bool flush = true)
     {
-        auto ex = handle->processStderr(sink, source);
+        auto ex = handle->processStderr(sink, source, flush);
         if (ex) {
             daemonException = true;
             std::rethrow_exception(ex);
@@ -535,6 +535,8 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
 
         if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) {
 
+            conn->to.flush();
+
             std::exception_ptr ex;
 
             struct FramedSink : BufferedSink
@@ -574,7 +576,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
             std::thread stderrThread([&]()
             {
                 try {
-                    conn.processStderr();
+                    conn.processStderr(nullptr, nullptr, false);
                 } catch (...) {
                     ex = std::current_exception();
                 }
@@ -884,9 +886,10 @@ static Logger::Fields readFields(Source & from)
 }
 
 
-std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * source)
+std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * source, bool flush)
 {
-    to.flush();
+    if (flush)
+        to.flush();
 
     while (true) {
 
diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh
index b319e774b..7cf4c4d12 100644
--- a/src/libstore/remote-store.hh
+++ b/src/libstore/remote-store.hh
@@ -114,7 +114,7 @@ protected:
 
         virtual ~Connection();
 
-        std::exception_ptr processStderr(Sink * sink = 0, Source * source = 0);
+        std::exception_ptr processStderr(Sink * sink = 0, Source * source = 0, bool flush = true);
     };
 
     ref<Connection> openConnectionWrapper();
diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh
index 69ae0874a..8f17bc34c 100644
--- a/src/libutil/serialise.hh
+++ b/src/libutil/serialise.hh
@@ -23,7 +23,8 @@ struct Sink
 };
 
 
-/* A buffered abstract sink. */
+/* A buffered abstract sink. Warning: a BufferedSink should not be
+   used from multiple threads concurrently. */
 struct BufferedSink : virtual Sink
 {
     size_t bufSize, bufPos;
@@ -66,7 +67,8 @@ struct Source
 };
 
 
-/* A buffered abstract source. */
+/* A buffered abstract source. Warning: a BufferedSink should not be
+   used from multiple threads concurrently. */
 struct BufferedSource : Source
 {
     size_t bufSize, bufPosIn, bufPosOut;

From e915fd6d2afb0299bcb77069503698faabe5f233 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 27 Aug 2020 14:51:50 +0200
Subject: [PATCH 09/13] Typo

---
 src/libutil/serialise.hh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh
index 8f17bc34c..a6f1c42e9 100644
--- a/src/libutil/serialise.hh
+++ b/src/libutil/serialise.hh
@@ -67,7 +67,7 @@ struct Source
 };
 
 
-/* A buffered abstract source. Warning: a BufferedSink should not be
+/* A buffered abstract source. Warning: a BufferedSource should not be
    used from multiple threads concurrently. */
 struct BufferedSource : Source
 {

From 626200713bb3cc844a9feb6af583c9b6b42c6dbc Mon Sep 17 00:00:00 2001
From: Griffin Smith <root@gws.fyi>
Date: Thu, 27 Aug 2020 12:28:12 -0400
Subject: [PATCH 10/13] Pass all args when auto-calling a function with an
 ellipsis

The command line options --arg and --argstr that are used by a bunch of
CLI commands to pass arguments to top-level functions in files go
through the same code-path as auto-calling top-level functions with
their default arguments - this, however, was only passing the arguments
that were *explicitly* mentioned in the formals of the function - in the
case of an as-pattern with an ellipsis (eg args @ { ... }) extra passed
arguments would get omitted. This fixes that to instead pass *all*
specified auto args in the case that our function has an ellipsis.

Fixes #598
---
 src/libexpr/eval.cc | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 0123070d1..00191bce0 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -1299,12 +1299,23 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res)
     Value * actualArgs = allocValue();
     mkAttrs(*actualArgs, fun.lambda.fun->formals->formals.size());
 
-    for (auto & i : fun.lambda.fun->formals->formals) {
-        Bindings::iterator j = args.find(i.name);
-        if (j != args.end())
-            actualArgs->attrs->push_back(*j);
-        else if (!i.def)
-            throwTypeError("cannot auto-call a function that has an argument without a default value ('%1%')", i.name);
+    if (fun.lambda.fun->formals->ellipsis) {
+        // If the formals have an ellipsis (eg the function accepts extra args) pass
+        // all available automatic arguments (which includes arguments specified on
+        // the command line via --arg/--argstr)
+        for (auto& v : args) {
+            actualArgs->attrs->push_back(v);
+        }
+    } else {
+        // Otherwise, only pass the arguments that the function accepts
+        for (auto & i : fun.lambda.fun->formals->formals) {
+            Bindings::iterator j = args.find(i.name);
+            if (j != args.end()) {
+                actualArgs->attrs->push_back(*j);
+            } else if (!i.def) {
+                throwTypeError("cannot auto-call a function that has an argument without a default value ('%1%')", i.name);
+            }
+        }
     }
 
     actualArgs->attrs->sort();

From 3156560d413634eea170e8dd88b3c2208149d999 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Fri, 28 Aug 2020 18:16:03 +0200
Subject: [PATCH 11/13] nix develop: Set output paths to writable locations

Currently, they're set to $(pwd)/outputs/$outputName. This allows
commands like 'make install' to work.
---
 flake.nix          | 11 ++-----
 src/nix/develop.cc | 81 +++++++++++++++++++++++++++++-----------------
 src/nix/get-env.sh | 17 +++++-----
 3 files changed, 63 insertions(+), 46 deletions(-)

diff --git a/flake.nix b/flake.nix
index a707e90e7..32ebb4936 100644
--- a/flake.nix
+++ b/flake.nix
@@ -421,6 +421,8 @@
         stdenv.mkDerivation {
           name = "nix";
 
+          outputs = [ "out" "dev" "doc" ];
+
           buildInputs = buildDeps ++ propagatedDeps ++ perlDeps;
 
           inherit configureFlags;
@@ -428,15 +430,6 @@
           enableParallelBuilding = true;
 
           installFlags = "sysconfdir=$(out)/etc";
-
-          shellHook =
-            ''
-              export prefix=$(pwd)/inst
-              configureFlags+=" --prefix=$prefix"
-              PKG_CONFIG_PATH=$prefix/lib/pkgconfig:$PKG_CONFIG_PATH
-              PATH=$prefix/bin:$PATH
-              unset PYTHONPATH
-            '';
         });
 
   };
diff --git a/src/nix/develop.cc b/src/nix/develop.cc
index 8a14e45c9..20316c477 100644
--- a/src/nix/develop.cc
+++ b/src/nix/develop.cc
@@ -15,7 +15,7 @@ struct Var
 {
     bool exported = true;
     bool associative = false;
-    std::string value; // quoted string or array
+    std::string quoted; // quoted string or array
 };
 
 struct BuildEnvironment
@@ -75,12 +75,12 @@ BuildEnvironment readEnvironment(const Path & path)
 
         else if (std::regex_search(pos, file.cend(), match, varRegex, std::regex_constants::match_continuous)) {
             pos = match[0].second;
-            res.env.insert({match[1], Var { .exported = exported.count(match[1]) > 0, .value = match[2] }});
+            res.env.insert({match[1], Var { .exported = exported.count(match[1]) > 0, .quoted = match[2] }});
         }
 
         else if (std::regex_search(pos, file.cend(), match, assocArrayRegex, std::regex_constants::match_continuous)) {
             pos = match[0].second;
-            res.env.insert({match[1], Var { .associative = true, .value = match[2] }});
+            res.env.insert({match[1], Var { .associative = true, .quoted = match[2] }});
         }
 
         else if (std::regex_search(pos, file.cend(), match, functionRegex, std::regex_constants::match_continuous)) {
@@ -92,6 +92,8 @@ BuildEnvironment readEnvironment(const Path & path)
             path, file.substr(pos - file.cbegin(), 60));
     }
 
+    res.env.erase("__output");
+
     return res;
 }
 
@@ -125,27 +127,32 @@ StorePath getDerivationEnvironment(ref<Store> store, const StorePath & drvPath)
     /* Rehash and write the derivation. FIXME: would be nice to use
        'buildDerivation', but that's privileged. */
     drv.name += "-env";
-    for (auto & output : drv.outputs)
-        drv.env.erase(output.first);
-    drv.outputs = {{"out", DerivationOutput { .output = DerivationOutputInputAddressed { .path = StorePath::dummy }}}};
-    drv.env["out"] = "";
-    drv.env["_outputs_saved"] = drv.env["outputs"];
-    drv.env["outputs"] = "out";
+    for (auto & output : drv.outputs) {
+        output.second = { .output = DerivationOutputInputAddressed { .path = StorePath::dummy } };
+        drv.env[output.first] = "";
+    }
     drv.inputSrcs.insert(std::move(getEnvShPath));
     Hash h = std::get<0>(hashDerivationModulo(*store, drv, true));
-    auto shellOutPath = store->makeOutputPath("out", h, drv.name);
-    drv.outputs.insert_or_assign("out", DerivationOutput { .output = DerivationOutputInputAddressed {
-                .path = shellOutPath
-            } });
-    drv.env["out"] = store->printStorePath(shellOutPath);
-    auto shellDrvPath2 = writeDerivation(*store, drv);
+
+    for (auto & output : drv.outputs) {
+        auto outPath = store->makeOutputPath(output.first, h, drv.name);
+        output.second = { .output = DerivationOutputInputAddressed { .path = outPath } };
+        drv.env[output.first] = store->printStorePath(outPath);
+    }
+
+    auto shellDrvPath = writeDerivation(*store, drv);
 
     /* Build the derivation. */
-    store->buildPaths({{shellDrvPath2}});
+    store->buildPaths({{shellDrvPath}});
 
-    assert(store->isValidPath(shellOutPath));
+    for (auto & outPath : drv.outputPaths(*store)) {
+        assert(store->isValidPath(outPath));
+        auto outPathS = store->toRealPath(outPath);
+        if (lstat(outPathS).st_size)
+            return outPath;
+    }
 
-    return shellOutPath;
+    throw Error("get-env.sh failed to produce an environment");
 }
 
 struct Common : InstallableCommand, MixProfile
@@ -171,8 +178,12 @@ struct Common : InstallableCommand, MixProfile
         "UID",
     };
 
-    void makeRcScript(const BuildEnvironment & buildEnvironment, std::ostream & out)
+    std::string makeRcScript(
+        const BuildEnvironment & buildEnvironment,
+        const Path & outputsDir = absPath(".") + "/outputs")
     {
+        std::ostringstream out;
+
         out << "unset shellHook\n";
 
         out << "nix_saved_PATH=\"$PATH\"\n";
@@ -180,9 +191,9 @@ struct Common : InstallableCommand, MixProfile
         for (auto & i : buildEnvironment.env) {
             if (!ignoreVars.count(i.first) && !hasPrefix(i.first, "BASH_")) {
                 if (i.second.associative)
-                    out << fmt("declare -A %s=(%s)\n", i.first, i.second.value);
+                    out << fmt("declare -A %s=(%s)\n", i.first, i.second.quoted);
                 else {
-                    out << fmt("%s=%s\n", i.first, i.second.value);
+                    out << fmt("%s=%s\n", i.first, i.second.quoted);
                     if (i.second.exported)
                         out << fmt("export %s\n", i.first);
                 }
@@ -193,13 +204,26 @@ struct Common : InstallableCommand, MixProfile
 
         out << buildEnvironment.bashFunctions << "\n";
 
-        // FIXME: set outputs
-
         out << "export NIX_BUILD_TOP=\"$(mktemp -d --tmpdir nix-shell.XXXXXX)\"\n";
         for (auto & i : {"TMP", "TMPDIR", "TEMP", "TEMPDIR"})
             out << fmt("export %s=\"$NIX_BUILD_TOP\"\n", i);
 
         out << "eval \"$shellHook\"\n";
+
+        /* Substitute occurrences of output paths. */
+        auto outputs = buildEnvironment.env.find("outputs");
+        assert(outputs != buildEnvironment.env.end());
+
+        // FIXME: properly unquote 'outputs'.
+        StringMap rewrites;
+        for (auto & outputName : tokenizeString<std::vector<std::string>>(replaceStrings(outputs->second.quoted, "'", ""))) {
+            auto from = buildEnvironment.env.find(outputName);
+            assert(from != buildEnvironment.env.end());
+            // FIXME: unquote
+            rewrites.insert({from->second.quoted, outputsDir + "/" + outputName});
+        }
+
+        return rewriteStrings(out.str(), rewrites);
     }
 
     Strings getDefaultFlakeAttrPaths() override
@@ -288,19 +312,18 @@ struct CmdDevelop : Common, MixEnvironment
 
         auto [rcFileFd, rcFilePath] = createTempFile("nix-shell");
 
-        std::ostringstream ss;
-        makeRcScript(buildEnvironment, ss);
+        auto script = makeRcScript(buildEnvironment);
 
-        ss << fmt("rm -f '%s'\n", rcFilePath);
+        script +=  fmt("rm -f '%s'\n", rcFilePath);
 
         if (!command.empty()) {
             std::vector<std::string> args;
             for (auto s : command)
                 args.push_back(shellEscape(s));
-            ss << fmt("exec %s\n", concatStringsSep(" ", args));
+            script += fmt("exec %s\n", concatStringsSep(" ", args));
         }
 
-        writeFull(rcFileFd.get(), ss.str());
+        writeFull(rcFileFd.get(), script);
 
         stopProgressBar();
 
@@ -362,7 +385,7 @@ struct CmdPrintDevEnv : Common
 
         stopProgressBar();
 
-        makeRcScript(buildEnvironment, std::cout);
+        std::cout << makeRcScript(buildEnvironment);
     }
 };
 
diff --git a/src/nix/get-env.sh b/src/nix/get-env.sh
index 2e0e83561..091c0f573 100644
--- a/src/nix/get-env.sh
+++ b/src/nix/get-env.sh
@@ -1,12 +1,6 @@
 set -e
 if [ -e .attrs.sh ]; then source .attrs.sh; fi
 
-outputs=$_outputs_saved
-for __output in $_outputs_saved; do
-    declare "$__output"="$out"
-done
-unset _outputs_saved __output
-
 export IN_NIX_SHELL=impure
 export dontAddDisableDepTrack=1
 
@@ -14,5 +8,12 @@ if [[ -n $stdenv ]]; then
     source $stdenv/setup
 fi
 
-export > $out
-set >> $out
+for __output in $outputs; do
+    if [[ -z $__done ]]; then
+        export > ${!__output}
+        set >> ${!__output}
+        __done=1
+    else
+        echo -n >> ${!__output}
+    fi
+done

From 50a8710ed14c7a0236a656bb62d5117519ec5813 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Fri, 28 Aug 2020 18:43:34 +0200
Subject: [PATCH 12/13] Close stdin while running tests

For some reason, the bash shell started by 'nix develop' sometimes
reads from stdin, which can hang.
---
 mk/tests.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mk/tests.mk b/mk/tests.mk
index 2e39bb694..c1e140bac 100644
--- a/mk/tests.mk
+++ b/mk/tests.mk
@@ -8,7 +8,7 @@ define run-install-test
 
   .PHONY: $1.test
   $1.test: $1 $(test-deps)
-	@env TEST_NAME=$(notdir $(basename $1)) TESTS_ENVIRONMENT="$(tests-environment)" mk/run_test.sh $1
+	@env TEST_NAME=$(notdir $(basename $1)) TESTS_ENVIRONMENT="$(tests-environment)" mk/run_test.sh $1 < /dev/null
 
 endef
 

From f15651303f8596bf34c67fc8d536b1e9e7843a87 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Fri, 28 Aug 2020 19:24:29 +0200
Subject: [PATCH 13/13] nix develop: Add convenience flags for running specific
 phases

For example, for building the Nix flake, you would do:

  $ nix develop --configure
  $ nix develop --install
  $ nix develop --installcheck
---
 src/nix/develop.cc | 55 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/src/nix/develop.cc b/src/nix/develop.cc
index 20316c477..516e7bda9 100644
--- a/src/nix/develop.cc
+++ b/src/nix/develop.cc
@@ -264,6 +264,7 @@ struct Common : InstallableCommand, MixProfile
 struct CmdDevelop : Common, MixEnvironment
 {
     std::vector<std::string> command;
+    std::optional<std::string> phase;
 
     CmdDevelop()
     {
@@ -277,6 +278,43 @@ struct CmdDevelop : Common, MixEnvironment
                 command = ss;
             }}
         });
+
+        addFlag({
+            .longName = "phase",
+            .description = "phase to run (e.g. `build` or `configure`)",
+            .labels = {"phase-name"},
+            .handler = {&phase},
+        });
+
+        addFlag({
+            .longName = "configure",
+            .description = "run the configure phase",
+            .handler = {&phase, {"configure"}},
+        });
+
+        addFlag({
+            .longName = "build",
+            .description = "run the build phase",
+            .handler = {&phase, {"build"}},
+        });
+
+        addFlag({
+            .longName = "check",
+            .description = "run the check phase",
+            .handler = {&phase, {"check"}},
+        });
+
+        addFlag({
+            .longName = "install",
+            .description = "run the install phase",
+            .handler = {&phase, {"install"}},
+        });
+
+        addFlag({
+            .longName = "installcheck",
+            .description = "run the installcheck phase",
+            .handler = {&phase, {"installCheck"}},
+        });
     }
 
     std::string description() override
@@ -314,9 +352,22 @@ struct CmdDevelop : Common, MixEnvironment
 
         auto script = makeRcScript(buildEnvironment);
 
-        script +=  fmt("rm -f '%s'\n", rcFilePath);
+        if (verbosity >= lvlDebug)
+            script += "set -x\n";
 
-        if (!command.empty()) {
+        script += fmt("rm -f '%s'\n", rcFilePath);
+
+        if (phase) {
+            if (!command.empty())
+                throw UsageError("you cannot use both '--command' and '--phase'");
+            // FIXME: foundMakefile is set by buildPhase, need to get
+            // rid of that.
+            script += fmt("foundMakefile=1\n");
+            script += fmt("runHook %1%Phase\n", *phase);
+            script += fmt("exit 0\n", *phase);
+        }
+
+        else if (!command.empty()) {
             std::vector<std::string> args;
             for (auto s : command)
                 args.push_back(shellEscape(s));