Merge remote-tracking branch 'origin/master' into lazy-trees

edolstra · May 14, 2024 · 4d39908 · 4d39908
2 parents 584fece + 5220047
commit 4d39908
Show file tree

Hide file tree

Showing 46 changed files with 269 additions and 285 deletions.
diff --git a/doc/manual/src/contributing/testing.md b/doc/manual/src/contributing/testing.md
@@ -162,14 +162,14 @@ ran test tests/functional/${testName}.sh... [PASS]
 or without `make`:
 
 ```shell-session
-$ ./mk/run-test.sh tests/functional/${testName}.sh tests/functional/init.sh
+$ ./mk/run-test.sh tests/functional/${testName}.sh
 ran test tests/functional/${testName}.sh... [PASS]
 ```
 
 To see the complete output, one can also run:
 
 ```shell-session
-$ ./mk/debug-test.sh tests/functional/${testName}.sh tests/functional/init.sh
+$ ./mk/debug-test.sh tests/functional/${testName}.sh
 +(${testName}.sh:1) foo
 output from foo
 +(${testName}.sh:2) bar
@@ -204,7 +204,7 @@ edit it like so:
 Then, running the test with `./mk/debug-test.sh` will drop you into GDB once the script reaches that point:
 
 ```shell-session
-$ ./mk/debug-test.sh tests/functional/${testName}.sh tests/functional/init.sh
+$ ./mk/debug-test.sh tests/functional/${testName}.sh
 ...
 + gdb blash blub
 GNU gdb (GDB) 12.1

diff --git a/mk/debug-test.sh b/mk/debug-test.sh
@@ -3,12 +3,8 @@
 set -eu -o pipefail
 
 test=$1
-init=${2-}
 
 dir="$(dirname "${BASH_SOURCE[0]}")"
 source "$dir/common-test.sh"
 
-if [ -n "$init" ]; then
-    (run "$init" 2>/dev/null > /dev/null)
-fi
 run "$test"
