Merge remote-tracking branch 'origin/master' into flakes

doc/manual/release-notes/rl-2.3.xml

@@ -4,7 +4,7 @@
       version="5.0"
       xml:id="ssec-relnotes-2.3">
 
-<title>Release 2.3 (2019-08-??)</title>
+<title>Release 2.3 (2019-09-04)</title>
 
 <para>This is primarily a bug fix release. However, it makes some
 incompatible changes:</para>

misc/launchd/org.nixos.nix-daemon.plist.in

@@ -13,8 +13,12 @@
     <true/>
     <key>RunAtLoad</key>
     <true/>
-    <key>Program</key>
+    <key>ProgramArguments</key>
-    <string>@bindir@/nix-daemon</string>
+    <array>
+      <string>/bin/sh</string>
+      <string>-c</string>
+      <string>/bin/wait4path @bindir@/nix-daemon &amp;&amp; @bindir@/nix-daemon</string>
+    </array>
     <key>StandardErrorPath</key>
     <string>/var/log/nix-daemon.log</string>
     <key>StandardOutPath</key>

src/libstore/binary-cache-store.cc

@@ -55,7 +55,7 @@ void BinaryCacheStore::init()
 }
 
 void BinaryCacheStore::getFile(const std::string & path,
-    Callback<std::shared_ptr<std::string>> callback)
+    Callback<std::shared_ptr<std::string>> callback) noexcept
 {
     try {
         callback(getFile(path));
@@ -240,7 +240,7 @@ void BinaryCacheStore::narFromPath(const Path & storePath, Sink & sink)
 }
 
 void BinaryCacheStore::queryPathInfoUncached(const Path & storePath,
-    Callback<std::shared_ptr<ValidPathInfo>> callback)
+    Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept
 {
     auto uri = getUri();
     auto act = std::make_shared<Activity>(*logger, lvlTalkative, actQueryPathInfo,
@@ -249,21 +249,23 @@ void BinaryCacheStore::queryPathInfoUncached(const Path & storePath,
 
     auto narInfoFile = narInfoFileFor(storePath);
 
+    auto callbackPtr = std::make_shared<decltype(callback)>(std::move(callback));
+
     getFile(narInfoFile,
         {[=](std::future<std::shared_ptr<std::string>> fut) {
             try {
                 auto data = fut.get();
 
-                if (!data) return callback(nullptr);
+                if (!data) return (*callbackPtr)(nullptr);
 
                 stats.narInfoRead++;
 
-                callback((std::shared_ptr<ValidPathInfo>)
+                (*callbackPtr)((std::shared_ptr<ValidPathInfo>)
                     std::make_shared<NarInfo>(*this, *data, narInfoFile));
 
                 (void) act; // force Activity into this lambda to ensure it stays alive
             } catch (...) {
-                callback.rethrow();
+                callbackPtr->rethrow();
             }
         }});
 }

src/libstore/binary-cache-store.hh

@@ -47,7 +47,7 @@ public:
     /* Fetch the specified file and call the specified callback with
        the result. A subclass may implement this asynchronously. */
     virtual void getFile(const std::string & path,
-        Callback<std::shared_ptr<std::string>> callback);
+        Callback<std::shared_ptr<std::string>> callback) noexcept;
 
     std::shared_ptr<std::string> getFile(const std::string & path);
 
@@ -73,7 +73,7 @@ public:
     bool isValidPathUncached(const Path & path) override;
 
     void queryPathInfoUncached(const Path & path,
-        Callback<std::shared_ptr<ValidPathInfo>> callback) override;
+        Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept override;
 
     Path queryPathFromHashPart(const string & hashPart) override
     { unsupported("queryPathFromHashPart"); }

src/libstore/build.cc

@@ -1197,7 +1197,7 @@ void DerivationGoal::haveDerivation()
     /* We are first going to try to create the invalid output paths
        through substitutes.  If that doesn't work, we'll build
        them. */
-    if (settings.useSubstitutes && drv->substitutesAllowed())
+    if (settings.useSubstitutes && parsedDrv->substitutesAllowed())
         for (auto & i : invalidOutputs)
             addWaitee(worker.makeSubstitutionGoal(i, buildMode == bmRepair ? Repair : NoRepair));
 

src/libstore/derivations.cc

@@ -36,12 +36,6 @@ Path BasicDerivation::findOutput(const string & id) const
 }
 
 
-bool BasicDerivation::substitutesAllowed() const
-{
-    return get(env, "allowSubstitutes", "1") == "1";
-}
-
-
 bool BasicDerivation::isBuiltin() const
 {
     return string(builder, 0, 8) == "builtin:";

src/libstore/derivations.hh

@@ -56,8 +56,6 @@ struct BasicDerivation
        the given derivation. */
     Path findOutput(const string & id) const;
 
-    bool substitutesAllowed() const;
-
     bool isBuiltin() const;
 
     /* Return true iff this is a fixed-output derivation. */

src/libstore/download.cc

@@ -77,13 +77,13 @@ struct CurlDownloader : public Downloader
 
         DownloadItem(CurlDownloader & downloader,
             const DownloadRequest & request,
-            Callback<DownloadResult> callback)
+            Callback<DownloadResult> && callback)
             : downloader(downloader)
             , request(request)
             , act(*logger, lvlTalkative, actDownload,
                 fmt(request.data ? "uploading '%s'" : "downloading '%s'", request.uri),
                 {request.uri}, request.parentAct)
-            , callback(callback)
+            , callback(std::move(callback))
             , finalSink([this](const unsigned char * data, size_t len) {
                 if (this->request.dataCallback) {
                     writtenToSink += len;
@@ -342,15 +342,9 @@ struct CurlDownloader : public Downloader
                 (httpStatus == 200 || httpStatus == 201 || httpStatus == 204 || httpStatus == 206 || httpStatus == 304 || httpStatus == 226 /* FTP */ || httpStatus == 0 /* other protocol */))
             {
                 result.cached = httpStatus == 304;
-                done = true;
-
-                try {
                 act.progress(result.bodySize, result.bodySize);
-                    callback(std::move(result));
-                } catch (...) {
                 done = true;
-                    callback.rethrow();
+                callback(std::move(result));
-                }
             }
 
             else {
@@ -671,7 +665,7 @@ struct CurlDownloader : public Downloader
             return;
         }
 
-        enqueueItem(std::make_shared<DownloadItem>(*this, request, callback));
+        enqueueItem(std::make_shared<DownloadItem>(*this, request, std::move(callback)));
     }
 };
 

src/libstore/http-binary-cache-store.cc

@@ -131,23 +131,25 @@ protected:
     }
 
     void getFile(const std::string & path,
-        Callback<std::shared_ptr<std::string>> callback) override
+        Callback<std::shared_ptr<std::string>> callback) noexcept override
     {
         checkEnabled();
 
         auto request(makeRequest(path));
 
+        auto callbackPtr = std::make_shared<decltype(callback)>(std::move(callback));
+
         getDownloader()->enqueueDownload(request,
-            {[callback, this](std::future<DownloadResult> result) {
+            {[callbackPtr, this](std::future<DownloadResult> result) {
                 try {
-                    callback(result.get().data);
+                    (*callbackPtr)(result.get().data);
                 } catch (DownloadError & e) {
                     if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden)
-                        return callback(std::shared_ptr<std::string>());
+                        return (*callbackPtr)(std::shared_ptr<std::string>());
                     maybeDisable();
-                    callback.rethrow();
+                    callbackPtr->rethrow();
                 } catch (...) {
-                    callback.rethrow();
+                    callbackPtr->rethrow();
                 }
             }});
     }

src/libstore/legacy-ssh-store.cc

@@ -88,7 +88,7 @@ struct LegacySSHStore : public Store
     }
 
     void queryPathInfoUncached(const Path & path,
-        Callback<std::shared_ptr<ValidPathInfo>> callback) override
+        Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept override
     {
         try {
             auto conn(connections->get());

src/libstore/local-store.cc

@@ -622,7 +622,7 @@ uint64_t LocalStore::addValidPath(State & state,
 
 
 void LocalStore::queryPathInfoUncached(const Path & path,
-    Callback<std::shared_ptr<ValidPathInfo>> callback)
+    Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept
 {
     try {
         auto info = std::make_shared<ValidPathInfo>();

src/libstore/local-store.hh

@@ -127,7 +127,7 @@ public:
     PathSet queryAllValidPaths() override;
 
     void queryPathInfoUncached(const Path & path,
-        Callback<std::shared_ptr<ValidPathInfo>> callback) override;
+        Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept override;
 
     void queryReferrers(const Path & path, PathSet & referrers) override;
 

src/libstore/misc.cc

@@ -1,4 +1,5 @@
 #include "derivations.hh"
+#include "parsed-derivations.hh"
 #include "globals.hh"
 #include "local-store.hh"
 #include "store-api.hh"
@@ -189,6 +190,7 @@ void Store::queryMissing(const PathSet & targets,
             }
 
             Derivation drv = derivationFromPath(i2.first);
+            ParsedDerivation parsedDrv(i2.first, drv);
 
             PathSet invalid;
             for (auto & j : drv.outputs)
@@ -197,7 +199,7 @@ void Store::queryMissing(const PathSet & targets,
                     invalid.insert(j.second.path);
             if (invalid.empty()) return;
 
-            if (settings.useSubstitutes && drv.substitutesAllowed()) {
+            if (settings.useSubstitutes && parsedDrv.substitutesAllowed()) {
                 auto drvState = make_ref<Sync<DrvState>>(DrvState(invalid.size()));
                 for (auto & output : invalid)
                     pool.enqueue(std::bind(checkOutput, i2.first, make_ref<Derivation>(drv), output, drvState));

src/libstore/parsed-derivations.cc

@@ -108,4 +108,9 @@ bool ParsedDerivation::willBuildLocally() const
     return getBoolAttr("preferLocalBuild") && canBuildLocally();
 }
 
+bool ParsedDerivation::substitutesAllowed() const
+{
+    return getBoolAttr("allowSubstitutes", true);
+}
+
 }

src/libstore/parsed-derivations.hh

@@ -30,6 +30,8 @@ public:
     bool canBuildLocally() const;
 
     bool willBuildLocally() const;
+
+    bool substitutesAllowed() const;
 };
 
 }

src/libstore/remote-store.cc

@@ -349,7 +349,7 @@ void RemoteStore::querySubstitutablePathInfos(const PathSet & paths,
 
 
 void RemoteStore::queryPathInfoUncached(const Path & path,
-    Callback<std::shared_ptr<ValidPathInfo>> callback)
+    Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept
 {
     try {
         std::shared_ptr<ValidPathInfo> info;

src/libstore/remote-store.hh

@@ -41,7 +41,7 @@ public:
     PathSet queryAllValidPaths() override;
 
     void queryPathInfoUncached(const Path & path,
-        Callback<std::shared_ptr<ValidPathInfo>> callback) override;
+        Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept override;
 
     void queryReferrers(const Path & path, PathSet & referrers) override;
 

src/libstore/store-api.cc

@@ -329,13 +329,14 @@ ref<const ValidPathInfo> Store::queryPathInfo(const Path & storePath)
 
 
 void Store::queryPathInfo(const Path & storePath,
-    Callback<ref<ValidPathInfo>> callback)
+    Callback<ref<ValidPathInfo>> callback) noexcept
 {
-    assertStorePath(storePath);
+    std::string hashPart;
-
-    auto hashPart = storePathToHash(storePath);
 
     try {
+        assertStorePath(storePath);
+
+        hashPart = storePathToHash(storePath);
 
         {
             auto res = state.lock()->pathInfoCache.get(hashPart);
@@ -365,8 +366,10 @@ void Store::queryPathInfo(const Path & storePath,
 
     } catch (...) { return callback.rethrow(); }
 
+    auto callbackPtr = std::make_shared<decltype(callback)>(std::move(callback));
+
     queryPathInfoUncached(storePath,
-        {[this, storePath, hashPart, callback](std::future<std::shared_ptr<ValidPathInfo>> fut) {
+        {[this, storePath, hashPart, callbackPtr](std::future<std::shared_ptr<ValidPathInfo>> fut) {
 
             try {
                 auto info = fut.get();
@@ -386,8 +389,8 @@ void Store::queryPathInfo(const Path & storePath,
                     throw InvalidPath("path '%s' is not valid", storePath);
                 }
 
-                callback(ref<ValidPathInfo>(info));
+                (*callbackPtr)(ref<ValidPathInfo>(info));
-            } catch (...) { callback.rethrow(); }
+            } catch (...) { callbackPtr->rethrow(); }
         }});
 }
 

src/libstore/store-api.hh

@@ -361,12 +361,12 @@ public:
 
     /* Asynchronous version of queryPathInfo(). */
     void queryPathInfo(const Path & path,
-        Callback<ref<ValidPathInfo>> callback);
+        Callback<ref<ValidPathInfo>> callback) noexcept;
 
 protected:
 
     virtual void queryPathInfoUncached(const Path & path,
-        Callback<std::shared_ptr<ValidPathInfo>> callback) = 0;
+        Callback<std::shared_ptr<ValidPathInfo>> callback) noexcept = 0;
 
 public:
 

src/libutil/util.hh

@@ -469,21 +469,34 @@ string get(const T & map, const string & key, const string & def = "")
    type T or an exception. (We abuse std::future<T> to pass the value or
    exception.) */
 template<typename T>
-struct Callback
+class Callback
 {
     std::function<void(std::future<T>)> fun;
+    std::atomic_flag done = ATOMIC_FLAG_INIT;
+
+public:
 
     Callback(std::function<void(std::future<T>)> fun) : fun(fun) { }
 
-    void operator()(T && t) const
+    Callback(Callback && callback) : fun(std::move(callback.fun))
     {
+        auto prev = callback.done.test_and_set();
+        if (prev) done.test_and_set();
+    }
+
+    void operator()(T && t) noexcept
+    {
+        auto prev = done.test_and_set();
+        assert(!prev);
         std::promise<T> promise;
         promise.set_value(std::move(t));
         fun(promise.get_future());
     }
 
-    void rethrow(const std::exception_ptr & exc = std::current_exception()) const
+    void rethrow(const std::exception_ptr & exc = std::current_exception()) noexcept
     {
+        auto prev = done.test_and_set();
+        assert(!prev);
         std::promise<T> promise;
         promise.set_exception(exc);
         fun(promise.get_future());

tests/gc-auto.sh

@@ -17,42 +17,50 @@ expr=$(cat <<EOF
 with import ./config.nix; mkDerivation {
   name = "gc-A";
   buildCommand = ''
+    set -x
     [[ \$(ls \$NIX_STORE/*-garbage? | wc -l) = 3 ]]
     mkdir \$out
     echo foo > \$out/bar
     echo 1...
     sleep 2
-    echo 200 > $fake_free
+    echo 200 > ${fake_free}.tmp1
+    mv ${fake_free}.tmp1 $fake_free
     echo 2...
     sleep 2
     echo 3...
+    sleep 2
+    echo 4...
     [[ \$(ls \$NIX_STORE/*-garbage? | wc -l) = 1 ]]
   '';
 }
 EOF
 )
 
+expr2=$(cat <<EOF
+with import ./config.nix; mkDerivation {
+  name = "gc-B";
+  buildCommand = ''
+    set -x
+    mkdir \$out
+    echo foo > \$out/bar
+    echo 1...
+    sleep 2
+    echo 200 > ${fake_free}.tmp2
+    mv ${fake_free}.tmp2 $fake_free
+    echo 2...
+    sleep 2
+    echo 3...
+    sleep 2
+    echo 4...
+  '';
+}
+EOF
+)
+
 nix build --impure -v -o $TEST_ROOT/result-A -L "($expr)" \
     --min-free 1000 --max-free 2000 --min-free-check-interval 1 &
 pid=$!
 
-expr2=$(cat <<EOF
-with import ./config.nix; mkDerivation {
-  name = "gc-B";
-  buildCommand = ''
-    mkdir \$out
-    echo foo > \$out/bar
-    echo 1...
-    sleep 2
-    echo 200 > $fake_free
-    echo 2...
-    sleep 2
-    echo 3...
-  '';
-}
-EOF
-)
-
 nix build --impure -v -o $TEST_ROOT/result-B -L "($expr2)" \
     --min-free 1000 --max-free 2000 --min-free-check-interval 1