From c78686e411e0a14cff51836fe6c35d7584171df3 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 10 May 2019 16:39:31 -0400 Subject: build: run diff-hook under --check and document diff-hook --- src/libstore/build.cc | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 91eb97dfb..026828c53 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -461,6 +461,19 @@ static void commonChildInit(Pipe & logPipe) close(fdDevNull); } +void handleDiffHook(Path tryA, Path tryB, Path drvPath) +{ + auto diffHook = settings.diffHook; + if (diffHook != "" && settings.runDiffHook) { + try { + auto diff = runProgram(diffHook, true, {tryA, tryB, drvPath}); + if (diff != "") + printError(chomp(diff)); + } catch (Error & error) { + printError("diff hook execution failed: %s", error.what()); + } + } +} ////////////////////////////////////////////////////////////////////// @@ -3039,8 +3052,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; @@ -3185,11 +3197,14 @@ void DerivationGoal::registerOutputs() if (!worker.store.isValidPath(path)) continue; auto info = *worker.store.queryPathInfo(path); if (hash.first != info.narHash) { + handleDiffHook(path, actualPath, drvPath); + if (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); + throw Error(format("derivation '%1%' may not be deterministic: output '%2%' differs from '%3%'") % drvPath % path % dst); } else @@ -3254,16 +3269,7 @@ 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(prev, i->second.path, drvPath); if (settings.enforceDeterminism) throw NotDeterministic(msg); -- cgit v1.2.3 From 6df61db0600ca73ccd51e3e5bec5312a04e99da1 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 10 May 2019 20:59:39 -0400 Subject: diff hook: execute as the build user, and pass the temp dir --- src/libstore/build.cc | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 026828c53..f38d2eaa0 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -461,17 +461,26 @@ static void commonChildInit(Pipe & logPipe) close(fdDevNull); } -void handleDiffHook(Path tryA, Path tryB, Path drvPath) +void handleDiffHook(bool allowVfork, uid_t uid, uid_t gid, Path tryA, Path tryB, Path drvPath, Path tmpDir) { auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { - try { - auto diff = runProgram(diffHook, true, {tryA, tryB, drvPath}); - if (diff != "") - printError(chomp(diff)); - } catch (Error & error) { - printError("diff hook execution failed: %s", error.what()); - } + auto wrapper = [&]() { + if (setgid(gid) == -1) + throw SysError("setgid failed"); + if (setuid(uid) == -1) + throw SysError("setuid failed"); + + try { + auto diff = runProgram(diffHook, true, {tryA, tryB, drvPath, tmpDir}); + if (diff != "") + printError(chomp(diff)); + } catch (Error & error) { + printError("diff hook execution failed: %s", error.what()); + } + }; + + doFork(allowVfork, wrapper); } } @@ -3197,14 +3206,18 @@ void DerivationGoal::registerOutputs() if (!worker.store.isValidPath(path)) continue; auto info = *worker.store.queryPathInfo(path); if (hash.first != info.narHash) { - handleDiffHook(path, actualPath, drvPath); - - 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 && !drv->isBuiltin(), + 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 @@ -3269,7 +3282,11 @@ 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); - handleDiffHook(prev, i->second.path, drvPath); + handleDiffHook( + !buildUser && !drv->isBuiltin(), + buildUser ? buildUser->getUID() : getuid(), + buildUser ? buildUser->getGID() : getgid(), + prev, i->second.path, drvPath, tmpDir); if (settings.enforceDeterminism) throw NotDeterministic(msg); -- cgit v1.2.3 From dde8eeb39ae9fb73011462c74e5fa6405e432147 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Sat, 11 May 2019 15:57:38 -0400 Subject: chdir, setgroups --- src/libstore/build.cc | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/libstore') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index f38d2eaa0..8397cd0d1 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -466,8 +466,12 @@ void handleDiffHook(bool allowVfork, uid_t uid, uid_t gid, Path tryA, Path tryB, auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { auto wrapper = [&]() { + if (chdir("/") == -1) + throw SysError("chdir / failed"); if (setgid(gid) == -1) throw SysError("setgid failed"); + if (setgroups(0, 0) == -1) + throw SysError("setgroups failed"); if (setuid(uid) == -1) throw SysError("setuid failed"); -- cgit v1.2.3 From b4a05edbfe49f87555fd284dfb0d6c56ed43217d Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Sat, 11 May 2019 16:35:53 -0400 Subject: runProgram: support gid, uid, chdir --- src/libstore/build.cc | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 8397cd0d1..8902e22bd 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -465,26 +465,22 @@ void handleDiffHook(bool allowVfork, uid_t uid, uid_t gid, Path tryA, Path tryB, { auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { - auto wrapper = [&]() { - if (chdir("/") == -1) - throw SysError("chdir / failed"); - if (setgid(gid) == -1) - throw SysError("setgid failed"); - if (setgroups(0, 0) == -1) - throw SysError("setgroups failed"); - if (setuid(uid) == -1) - throw SysError("setuid failed"); - - try { - auto diff = runProgram(diffHook, true, {tryA, tryB, drvPath, tmpDir}); - if (diff != "") - printError(chomp(diff)); - } catch (Error & error) { - printError("diff hook execution failed: %s", error.what()); - } - }; - - doFork(allowVfork, wrapper); + 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()); + } } } -- cgit v1.2.3 From 73b797c207e1c7a0fd9059d2cf1e3479502f8f1b Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Sun, 12 May 2019 13:44:22 -0400 Subject: handleDiffHook: stop passing allowVfork --- src/libstore/build.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 8902e22bd..b07461013 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -461,7 +461,7 @@ static void commonChildInit(Pipe & logPipe) close(fdDevNull); } -void handleDiffHook(bool allowVfork, uid_t uid, uid_t gid, Path tryA, Path tryB, Path drvPath, Path tmpDir) +void handleDiffHook(uid_t uid, uid_t gid, Path tryA, Path tryB, Path drvPath, Path tmpDir) { auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { @@ -3213,7 +3213,6 @@ void DerivationGoal::registerOutputs() throw SysError(format("renaming '%1%' to '%2%'") % actualPath % dst); handleDiffHook( - !buildUser && !drv->isBuiltin(), buildUser ? buildUser->getUID() : getuid(), buildUser ? buildUser->getGID() : getgid(), path, dst, drvPath, tmpDir); @@ -3283,7 +3282,6 @@ void DerivationGoal::registerOutputs() : fmt("output '%1%' of '%2%' differs from previous round", i->second.path, drvPath); handleDiffHook( - !buildUser && !drv->isBuiltin(), buildUser ? buildUser->getUID() : getuid(), buildUser ? buildUser->getGID() : getgid(), prev, i->second.path, drvPath, tmpDir); -- cgit v1.2.3