Merge pull request #7099 from lheckemann/remove-repeat

Remove repeat option

doc/manual/redirects.js

@@ -35,7 +35,6 @@ const redirects = {
     "conf-build-max-jobs": "command-ref/conf-file.html#conf-build-max-jobs",
     "conf-build-max-log-size": "command-ref/conf-file.html#conf-build-max-log-size",
     "conf-build-max-silent-time": "command-ref/conf-file.html#conf-build-max-silent-time",
-    "conf-build-repeat": "command-ref/conf-file.html#conf-build-repeat",
     "conf-build-timeout": "command-ref/conf-file.html#conf-build-timeout",
     "conf-build-use-chroot": "command-ref/conf-file.html#conf-build-use-chroot",
     "conf-build-use-sandbox": "command-ref/conf-file.html#conf-build-use-sandbox",
@@ -47,7 +46,6 @@ const redirects = {
     "conf-connect-timeout": "command-ref/conf-file.html#conf-connect-timeout",
     "conf-cores": "command-ref/conf-file.html#conf-cores",
     "conf-diff-hook": "command-ref/conf-file.html#conf-diff-hook",
-    "conf-enforce-determinism": "command-ref/conf-file.html#conf-enforce-determinism",
     "conf-env-keep-derivations": "command-ref/conf-file.html#conf-env-keep-derivations",
     "conf-extra-binary-caches": "command-ref/conf-file.html#conf-extra-binary-caches",
     "conf-extra-platforms": "command-ref/conf-file.html#conf-extra-platforms",
@@ -74,7 +72,6 @@ const redirects = {
     "conf-plugin-files": "command-ref/conf-file.html#conf-plugin-files",
     "conf-post-build-hook": "command-ref/conf-file.html#conf-post-build-hook",
     "conf-pre-build-hook": "command-ref/conf-file.html#conf-pre-build-hook",
-    "conf-repeat": "command-ref/conf-file.html#conf-repeat",
     "conf-require-sigs": "command-ref/conf-file.html#conf-require-sigs",
     "conf-restrict-eval": "command-ref/conf-file.html#conf-restrict-eval",
     "conf-run-diff-hook": "command-ref/conf-file.html#conf-run-diff-hook",

doc/manual/src/advanced-topics/diff-hook.md

@@ -121,37 +121,3 @@ error:
     are not valid, so checking is not possible
 
 Run the build without `--check`, and then try with `--check` again.
-
-# Automatic and Optionally Enforced Determinism Verification
-
-Automatically verify every build at build time by executing the build
-multiple times.
-
-Setting `repeat` and `enforce-determinism` in your `nix.conf` permits
-the automated verification of every build Nix performs.
-
-The following configuration will run each build three times, and will
-require the build to be deterministic:
-
-    enforce-determinism = true
-    repeat = 2
-
-Setting `enforce-determinism` to false as in the following
-configuration will run the build multiple times, execute the build
-hook, but will allow the build to succeed even if it does not build
-reproducibly:
-
-    enforce-determinism = false
-    repeat = 1
-
-An example output of this configuration:
-
-```console
-$ nix-build ./test.nix -A unstable
-this derivation will be built:
-  /nix/store/ch6llwpr2h8c3jmnf3f2ghkhx59aa97f-unstable.drv
-building '/nix/store/ch6llwpr2h8c3jmnf3f2ghkhx59aa97f-unstable.drv' (round 1/2)...
-building '/nix/store/ch6llwpr2h8c3jmnf3f2ghkhx59aa97f-unstable.drv' (round 2/2)...
-output '/nix/store/6xg356v9gl03hpbbg8gws77n19qanh02-unstable' of '/nix/store/ch6llwpr2h8c3jmnf3f2ghkhx59aa97f-unstable.drv' differs from '/nix/store/6xg356v9gl03hpbbg8gws77n19qanh02-unstable.check' from previous round
-/nix/store/6xg356v9gl03hpbbg8gws77n19qanh02-unstable
-```

doc/manual/src/command-ref/nix-store.md

@@ -104,10 +104,6 @@ The following flags are available:
     previous build, the new output path is left in
     `/nix/store/name.check.`
 
-    See also the `build-repeat` configuration option, which repeats a
-    derivation a number of times and prevents its outputs from being
-    registered as “valid” in the Nix store unless they are identical.
-
 Special exit codes:
 
   - `100`\

doc/manual/src/release-notes/rl-next.md

@@ -1,2 +1,4 @@
 # Release X.Y (202?-??-??)
 
+* The `repeat` and `enforce-determinism` options have been removed
+  since they had been broken under many circumstances for a long time.

src/libmain/progress-bar.cc

@@ -180,10 +180,12 @@ public:
             auto machineName = getS(fields, 1);
             if (machineName != "")
                 i->s += fmt(" on " ANSI_BOLD "%s" ANSI_NORMAL, machineName);
-            auto curRound = getI(fields, 2);
+
-            auto nrRounds = getI(fields, 3);
+            // Used to be curRound and nrRounds, but the
-            if (nrRounds != 1)
+            // implementation was broken for a long time.
-                i->s += fmt(" (round %d/%d)", curRound, nrRounds);
+            if (getI(fields, 2) != 1 || getI(fields, 3) != 1) {
+                throw Error("log message indicated repeating builds, but this is not currently implemented");
+            }
             i->name = DrvName(name).name;
         }
 

src/libstore/build/derivation-goal.cc

@@ -571,10 +571,6 @@ void DerivationGoal::inputsRealised()
     /* What type of derivation are we building? */
     derivationType = drv->type();
 
-    /* Don't repeat fixed-output derivations since they're already
-       verified by their output hash.*/
-    nrRounds = derivationType.isFixed() ? 1 : settings.buildRepeat + 1;
-
     /* Okay, try to build.  Note that here we don't wait for a build
        slot to become available, since we don't need one if there is a
        build hook. */
@@ -589,12 +585,11 @@ void DerivationGoal::started()
     auto msg = fmt(
         buildMode == bmRepair ? "repairing outputs of '%s'" :
         buildMode == bmCheck ? "checking outputs of '%s'" :
-        nrRounds > 1 ? "building '%s' (round %d/%d)" :
+        "building '%s'", worker.store.printStorePath(drvPath));
-        "building '%s'", worker.store.printStorePath(drvPath), curRound, nrRounds);
     fmt("building '%s'", worker.store.printStorePath(drvPath));
     if (hook) msg += fmt(" on '%s'", machineName);
     act = std::make_unique<Activity>(*logger, lvlInfo, actBuild, msg,
-        Logger::Fields{worker.store.printStorePath(drvPath), hook ? machineName : "", curRound, nrRounds});
+        Logger::Fields{worker.store.printStorePath(drvPath), hook ? machineName : "", 1, 1});
     mcRunningBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.runningBuilds);
     worker.updateProgress();
 }
