1 files changed, 50 insertions, 25 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 3f4d82490..004be8010 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -461,6 +461,28 @@ static void commonChildInit(Pipe & logPipe)
     close(fdDevNull);
 }
 
+void handleDiffHook(uid_t uid, uid_t gid, Path tryA, Path tryB, Path drvPath, Path tmpDir)
+{
+    auto diffHook = settings.diffHook;
+    if (diffHook != "" && settings.runDiffHook) {
+        try {
+            RunOptions diffHookOptions(diffHook,{tryA, tryB, drvPath, tmpDir});
+            diffHookOptions.searchPath = true;
+            diffHookOptions.uid = uid;
+            diffHookOptions.gid = gid;
+            diffHookOptions.chdir = "/";
+
+            auto diffRes = runProgram(diffHookOptions);
+            if (!statusOk(diffRes.first))
+                throw ExecError(diffRes.first, fmt("diff-hook program '%1%' %2%", diffHook, statusToString(diffRes.first)));
+
+            if (diffRes.second != "")
+                printError(chomp(diffRes.second));
+        } catch (Error & error) {
+            printError("diff hook execution failed: %s", error.what());
+        }
+    }
+}
 
 //////////////////////////////////////////////////////////////////////
 
@@ -803,9 +825,6 @@ private:
     /* Whether we're currently doing a chroot build. */
     bool useChroot = false;
 
-    /* Whether we need to perform hash rewriting if there are valid output paths. */
-    bool needsHashRewrite;
-
     Path chrootRootDir;
 
     /* RAII object to delete the chroot directory. */
@@ -885,6 +904,9 @@ public:
         Worker & worker, BuildMode buildMode = bmNormal);
     ~DerivationGoal();
 
+    /* Whether we need to perform hash rewriting if there are valid output paths. */
+    bool needsHashRewrite();
+
     void timedOut() override;
 
     string key() override
@@ -997,13 +1019,6 @@ DerivationGoal::DerivationGoal(const Path & drvPath, const StringSet & wantedOut
     , wantedOutputs(wantedOutputs)
     , buildMode(buildMode)
 {
-#if __linux__
-    needsHashRewrite = !useChroot;
-#else
-    /* Darwin requires hash rewriting even when sandboxing is enabled. */
-    needsHashRewrite = true;
-#endif
-
     state = &DerivationGoal::getDerivation;
     name = (format("building of '%1%'") % drvPath).str();
     trace("created");
@@ -1044,6 +1059,17 @@ DerivationGoal::~DerivationGoal()
 }
 
 
+inline bool DerivationGoal::needsHashRewrite()
+{
+#if __linux__
+    return !useChroot;
+#else
+    /* Darwin requires hash rewriting even when sandboxing is enabled. */
+    return true;
+#endif
+}
+
+
 void DerivationGoal::killChild()
 {
     if (pid != -1) {
@@ -2083,7 +2109,7 @@ void DerivationGoal::startBuilder()
 #endif
     }
 
-    if (needsHashRewrite) {
+    if (needsHashRewrite()) {
 
         if (pathExists(homeDir))
             throw Error(format("directory '%1%' exists; please remove it") % homeDir);
@@ -3039,8 +3065,7 @@ void DerivationGoal::registerOutputs()
     InodesSeen inodesSeen;
 
     Path checkSuffix = ".check";
-    bool runDiffHook = settings.runDiffHook;
-    bool keepPreviousRound = settings.keepFailed || runDiffHook;
+    bool keepPreviousRound = settings.keepFailed || settings.runDiffHook;
 
     std::exception_ptr delayedException;
 
@@ -3067,7 +3092,7 @@ void DerivationGoal::registerOutputs()
             if (buildMode != bmCheck) actualPath = worker.store.toRealPath(path);
         }
 
-        if (needsHashRewrite) {
+        if (needsHashRewrite()) {
             Path redirected = redirectedOutputs[path];
             if (buildMode == bmRepair
                 && redirectedBadOutputs.find(path) != redirectedBadOutputs.end()
@@ -3185,11 +3210,17 @@ void DerivationGoal::registerOutputs()
             if (!worker.store.isValidPath(path)) continue;
             auto info = *worker.store.queryPathInfo(path);
             if (hash.first != info.narHash) {
-                if (settings.keepFailed) {
+                if (settings.runDiffHook || settings.keepFailed) {
                     Path dst = worker.store.toRealPath(path + checkSuffix);
                     deletePath(dst);
                     if (rename(actualPath.c_str(), dst.c_str()))
                         throw SysError(format("renaming '%1%' to '%2%'") % actualPath % dst);
+
+                    handleDiffHook(
+                        buildUser ? buildUser->getUID() : getuid(),
+                        buildUser ? buildUser->getGID() : getgid(),
+                        path, dst, drvPath, tmpDir);
+
                     throw Error(format("derivation '%1%' may not be deterministic: output '%2%' differs from '%3%'")
                         % drvPath % path % dst);
                 } else
@@ -3254,16 +3285,10 @@ void DerivationGoal::registerOutputs()
                     ? fmt("output '%1%' of '%2%' differs from '%3%' from previous round", i->second.path, drvPath, prev)
                     : fmt("output '%1%' of '%2%' differs from previous round", i->second.path, drvPath);
 
-                auto diffHook = settings.diffHook;
-                if (prevExists && diffHook != "" && runDiffHook) {
-                    try {
-                        auto diff = runProgram(diffHook, true, {prev, i->second.path});
-                        if (diff != "")
-                            printError(chomp(diff));
-                    } catch (Error & error) {
-                        printError("diff hook execution failed: %s", error.what());
-                    }
-                }
+                handleDiffHook(
+                    buildUser ? buildUser->getUID() : getuid(),
+                    buildUser ? buildUser->getGID() : getgid(),
+                    prev, i->second.path, drvPath, tmpDir);
 
                 if (settings.enforceDeterminism)
                     throw NotDeterministic(msg);