Merge pull request #7048 from puffnfresh/lutimes

Guard uses of lutimes, for portability

src/libcmd/command.cc

@@ -126,9 +126,11 @@ ref<EvalState> EvalCommand::getEvalState()
 {
     if (!evalState) {
         evalState =
-            std::allocate_shared<EvalState>(
             #if HAVE_BOEHMGC
+            std::allocate_shared<EvalState>(
                 traceable_allocator<EvalState>(),
+            #else
+            std::make_shared<EvalState>(
             #endif
                 lookupPath, getEvalStore(), evalSettings, getStore())
             ;

src/libfetchers/git.cc

@@ -41,21 +41,6 @@ bool isCacheFileWithinTtl(time_t now, const struct stat & st)
     return st.st_mtime + settings.tarballTtl > now;
 }
 
-bool touchCacheFile(const Path & path, time_t touch_time)
-{
-#ifndef _WIN32 // TODO implement
-    struct timeval times[2];
-    times[0].tv_sec = touch_time;
-    times[0].tv_usec = 0;
-    times[1].tv_sec = touch_time;
-    times[1].tv_usec = 0;
-
-    return lutimes(path.c_str(), times) == 0;
-#else
-    return false;
-#endif
-}
-
 Path getCachePath(std::string_view key, bool shallow)
 {
     return getCacheDir()
@@ -594,8 +579,11 @@ struct GitInputScheme : InputScheme
                     warn("could not update local clone of Git repository '%s'; continuing with the most recent version", repoInfo.url);
                 }
 
-                if (!touchCacheFile(localRefFile, now))
+                try {
-                    warn("could not update mtime for file '%s': %s", localRefFile, strerror(errno));
+                    setWriteTime(localRefFile, now, now);
+                } catch (Error & e) {
+                    warn("could not update mtime for file '%s': %s", localRefFile, e.msg());
+                }
                 if (!originalRef && !storeCachedHead(repoInfo.url, ref))
                     warn("could not update cached head '%s' for '%s'", ref, repoInfo.url);
             }

src/libstore/posix-fs-canonicalise.cc

@@ -33,19 +33,9 @@ static void canonicaliseTimestampAndPermissions(const Path & path, const struct
 
 #ifndef _WIN32 // TODO implement
     if (st.st_mtime != mtimeStore) {
-        struct timeval times[2];
+        struct stat st2 = st;
-        times[0].tv_sec = st.st_atime;
+        st2.st_mtime = mtimeStore,
-        times[0].tv_usec = 0;
+        setWriteTime(path, st2);
-        times[1].tv_sec = mtimeStore;
-        times[1].tv_usec = 0;
-#if HAVE_LUTIMES
-        if (lutimes(path.c_str(), times) == -1)
-            if (errno != ENOSYS ||
-                (!S_ISLNK(st.st_mode) && utimes(path.c_str(), times) == -1))
-#else
-        if (!S_ISLNK(st.st_mode) && utimes(path.c_str(), times) == -1)
-#endif
-            throw SysError("changing modification time of '%1%'", path);
     }
 #endif
 }

src/libutil/file-system.cc

@@ -557,29 +557,69 @@ void replaceSymlink(const Path & target, const Path & link)
     }
 }
 
-#ifndef _WIN32
+void setWriteTime(
-static void setWriteTime(const fs::path & p, const struct stat & st)
+    const std::filesystem::path & path,
+    time_t accessedTime,
+    time_t modificationTime,
+    std::optional<bool> optIsSymlink)
 {
-    struct timeval times[2];
+#ifndef _WIN32
-    times[0] = {
+    struct timeval times[2] = {
-        .tv_sec = st.st_atime,
+        {
-        .tv_usec = 0,
+            .tv_sec = accessedTime,
+            .tv_usec = 0,
+        },
+        {
+            .tv_sec = modificationTime,
+            .tv_usec = 0,
+        },
     };
-    times[1] = {
-        .tv_sec = st.st_mtime,
-        .tv_usec = 0,
-    };
-    if (lutimes(p.c_str(), times) != 0)
-        throw SysError("changing modification time of '%s'", p);
-}
 #endif
 
+    auto nonSymlink = [&]{
+        bool isSymlink = optIsSymlink
+            ? *optIsSymlink
+            : fs::is_symlink(path);
+
+        if (!isSymlink) {
+#ifdef _WIN32
+            // FIXME use `fs::last_write_time`.
+            //
+            // Would be nice to use std::filesystem unconditionally, but
+            // doesn't support access time just modification time.
+            //
+            // System clock vs File clock issues also make that annoying.
+            warn("Changing file times is not yet implemented on Windows, path is '%s'", path);
+#else
+            if (utimes(path.c_str(), times) == -1) {
+
+                throw SysError("changing modification time of '%s' (not a symlink)", path);
+            }
+#endif
+        } else {
+            throw Error("Cannot modification time of symlink '%s'", path);
+        }
+    };
+
+#if HAVE_LUTIMES
+    if (lutimes(path.c_str(), times) == -1) {
+        if (errno == ENOSYS)
+            nonSymlink();
+        else
+            throw SysError("changing modification time of '%s'", path);
+    }
+#else
+    nonSymlink();
+#endif
+}
+
+void setWriteTime(const fs::path & path, const struct stat & st)
+{
+    setWriteTime(path, st.st_atime, st.st_mtime, S_ISLNK(st.st_mode));
+}
+
 void copyFile(const fs::path & from, const fs::path & to, bool andDelete)
 {
-#ifndef _WIN32
-    // TODO: Rewrite the `is_*` to use `symlink_status()`
-    auto statOfFrom = lstat(from.c_str());
-#endif
     auto fromStatus = fs::symlink_status(from);
 
     // Mark the directory as writable so that we can delete its children
@@ -599,9 +639,7 @@ void copyFile(const fs::path & from, const fs::path & to, bool andDelete)
         throw Error("file '%s' has an unsupported type", from);
     }
 
-#ifndef _WIN32
+    setWriteTime(to, lstat(from.string().c_str()));
-    setWriteTime(to, statOfFrom);
-#endif
     if (andDelete) {
         if (!fs::is_symlink(fromStatus))
             fs::permissions(from, fs::perms::owner_write, fs::perm_options::add | fs::perm_options::nofollow);

src/libutil/file-system.hh

@@ -156,6 +156,30 @@ inline void createDirs(PathView path)
     return createDirs(Path(path));
 }
 
+/**
+ * Set the access and modification times of the given path, not
+ * following symlinks.
+ *
+ * @param accessTime Specified in seconds.
+ *
+ * @param modificationTime Specified in seconds.
+ *
+ * @param isSymlink Whether the file in question is a symlink. Used for
+ * fallback code where we don't have `lutimes` or similar. if
+ * `std::optional` is passed, the information will be recomputed if it
+ * is needed. Race conditions are possible so be careful!
+ */
+void setWriteTime(
+    const std::filesystem::path & path,
+    time_t accessedTime,
+    time_t modificationTime,
+    std::optional<bool> isSymlink = std::nullopt);
+
+/**
+ * Convenience wrapper that takes all arguments from the `struct stat`.
+ */
+void setWriteTime(const std::filesystem::path & path, const struct stat & st);
+
 /**
  * Create a symlink.
  */