diff --git a/mk/lib.mk b/mk/lib.mk
@@ -87,15 +87,14 @@ $(foreach script, $(bin-scripts), $(eval $(call install-program-in,$(script),$(b
 $(foreach script, $(bin-scripts), $(eval programs-list += $(script)))
 $(foreach script, $(noinst-scripts), $(eval programs-list += $(script)))
 $(foreach template, $(template-files), $(eval $(call instantiate-template,$(template))))
-install_test_init=tests/functional/init.sh
 $(foreach test, $(install-tests), \
-  $(eval $(call run-test,$(test),$(install_test_init))) \
+  $(eval $(call run-test,$(test))) \
   $(eval installcheck: $(test).test))
 $(foreach test-group, $(install-tests-groups), \
-  $(eval $(call run-test-group,$(test-group),$(install_test_init))) \
+  $(eval $(call run-test-group,$(test-group))) \
   $(eval installcheck: $(test-group).test-group) \
   $(foreach test, $($(test-group)-tests), \
-    $(eval $(call run-test,$(test),$(install_test_init))) \
+    $(eval $(call run-test,$(test))) \
     $(eval $(test-group).test-group: $(test).test)))
 
 # Compilation database.

diff --git a/mk/run-test.sh b/mk/run-test.sh
@@ -8,7 +8,6 @@ yellow=""
 normal=""
 
 test=$1
-init=${2-}
 
 dir="$(dirname "${BASH_SOURCE[0]}")"
 source "$dir/common-test.sh"
@@ -22,9 +21,6 @@ if [ -t 1 ]; then
 fi
 
 run_test () {
-    if [ -n "$init" ]; then
-        (run "$init" 2>/dev/null > /dev/null)
-    fi
     log="$(run "$test" 2>&1)" && status=0 || status=$?
 }
 

diff --git a/mk/tests.mk b/mk/tests.mk
@@ -12,8 +12,8 @@ endef
 
 define run-test
 
-  $(eval $(call run-bash,$1.test,$1 $(test-deps),mk/run-test.sh $1 $2))
-  $(eval $(call run-bash,$1.test-debug,$1 $(test-deps),mk/debug-test.sh $1 $2))
+  $(eval $(call run-bash,$1.test,$1 $(test-deps),mk/run-test.sh $1))
+  $(eval $(call run-bash,$1.test-debug,$1 $(test-deps),mk/debug-test.sh $1))
 
 endef
 

diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc
@@ -259,7 +259,7 @@ StringSet NixRepl::completePrefix(const std::string & prefix)
         try {
             auto dir = std::string(cur, 0, slash);
             auto prefix2 = std::string(cur, slash + 1);
-            for (auto & entry : readDirectory(dir == "" ? "/" : dir)) {
+            for (auto & entry : std::filesystem::directory_iterator{dir == "" ? "/" : dir}) {
                 auto name = entry.path().filename().string();
                 if (name[0] != '.' && hasPrefix(name, prefix2))
                     completions.insert(prev + entry.path().string());

diff --git a/src/libexpr/flake/config.cc b/src/libexpr/flake/config.cc
@@ -32,7 +32,7 @@ static void writeTrustedList(const TrustedList & trustedList)
 
 void ConfigFile::apply()
 {
-    std::set<std::string> whitelist{"bash-prompt", "bash-prompt-prefix", "bash-prompt-suffix", "flake-registry", "commit-lockfile-summary"};
+    std::set<std::string> whitelist{"bash-prompt", "bash-prompt-prefix", "bash-prompt-suffix", "flake-registry", "commit-lock-file-summary", "commit-lockfile-summary"};
 
     for (auto & [name, value] : settings) {
 

diff --git a/src/libfetchers/fetch-settings.hh b/src/libfetchers/fetch-settings.hh
@@ -87,12 +87,12 @@ struct FetchSettings : public Config
         {}, true, Xp::Flakes};
 
     Setting<std::string> commitLockFileSummary{
-        this, "", "commit-lockfile-summary",
+        this, "", "commit-lock-file-summary",
         R"(
           The commit summary to use when committing changed flake lock files. If
           empty, the summary is generated based on the action performed.
         )",
-        {}, true, Xp::Flakes};
+        {"commit-lockfile-summary"}, true, Xp::Flakes};
 
     Setting<bool> trustTarballsFromGitForges{
         this, true, "trust-tarballs-from-git-forges",

diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc
@@ -17,10 +17,10 @@ struct State
 /* For each activated package, create symlinks */
 static void createLinks(State & state, const Path & srcDir, const Path & dstDir, int priority)
 {
-    std::vector<std::filesystem::directory_entry> srcFiles;
+    std::filesystem::directory_iterator srcFiles;
 
     try {
-        srcFiles = readDirectory(srcDir);
+        srcFiles = std::filesystem::directory_iterator{srcDir};
     } catch (std::filesystem::filesystem_error & e) {
         if (e.code() == std::errc::not_a_directory) {
             warn("not including '%s' in the user environment because it's not a directory", srcDir);

diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
@@ -161,7 +161,7 @@ void LocalStore::findTempRoots(Roots & tempRoots, bool censor)
 {
     /* Read the `temproots' directory for per-process temporary root
        files. */
-    for (auto & i : readDirectory(tempRootsDir)) {
+    for (auto & i : std::filesystem::directory_iterator{tempRootsDir}) {
         auto name = i.path().filename().string();
         if (name[0] == '.') {
             // Ignore hidden files. Some package managers (notably portage) create
@@ -228,7 +228,7 @@ void LocalStore::findRoots(const Path & path, std::filesystem::file_type type, R
             type = std::filesystem::symlink_status(path).type();
 
         if (type == std::filesystem::file_type::directory) {
-            for (auto & i : readDirectory(path))
+            for (auto & i : std::filesystem::directory_iterator{path})
                 findRoots(i.path().string(), i.symlink_status().type(), roots);
         }
 

diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc
@@ -345,7 +345,7 @@ void initPlugins()
     for (const auto & pluginFile : settings.pluginFiles.get()) {
         std::vector<std::filesystem::path> pluginFiles;
         try {
-            auto ents = readDirectory(pluginFile);
+            auto ents = std::filesystem::directory_iterator{pluginFile};
             for (const auto & ent : ents)
                 pluginFiles.emplace_back(ent.path());
         } catch (std::filesystem::filesystem_error & e) {

diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc
@@ -83,7 +83,7 @@ class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public
     {
         StorePathSet paths;
 
-        for (auto & entry : readDirectory(binaryCacheDir)) {
+        for (auto & entry : std::filesystem::directory_iterator{binaryCacheDir}) {
             auto name = entry.path().filename().string();
             if (name.size() != 40 ||
                 !hasSuffix(name, ".narinfo"))

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
@@ -1406,7 +1406,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 
         printInfo("checking link hashes...");
 
-        for (auto & link : readDirectory(linksDir)) {
+        for (auto & link : std::filesystem::directory_iterator{linksDir}) {
             auto name = link.path().filename();
             printMsg(lvlTalkative, "checking contents of '%s'", name);
             PosixSourceAccessor accessor;
@@ -1498,7 +1498,7 @@ LocalStore::VerificationResult LocalStore::verifyAllValidPaths(RepairFlag repair
        database and the filesystem) in the loop below, in order to catch
        invalid states.
      */
-    for (auto & i : readDirectory(realStoreDir)) {
+    for (auto & i : std::filesystem::directory_iterator{realStoreDir.to_string()}) {
         try {
             storePathsInStoreDir.insert({i.path().filename().string()});
         } catch (BadStorePath &) { }

diff --git a/src/libstore/posix-fs-canonicalise.cc b/src/libstore/posix-fs-canonicalise.cc
@@ -144,8 +144,7 @@ static void canonicalisePathMetaData_(
 #endif
 
     if (S_ISDIR(st.st_mode)) {
-        std::vector<std::filesystem::directory_entry> entries = readDirectory(path);
-        for (auto & i : entries)
+        for (auto & i : std::filesystem::directory_iterator{path})
             canonicalisePathMetaData_(
                 i.path().string(),
 #ifndef _WIN32

diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc
@@ -37,7 +37,7 @@ std::pair<Generations, std::optional<GenerationNumber>> findGenerations(Path pro
     std::filesystem::path profileDir = dirOf(profile);
     auto profileName = std::string(baseNameOf(profile));
 
-    for (auto & i : readDirectory(profileDir.string())) {
+    for (auto & i : std::filesystem::directory_iterator{profileDir}) {
         if (auto n = parseName(profileName, i.path().filename().string())) {
             auto path = i.path().string();
             gens.push_back({

diff --git a/src/libstore/unix/builtins/unpack-channel.cc b/src/libstore/unix/builtins/unpack-channel.cc
@@ -21,10 +21,13 @@ void builtinUnpackChannel(
 
     unpackTarfile(src, out);
 
-    auto entries = readDirectory(out);
-    if (entries.size() != 1)
+    auto entries = std::filesystem::directory_iterator{out};
+    auto fileName = entries->path().string();
+    auto fileCount = std::distance(std::filesystem::begin(entries), std::filesystem::end(entries));
+
+    if (fileCount != 1)
         throw Error("channel tarball '%s' contains more than one file", src);
-    std::filesystem::rename(entries[0].path().string(), (out + "/" + channelName));
+    std::filesystem::rename(fileName, (out + "/" + channelName));
 }
 
 }
diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc
@@ -222,20 +222,6 @@ Path readLink(const Path & path)
 }
 
 
-std::vector<fs::directory_entry> readDirectory(const Path & path)
-{
-    std::vector<fs::directory_entry> entries;
-    entries.reserve(64);
-
-    for (auto & entry : fs::directory_iterator{path}) {
-        checkInterrupt();
-        entries.push_back(std::move(entry));
-    }
-
-    return entries;
-}
-
-
 std::string readFile(const Path & path)
 {
     AutoCloseFD fd = toDescriptor(open(path.c_str(), O_RDONLY

diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh
@@ -122,12 +122,6 @@ Path readLink(const Path & path);
  */
 Descriptor openDirectory(const std::filesystem::path & path);
 
-/**
- * Read the contents of a directory.  The entries `.` and `..` are
- * removed.
- */
-std::vector<std::filesystem::directory_entry> readDirectory(const Path & path);
-
 /**
  * Read the contents of a file into a string.
  */

diff --git a/src/libutil/linux/cgroup.cc b/src/libutil/linux/cgroup.cc
@@ -64,7 +64,7 @@ static CgroupStats destroyCgroup(const std::filesystem::path & cgroup, bool retu
 
     /* Otherwise, manually kill every process in the subcgroups and
        this cgroup. */
-    for (auto & entry : readDirectory(cgroup)) {
+    for (auto & entry : std::filesystem::directory_iterator{cgroup}) {
         if (entry.symlink_status().type() != std::filesystem::file_type::directory) continue;
         destroyCgroup(cgroup / entry.path().filename(), false);
     }

diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc
@@ -132,7 +132,7 @@ SourceAccessor::DirEntries PosixSourceAccessor::readDirectory(const CanonPath &
 {
     assertNoSymlinks(path);
     DirEntries res;
-    for (auto & entry : nix::readDirectory(makeAbsPath(path).string())) {
+    for (auto & entry : std::filesystem::directory_iterator{makeAbsPath(path)}) {
         auto type = [&]() -> std::optional<Type> {
             std::filesystem::file_type nativeType;
             try {

diff --git a/src/libutil/unix/file-descriptor.cc b/src/libutil/unix/file-descriptor.cc
@@ -124,7 +124,7 @@ void closeMostFDs(const std::set<int> & exceptions)
 {
 #if __linux__
     try {
-        for (auto & s : readDirectory("/proc/self/fd")) {
+        for (auto & s : std::filesystem::directory_iterator{"/proc/self/fd"}) {
             auto fd = std::stoi(s.path().filename());
             if (!exceptions.count(fd)) {
                 debug("closing leaked FD %d", fd);

diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc
@@ -27,7 +27,7 @@ void removeOldGenerations(std::string dir)
 
     bool canWrite = access(dir.c_str(), W_OK) == 0;
 
-    for (auto & i : readDirectory(dir)) {
+    for (auto & i : std::filesystem::directory_iterator{dir}) {
         checkInterrupt();
 
         auto path = i.path().string();

diff --git a/src/nix/flake.md b/src/nix/flake.md
@@ -439,7 +439,7 @@ The following attributes are supported in `flake.nix`:
    - [`bash-prompt-prefix`](@docroot@/command-ref/conf-file.md#conf-bash-prompt-prefix)
    - [`bash-prompt-suffix`](@docroot@/command-ref/conf-file.md#conf-bash-prompt-suffix)
    - [`flake-registry`](@docroot@/command-ref/conf-file.md#conf-flake-registry)
-   - [`commit-lockfile-summary`](@docroot@/command-ref/conf-file.md#conf-commit-lockfile-summary)
+   - [`commit-lock-file-summary`](@docroot@/command-ref/conf-file.md#conf-commit-lock-file-summary)
 
 ## Flake inputs
 

diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc
@@ -115,9 +115,10 @@ std::tuple<StorePath, Hash> prefetchFile(
 
             /* If the archive unpacks to a single file/directory, then use
                that as the top-level. */
-            auto entries = readDirectory(unpacked);
-            if (entries.size() == 1)
-                tmpFile = entries[0].path();
+            auto entries = std::filesystem::directory_iterator{unpacked};
+            auto file_count = std::distance(entries, std::filesystem::directory_iterator{});
+            if (file_count == 1)
+                tmpFile = entries->path();
             else
                 tmpFile = unpacked;
         }

diff --git a/src/nix/run.cc b/src/nix/run.cc
@@ -248,7 +248,7 @@ void chrootHelper(int argc, char * * argv)
         if (mount(realStoreDir.c_str(), (tmpDir + storeDir).c_str(), "", MS_BIND, 0) == -1)
             throw SysError("mounting '%s' on '%s'", realStoreDir, storeDir);
 
-        for (auto entry : readDirectory("/")) {
+        for (auto entry : std::filesystem::directory_iterator{"/"}) {
             auto src = entry.path().string();
             Path dst = tmpDir + "/" + entry.path().filename().string();
             if (pathExists(dst)) continue;

diff --git a/tests/functional/common.sh b/tests/functional/common.sh
@@ -4,7 +4,11 @@ if [[ -z "${COMMON_SH_SOURCED-}" ]]; then
 
 COMMON_SH_SOURCED=1
 
-source "$(readlink -f "$(dirname "${BASH_SOURCE[0]-$0}")")/common/vars-and-functions.sh"
+dir="$(readlink -f "$(dirname "${BASH_SOURCE[0]-$0}")")"
+
+source "$dir"/common/vars-and-functions.sh
+source "$dir"/common/init.sh
+
 if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then
     startDaemon
 fi

diff --git a/tests/functional/init.sh → tests/functional/common/init.sh b/tests/functional/init.sh → tests/functional/common/init.sh
@@ -1,14 +1,13 @@
-# Don't start the daemon
-source common/vars-and-functions.sh
-
 test -n "$TEST_ROOT"
-if test -d "$TEST_ROOT"; then
-    chmod -R u+rw "$TEST_ROOT"
-    # We would delete any daemon socket, so let's stop the daemon first.
-    killDaemon
+# We would delete any daemon socket, so let's stop the daemon first.
+killDaemon
+# Destroy the test directory that may have persisted from previous runs
+if [[ -e "$TEST_ROOT" ]]; then
+    chmod -R u+w "$TEST_ROOT"
     rm -rf "$TEST_ROOT"
 fi
-mkdir "$TEST_ROOT"
+mkdir -p "$TEST_ROOT"
+mkdir "$TEST_HOME"
 
 mkdir "$NIX_STORE_DIR"
 mkdir "$NIX_LOCALSTATE_DIR"
@@ -36,7 +35,7 @@ extra-experimental-features = flakes
 EOF
 
 # Initialise the database.
+# The flag itself does nothing, but running the command touches the store
 nix-store --init
-
-# Did anything happen?
+# Sanity check
 test -e "$NIX_STATE_DIR"/db/db.sqlite
diff --git a/tests/functional/common/vars-and-functions.sh.in b/tests/functional/common/vars-and-functions.sh.in
@@ -1,3 +1,5 @@
+# NOTE: instances of @variable@ are substituted as defined in /mk/templates.mk
+
 set -eu -o pipefail
 
 if [[ -z "${COMMON_VARS_AND_FUNCTIONS_SH_SOURCED-}" ]]; then
@@ -34,7 +36,6 @@ unset XDG_DATA_HOME
 unset XDG_CONFIG_HOME
 unset XDG_CONFIG_DIRS
 unset XDG_CACHE_HOME
-mkdir -p $TEST_HOME
 
 export PATH=@bindir@:$PATH
 if [[ -n "${NIX_CLIENT_PACKAGE:-}" ]]; then

diff --git a/tests/functional/lang.sh b/tests/functional/lang.sh
@@ -72,7 +72,7 @@ for i in lang/eval-fail-*.nix; do
         if [[ -e "lang/$i.flags" ]]; then
             sed -e 's/#.*//' < "lang/$i.flags"
         else
-            # note that show-trace is also set by init.sh
+            # note that show-trace is also set by common/init.sh
             echo "--eval --strict --show-trace"
         fi
     )"

diff --git a/tests/functional/local-overlay-store/add-lower.sh b/tests/functional/local-overlay-store/add-lower.sh
@@ -1,4 +1,5 @@
 source common.sh
+source ../common/init.sh
 
 requireEnvironment
 setupConfig