Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for utimensat as an alternative to lutimes #11751

Merged
merged 1 commit into from
Oct 27, 2024
Merged

Add support for utimensat as an alternative to lutimes #11751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ AC_LANG_POP(C++)
AC_CHECK_FUNCS([statvfs pipe2 close_range])


# Check for lutimes, optionally used for changing the mtime of
# symlinks.
AC_CHECK_FUNCS([lutimes])
# Check for lutimes and utimensat, optionally used for changing the
# mtime of symlinks.
AC_CHECK_DECLS([AT_SYMLINK_NOFOLLOW], [], [], [[#include <fcntl.h>]])
AC_CHECK_FUNCS([lutimes utimensat])


# Check whether the store optimiser can optimise symlinks.
Expand Down
68 changes: 34 additions & 34 deletions src/libutil/file-system.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,28 @@ void setWriteTime(
time_t modificationTime,
std::optional<bool> optIsSymlink)
{
#ifndef _WIN32
#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);
#elif HAVE_UTIMENSAT && HAVE_DECL_AT_SYMLINK_NOFOLLOW
struct timespec times[2] = {
{
.tv_sec = accessedTime,
.tv_nsec = 0,
},
{
.tv_sec = modificationTime,
.tv_nsec = 0,
},
};
if (utimensat(AT_FDCWD, path.c_str(), times, AT_SYMLINK_NOFOLLOW) == -1)
throw SysError("changing modification time of '%s' (using `utimensat`)", path);
#else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need to keep lutimes if all modern OSes support utimensat?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine either way. I guess I would lean towards removing it in a separate commit/PR so if we need it back it's an easier revert.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with separating. It may have some knock-on implications like increasing the minimum macOS version enforced in the installer (I think utimensat was added in 10.13).

struct timeval times[2] = {
{
.tv_sec = accessedTime,
Expand All @@ -641,42 +662,21 @@ void setWriteTime(
.tv_usec = 0,
},
};
#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);
#if HAVE_LUTIMES
if (lutimes(path.c_str(), times) == -1)
throw SysError("changing modification time of '%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);
}
};
bool isSymlink = optIsSymlink
? *optIsSymlink
: fs::is_symlink(path);

#if HAVE_LUTIMES
if (lutimes(path.c_str(), times) == -1) {
if (errno == ENOSYS)
nonSymlink();
else
throw SysError("changing modification time of '%s'", path);
if (!isSymlink) {
if (utimes(path.c_str(), times) == -1)
throw SysError("changing modification time of '%s' (not a symlink)", path);
} else {
throw Error("Cannot modification time of symlink '%s'", path);
}
#else
nonSymlink();
#endif
#endif
}

Expand Down
4 changes: 4 additions & 0 deletions src/libutil/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,17 @@ check_funcs = [
# Optionally used to try to close more file descriptors (e.g. before
# forking) on Unix.
'sysconf',
# Optionally used for changing the mtime of files and symlinks.
'utimensat',
]
foreach funcspec : check_funcs
define_name = 'HAVE_' + funcspec.underscorify().to_upper()
define_value = cxx.has_function(funcspec).to_int()
configdata.set(define_name, define_value)
endforeach

configdata.set('HAVE_DECL_AT_SYMLINK_NOFOLLOW', cxx.has_header_symbol('fcntl.h', 'AT_SYMLINK_NOFOLLOW').to_int())

subdir('build-utils-meson/threads')

# Check if -latomic is needed
Expand Down
Loading