@@ -948,14 +943,6 @@ void DerivationGoal::buildDone()
 
         cleanupPostOutputsRegisteredModeNonCheck();
 
-        /* Repeat the build if necessary. */
-        if (curRound++ < nrRounds) {
-            outputLocks.unlock();
-            state = &DerivationGoal::tryToBuild;
-            worker.wakeUp(shared_from_this());
-            return;
-        }
-
         /* It is now safe to delete the lock files, since all future
            lockers will see that the output paths are valid; they will
            not create new lock files with the same names as the old

src/libstore/build/derivation-goal.hh

@@ -115,11 +115,6 @@ struct DerivationGoal : public Goal
 
     BuildMode buildMode;
 
-    /* The current round, if we're building multiple times. */
-    size_t curRound = 1;
-
-    size_t nrRounds;
-
     std::unique_ptr<MaintainCount<uint64_t>> mcExpectedBuilds, mcRunningBuilds;
 
     std::unique_ptr<Activity> act;

src/libstore/build/local-derivation-goal.cc

@@ -2260,7 +2260,6 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
     InodesSeen inodesSeen;
 
     Path checkSuffix = ".check";
-    bool keepPreviousRound = settings.keepFailed || settings.runDiffHook;
 
     std::exception_ptr delayedException;
 
@@ -2688,10 +2687,8 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
                 debug("unreferenced input: '%1%'", worker.store.printStorePath(i));
         }
 
-        if (curRound == nrRounds) {
         localStore.optimisePath(actualPath, NoRepair); // FIXME: combine with scanForReferences()
         worker.markContentsGood(newInfo.path);
-        }
 
         newInfo.deriver = drvPath;
         newInfo.ultimate = true;
@@ -2720,61 +2717,6 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
     /* Apply output checks. */
     checkOutputs(infos);
 
