Merge tag '2.32.2'

Tagging release 2.32.2

Author: edolstra

.version

@@ -1 +1 @@
-2.32.1
+2.32.2

src/libfetchers-tests/git-utils.cc

@@ -176,6 +176,12 @@ TEST_F(GitUtilsTest, peel_reference)
 
 TEST(GitUtils, isLegalRefName)
 {
+    ASSERT_TRUE(isLegalRefName("A/b"));
+    ASSERT_TRUE(isLegalRefName("AaA/b"));
+    ASSERT_TRUE(isLegalRefName("FOO/BAR/BAZ"));
+    ASSERT_TRUE(isLegalRefName("HEAD"));
+    ASSERT_TRUE(isLegalRefName("refs/tags/1.2.3"));
+    ASSERT_TRUE(isLegalRefName("refs/heads/master"));
     ASSERT_TRUE(isLegalRefName("foox"));
     ASSERT_TRUE(isLegalRefName("1337"));
     ASSERT_TRUE(isLegalRefName("foo.baz"));

src/libfetchers/git-utils.cc

@@ -15,6 +15,7 @@
 
 #include <git2/attr.h>
 #include <git2/blob.h>
+#include <git2/branch.h>
 #include <git2/commit.h>
 #include <git2/config.h>
 #include <git2/describe.h>
@@ -31,6 +32,7 @@
 #include <git2/submodule.h>
 #include <git2/sys/odb_backend.h>
 #include <git2/sys/mempack.h>
+#include <git2/tag.h>
 #include <git2/tree.h>
 
 #include <boost/unordered/unordered_flat_map.hpp>
@@ -1388,63 +1390,33 @@ GitRepo::WorkdirInfo GitRepo::getCachedWorkdirInfo(const std::filesystem::path &
     return workdirInfo;
 }
 
-/**
- * Checks that the git reference is valid and normalizes slash '/' sequences.
- *
- * Accepts shorthand references (one-level refnames are allowed).
- */
-bool isValidRefNameAllowNormalizations(const std::string & refName)
-{
-    /* Unfortunately libgit2 doesn't expose the limit in headers, but its internal
-       limit is also 1024. */
-    std::array<char, 1024> normalizedRefBuffer;
-
-    /* It would be nice to have a better API like git_reference_name_is_valid, but
-     * with GIT_REFERENCE_FORMAT_REFSPEC_SHORTHAND flag. libgit2 uses it internally
-     * but doesn't expose it in public headers [1].
-     * [1]:
-     * https://github.com/libgit2/libgit2/blob/9d5f1bacc23594c2ba324c8f0d41b88bf0e9ef04/src/libgit2/refs.c#L1362-L1365
-     */
-
-    auto res = git_reference_normalize_name(
-        normalizedRefBuffer.data(),
-        normalizedRefBuffer.size(),
-        refName.c_str(),
-        GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL | GIT_REFERENCE_FORMAT_REFSPEC_SHORTHAND);
-
-    return res == 0;
-}
-
 bool isLegalRefName(const std::string & refName)
 {
     initLibGit2();
 
-    /* Since `git_reference_normalize_name` is the best API libgit2 has for verifying
-     * reference names with shorthands (see comment in normalizeRefName), we need to
-     * ensure that exceptions to the validity checks imposed by normalization [1] are checked
-     * explicitly.
-     * [1]: https://git-scm.com/docs/git-check-ref-format#Documentation/git-check-ref-format.txt---normalize
-     */
-
     /* Check for cases that don't get rejected by libgit2.
      * FIXME: libgit2 should reject this. */
     if (refName == "@")
         return false;
 
-    /* Leading slashes and consecutive slashes are stripped during normalizatiton. */
-    if (refName.starts_with('/') || refName.find("//") != refName.npos)
-        return false;
-
-    /* Refer to libgit2. */
-    if (!isValidRefNameAllowNormalizations(refName))
-        return false;
-
     /* libgit2 doesn't barf on DEL symbol.
      * FIXME: libgit2 should reject this. */
     if (refName.find('\177') != refName.npos)
         return false;
 
-    return true;
+    for (auto * func : {
+             git_reference_name_is_valid,
+             git_branch_name_is_valid,
+             git_tag_name_is_valid,
+         }) {
+        int valid = 0;
+        if (func(&valid, refName.c_str()))
+            throw Error("checking git reference '%s': %s", refName, git_error_last()->message);
+        if (valid)
+            return true;
+    }
+
+    return false;
 }
 
 } // namespace nix

src/libfetchers/git.cc

@@ -496,6 +496,36 @@ struct GitInputScheme : InputScheme
                    Git interprets them as part of the file name. So get
                    rid of them. */
                 url.query.clear();
+            /* Backward compatibility hack: In old versions of Nix, if you had
+               a flake input like
+
+                 inputs.foo.url = "git+https://foo/bar?dir=subdir";
+
+               it would result in a lock file entry like
+
+                 "original": {
+                   "dir": "subdir",
+                   "type": "git",
+                   "url": "https://foo/bar?dir=subdir"
+                 }
+
+               New versions of Nix remove `?dir=subdir` from the `url` field,
+               since the subdirectory is intended for `FlakeRef`, not the
+               fetcher (and specifically the remote server), that is, the
+               flakeref is parsed into
+
+                 "original": {
+                   "dir": "subdir",
+                   "type": "git",
+                   "url": "https://foo/bar"
+                 }
+
+               However, new versions of nix parsing old flake.lock files would pass the dir=
+               query parameter in the "url" attribute to git, which will then complain.
+
+               For this reason, we are filtering the `dir` query parameter from the URL
+               before passing it to git. */
+            url.query.erase("dir");
             repoInfo.location = url;
         }
 