-    /* Compare the result with the previous round, and report which
-       path is different, if any.*/
-    if (curRound > 1 && prevInfos != infos) {
-        assert(prevInfos.size() == infos.size());
-        for (auto i = prevInfos.begin(), j = infos.begin(); i != prevInfos.end(); ++i, ++j)
-            if (!(*i == *j)) {
-                buildResult.isNonDeterministic = true;
-                Path prev = worker.store.printStorePath(i->second.path) + checkSuffix;
-                bool prevExists = keepPreviousRound && pathExists(prev);
-                hintformat hint = prevExists
-                    ? hintfmt("output '%s' of '%s' differs from '%s' from previous round",
-                        worker.store.printStorePath(i->second.path), worker.store.printStorePath(drvPath), prev)
-                    : hintfmt("output '%s' of '%s' differs from previous round",
-                        worker.store.printStorePath(i->second.path), worker.store.printStorePath(drvPath));
-
-                handleDiffHook(
-                    buildUser ? buildUser->getUID() : getuid(),
-                    buildUser ? buildUser->getGID() : getgid(),
-                    prev, worker.store.printStorePath(i->second.path),
-                    worker.store.printStorePath(drvPath), tmpDir);
-
-                if (settings.enforceDeterminism)
-                    throw NotDeterministic(hint);
-
-                printError(hint);
-
-                curRound = nrRounds; // we know enough, bail out early
-            }
-    }
-
-    /* If this is the first round of several, then move the output out of the way. */
-    if (nrRounds > 1 && curRound == 1 && curRound < nrRounds && keepPreviousRound) {
-        for (auto & [_, outputStorePath] : finalOutputs) {
-            auto path = worker.store.printStorePath(outputStorePath);
-            Path prev = path + checkSuffix;
-            deletePath(prev);
-            Path dst = path + checkSuffix;
-            renameFile(path, dst);
-        }
-    }
-
-    if (curRound < nrRounds) {
-        prevInfos = std::move(infos);
-        return {};
-    }
-
-    /* Remove the .check directories if we're done. FIXME: keep them
-       if the result was not determistic? */
-    if (curRound == nrRounds) {
-        for (auto & [_, outputStorePath] : finalOutputs) {
-            Path prev = worker.store.printStorePath(outputStorePath) + checkSuffix;
-            deletePath(prev);
-        }
-    }
-
     /* Register each output path as valid, and register the sets of
        paths referenced by each of them.  If there are cycles in the
        outputs, this will fail. */

src/libstore/daemon.cc

@@ -238,7 +238,6 @@ struct ClientSettings
                 }
                 else if (trusted
                     || name == settings.buildTimeout.name
-                    || name == settings.buildRepeat.name
                     || name == settings.maxSilentTime.name
                     || name == settings.pollInterval.name
                     || name == "connect-timeout"

src/libstore/globals.hh

@@ -373,11 +373,6 @@ public:
         )",
         {"build-max-log-size"}};
 
-    /* When buildRepeat > 0 and verboseBuild == true, whether to print
-       repeated builds (i.e. builds other than the first one) to
-       stderr. Hack to prevent Hydra logs from being polluted. */
-    bool printRepeatedBuilds = true;
-
     Setting<unsigned int> pollInterval{this, 5, "build-poll-interval",
         "How often (in seconds) to poll for locks."};
 
@@ -501,19 +496,6 @@ public:
     Setting<bool> sandboxFallback{this, true, "sandbox-fallback",
         "Whether to disable sandboxing when the kernel doesn't allow it."};
 
-    Setting<size_t> buildRepeat{
-        this, 0, "repeat",
-        R"(
-          How many times to repeat builds to check whether they are
-          deterministic. The default value is 0. If the value is non-zero,
-          every build is repeated the specified number of times. If the
-          contents of any of the runs differs from the previous ones and
-          `enforce-determinism` is true, the build is rejected and the
-          resulting store paths are not registered as “valid” in Nix’s
-          database.
-        )",
-        {"build-repeat"}};
-
 #if __linux__
     Setting<std::string> sandboxShmSize{
         this, "50%", "sandbox-dev-shm-size",
@@ -577,10 +559,6 @@ public:
           configuration file, and cannot be passed at the command line.
         )"};
 
-    Setting<bool> enforceDeterminism{
-        this, true, "enforce-determinism",
-        "Whether to fail if repeated builds produce different output. See `repeat`."};
-
     Setting<Strings> trustedPublicKeys{
         this,
         {"cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY="},

src/libstore/legacy-ssh-store.cc

@@ -255,8 +255,8 @@ private:
                 << settings.maxLogSize;
         if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 3)
             conn.to
-                << settings.buildRepeat
+                << 0 // buildRepeat hasn't worked for ages anyway
-                << settings.enforceDeterminism;
+                << 0;
 
         if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 7) {
             conn.to << ((int) settings.keepFailed);

src/nix-store/nix-store.cc

@@ -808,14 +808,17 @@ static void opServe(Strings opFlags, Strings opArgs)
         if (GET_PROTOCOL_MINOR(clientVersion) >= 2)
             settings.maxLogSize = readNum<unsigned long>(in);
         if (GET_PROTOCOL_MINOR(clientVersion) >= 3) {
-            settings.buildRepeat = readInt(in);
+            if (readInt(in) != 0) {
-            settings.enforceDeterminism = readInt(in);
+                throw Error("client requested repeating builds, but this is not currently implemented");
+            }
+            if (readInt(in) != 0) {
+                throw Error("client requested enforcing determinism, but this is not currently implemented");
+            }
             settings.runDiffHook = true;
         }
         if (GET_PROTOCOL_MINOR(clientVersion) >= 7) {
             settings.keepFailed = (bool) readInt(in);
         }
-        settings.printRepeatedBuilds = false;
     };
 
     while (true) {