src/libfetchers/include/nix/fetchers/git-utils.hh

@@ -156,9 +156,12 @@ struct Setter
 };
 
 /**
- * Checks that the git reference is valid and normalized.
+ * Checks that the string can be a valid git reference, branch or tag name.
+ * Accepts shorthand references (one-level refnames are allowed), pseudorefs
+ * like `HEAD`.
  *
- * Accepts shorthand references (one-level refnames are allowed).
+ * @note This is a coarse test to make sure that the refname is at least something
+ * that Git can make sense of.
  */
 bool isLegalRefName(const std::string & refName);
 

src/libmain/shared.cc

@@ -325,34 +325,29 @@ int handleExceptions(const std::string & programName, std::function<void()> fun)
     std::string error = ANSI_RED "error:" ANSI_NORMAL " ";
     try {
         try {
-            try {
-                fun();
-            } catch (...) {
-                /* Subtle: we have to make sure that any `interrupted'
-                   condition is discharged before we reach printMsg()
-                   below, since otherwise it will throw an (uncaught)
-                   exception. */
-                setInterruptThrown();
-                throw;
-            }
-        } catch (Exit & e) {
-            return e.status;
-        } catch (UsageError & e) {
-            logError(e.info());
-            printError("\nTry '%1% --help' for more information.", programName);
-            return 1;
-        } catch (BaseError & e) {
-            logError(e.info());
-            return e.info().status;
-        } catch (std::bad_alloc & e) {
-            printError(error + "out of memory");
-            return 1;
-        } catch (std::exception & e) {
-            printError(error + e.what());
-            return 1;
+            fun();
+        } catch (...) {
+            /* Subtle: we have to make sure that any `interrupted'
+               condition is discharged before we reach printMsg()
+               below, since otherwise it will throw an (uncaught)
+               exception. */
+            setInterruptThrown();
+            throw;
         }
-    } catch (...) {
-        /* In case logger also throws just give up. */
+    } catch (Exit & e) {
+        return e.status;
+    } catch (UsageError & e) {
+        logError(e.info());
+        printError("\nTry '%1% --help' for more information.", programName);
+        return 1;
+    } catch (BaseError & e) {
+        logError(e.info());
+        return e.info().status;
+    } catch (std::bad_alloc & e) {
+        printError(error + "out of memory");
+        return 1;
+    } catch (std::exception & e) {
+        printError(error + e.what());
         return 1;
     }
 

src/libstore/include/nix/store/serve-protocol-connection.hh

@@ -82,6 +82,8 @@ struct ServeProto::BasicClientConnection
     BuildResult getBuildDerivationResponse(const StoreDirConfig & store);
 
     void narFromPath(const StoreDirConfig & store, const StorePath & path, std::function<void(Source &)> fun);
+
+    void importPaths(const StoreDirConfig & store, std::function<void(Sink &)> fun);
 };
 
 struct ServeProto::BasicServerConnection

src/libstore/include/nix/store/serve-protocol.hh

@@ -108,8 +108,13 @@ enum struct ServeProto::Command : uint64_t {
     QueryValidPaths = 1,
     QueryPathInfos = 2,
     DumpStorePath = 3,
-    // ImportPaths = 4, // removed
-    // ExportPaths = 5, // removed
+    /**
+     * @note This is no longer used by Nix (as a client), but it is used
+     * by Hydra. We should therefore not remove it until Hydra no longer
+     * uses it either.
+     */
+    ImportPaths = 4,
+    // ExportPaths = 5,
     BuildPaths = 6,
     QueryClosure = 7,
     BuildDerivation = 8,

src/libstore/serve-protocol-connection.cc

@@ -93,4 +93,14 @@ void ServeProto::BasicClientConnection::narFromPath(
     fun(from);
 }
 
+void ServeProto::BasicClientConnection::importPaths(const StoreDirConfig & store, std::function<void(Sink &)> fun)
+{
+    to << ServeProto::Command::ImportPaths;
+    fun(to);
+    to.flush();
+
+    if (readInt(from) != 1)
+        throw Error("remote machine failed to import closure");
+}
+
 } // namespace nix

src/libstore/unix/build/derivation-builder.cc

@@ -1722,7 +1722,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
             if (buildMode == bmRepair) {
                 /* Path already exists, need to replace it */
                 replaceValidPath(store.toRealPath(finalDestPath), actualPath);
-                actualPath = store.toRealPath(finalDestPath);
             } else if (buildMode == bmCheck) {
                 /* Path already exists, and we want to compare, so we leave out
                    new path in place. */
@@ -1736,7 +1735,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
                 auto destPath = store.toRealPath(finalDestPath);
                 deletePath(destPath);
                 movePath(actualPath, destPath);
-                actualPath = destPath;
             }
         }
 
@@ -1789,7 +1787,9 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
                     debug("unreferenced input: '%1%'", store.printStorePath(i));
             }
 
-            store.optimisePath(actualPath, NoRepair); // FIXME: combine with scanForReferences()
+            if (!store.isValidPath(newInfo.path))
+                store.optimisePath(
+                    store.toRealPath(finalDestPath), NoRepair); // FIXME: combine with scanForReferences()
 
             newInfo.deriver = drvPath;
             newInfo.ultimate = true;