Nanosecond precision timestamps support broken on macOS / BSD / AIX / Solaris #4542

mc-butler · 2024-05-21T11:51:28Z

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/4542
Reporter	`zaytsev` (@zyv)
Mentions	`ag` (andrey.gursky@…-mail.ua)

Followup ticket to:

[PATCH] copying files: timestamps with nanosecond precision not preserved #3575 (implementation)
Initialize struct stat st_[acm]tim.tv_nsec when present #3821 (bugfix)
mc on IBM i PASE building 4.8.20 change #3927 (wrong AIX fix)
MC 4.8.22 doesn't compile on AIX 7.2 #3960 (more wrong AIX)
PR on GitHub FHS-Break: global config belongs into ${sysconfdir}/mc #130

Sadly no-one came around to fix this, so packagers still disable the support manually:

https://github.com/Homebrew/homebrew-core/blob/master/Formula/m/midnight-commander.rb
https://github.com/macports/macports-ports/blame/master/sysutils/mc/Portfile

Here are the notes how to possible fix this:

---

Unfortunately, I think that the easiest way to access XNU sources from macOS 10.13 right now would be to get hired as a kernel developer by Apple. The system is not even officially released to the public yet (target date is 25th of September), and sources are usually released some time after the final release.

Anyways, I believe that source code access is not really necessary in the first place, as I would speculate that all one needs to do is to add

AC_CHECK_MEMBERS([struct stat.st_blksize, struct stat.st_rdev, struct stat.st_mtim, struct stat.st_mtimespec])

and wherever we use HAVE_STRUCT_STAT_ST_MTIM *or* HAVE_UTIMENSAT replace it with

#ifdef HAVE_STRUCT_STAT_ST_MTIM
    st.st_atim.tv_nsec = st.st_mtim.tv_nsec = st.st_ctim.tv_nsec = 0;
#elif defined HAVE_STRUCT_STAT_ST_MTIMESPEC
    st.st_atimespec.tv_nsec = st.st_mtimespec.tv_nsec = st.st_mtimespec.tv_nsec = 0;
#endif

or something along these lines and you will magically not only fix the build, but gain nanosecond precision support for macOS 10.13+ *and* BSD at the same time. Of course, cruft keeps accumulating, so ideally one should think about defining clever struct manipulation functions limiting define proliferation...

Better yet would be to undertake a major refactoring to clean up our time mess afterwards, but that's, of course, just wishful thinking.

Note

Original attachments:

Screenshot 2024-07-27 at 22.04.32.png (raw) by zaytsev (@zyv) on Jul 27, 2024 at 20:07 UTC

The text was updated successfully, but these errors were encountered:

mc-butler · 2024-05-21T12:03:07Z

Changed by zaytsev (@zyv) on May 21, 2024 at 12:03 UTC (comment 1)

Status changed from new to accepted
Owner set to zaytsev

mc-butler · 2024-05-31T07:17:43Z

Changed by zaytsev (@zyv) on May 31, 2024 at 7:17 UTC (comment 2)

Apparently, macOS fixed itself: Homebrew/homebrew-core#173306

Need to check what's up in the BSD land.

mc-butler · 2024-05-31T07:51:21Z

Changed by zaytsev (@zyv) on May 31, 2024 at 7:51 UTC (comment 3)

stat -f %Fm filename can be used if ls is unable to show nanosecond timestamps.

mc-butler · 2024-05-31T08:04:55Z

Changed by zaytsev (@zyv) on May 31, 2024 at 8:04 UTC (comment 4)

MacPorts PR: macports/macports-ports#24224

mc-butler · 2024-05-31T08:32:03Z

Changed by zaytsev (@zyv) on May 31, 2024 at 8:32 UTC (comment 5)

Related code: #3960

mc-butler · 2024-05-31T08:33:15Z

Changed by zaytsev (@zyv) on May 31, 2024 at 8:33 UTC (comment 6)

Related code: #3927

mc-butler · 2024-05-31T20:28:09Z

Changed by zaytsev (@zyv) on May 31, 2024 at 20:28 UTC

Blocking set to #3960

mc-butler · 2024-06-02T17:14:54Z

Changed by zaytsev (@zyv) on Jun 2, 2024 at 17:14 UTC (comment 8)

Milestone changed from Future Releases to 4.8.32
Branch state changed from no branch to on review

Branch: 4542_nanoseconds_cleanup
Initial [934efd96383900ce38fb55d5592f6ac2aecb1280]

Hi Andrew, appreciate reviewing this stuff, maybe you can check on Linux? I've checked it on macOS and it works, will be trying on AIX shortly, it's a bit of a pain to build. I hope my idea is clear. If not just ask. Please feel free to commit fixups directly if you don't like something. I tried my best to keep to your style.

mc-butler · 2024-06-03T06:37:19Z

Changed by andrew_b (@aborodin) on Jun 3, 2024 at 6:37 UTC (comment 9)

Let's move cleanup stuff to new branch 4524_cleanup or commit it directly to master.

Move #include <config.h> to .h is not a good idea. config.h must be included first in .c files, but indirectly includes via other headers became it not first.
As a result, ENABLE_EXT2FS_ATTR is not defined in local.c here:

#ifdef ENABLE_EXT2FS_ATTR
#include <e2p/e2p.h>            /* fgetflags(), fsetflags() */
#endif

but defined here:

  CC       local.lo
src/vfs/local/local.c: In function 'local_fgetflags':
src/vfs/local/local.c:198:12: warning: implicit declaration of function 'fgetflags'; did you mean 'mc_fgetflags'? [-Wimplicit-function-declaration]
     return fgetflags (path, flags);
            ^~~~~~~~~
            mc_fgetflags
src/vfs/local/local.c: In function 'local_fsetflags':
src/vfs/local/local.c:209:12: warning: implicit declaration of function 'fsetflags'; did you mean 'mc_fsetflags'? [-Wimplicit-function-declaration]
     return fsetflags (path, flags);
            ^~~~~~~~~
            mc_fsetflags

mc-butler · 2024-06-03T07:16:29Z

Changed by zaytsev (@zyv) on Jun 3, 2024 at 7:16 UTC (comment 10)

Let's move cleanup stuff to new branch 4524_cleanup or commit it directly to master.

Sorry, I don't get what you want. I think it's better to have a separate branch for nanoseconds stuff instead of doing everything in cleanup. It's a new feature supporting it correctly on AIX & BSDs. The commits doing the refactoring make it possible to implement the new feature centrally, so I would also keep them to this branch.

If you want to take [607fe8] and [c50106] elsewhere, no problem, but I would prefer master, so that I can rebase this branch and continue testing. Regarding [c50106], I'm not sure if something else has to be changed / removed. The function was present before glib 2.31 according to git blame of their tree, so maybe something went wrong at some point?

Move #include <config.h> to .h is not a good idea. config.h must be included first in .c files, but indirectly includes via other headers became it not first.

Ah, ok, thank you, I put it back. I thought it was unused, but I was wrong.

But am I allowed also to use config.h in h-files? Are there any rules for that? I need to know the structure of the struct and I want the functions to be in h-files, so that they can be inlined everywhere, where they are used.

mc-butler · 2024-06-03T08:00:17Z

Changed by andrew_b (@aborodin) on Jun 3, 2024 at 8:00 UTC (comment 10.11)

Replying to zaytsev:

If you want to take [607fe8] and [c50106] elsewhere, no problem, but I would prefer master,

Ok.

Regarding [c50106], I'm not sure if something else has to be changed / removed. The function was present before glib 2.31 according to git blame of their tree, so maybe something went wrong at some point?

That's my mistake. There was no need to redefine g_direct_equal() in our code because it's implemented in glib since 2005.

Move #include <config.h> to .h is not a good idea. config.h must be included first in .c files, but indirectly includes via other headers became it not first.

Ah, ok, thank you, I put it back. I thought it was unused, but I was wrong.

But am I allowed also to use config.h in h-files?

I've never seen <config.h> included in h-files, only in c-files.

Are there any rules for that?

Probably yes, but Google can't help me.

I need to know the structure of the struct and I want the functions to be in h-files, so that they can be inlined everywhere, where they are used.

Your h-files are included in c-files after config.h so no problems.
But why you want make function inline?

mc-butler · 2024-06-03T08:19:07Z

Changed by zaytsev (@zyv) on Jun 3, 2024 at 8:19 UTC (comment 12)

I've never seen <config.h> included in h-files, only in c-files.

Okay, I understand, so the rule is to use config.h in C-files so that it doesn't contaminate other files per mistake if used in an H-file, and use it first. I can check if there are other H-files where we include it by mistake.

But why you want make function inline?

These are nothing but accessor functions that reshuffle struct fields. Before my changes it was handled at every place where it was needed in the code. Now everywhere there is an extra function call, but the code is reused. I wanted to make it a function and not a macro, because macros are evil and non-type-safe. But it has some performance impact. For inline functions the impact is zero.

But if we don't care, then I can move these functions in a C-file, probably the impact will be some microseconds anyways which are not important...

mc-butler · 2024-06-03T14:42:10Z

Changed by zaytsev (@zyv) on Jun 3, 2024 at 14:42 UTC (comment 13)

I have added a commit implementing this, took the two build fixes in master and rebased the branch squashing the fixups.

mc-butler · 2024-06-15T14:43:55Z

Changed by andrew_b (@aborodin) on Jun 15, 2024 at 14:43 UTC (comment 14)

I've created an alternative branch with some fixups: 4542_nanoseconds_2
Initial [e89d65e17053e10912a2b1ed3afc9be06ec8c746]

mc-butler · 2024-06-15T14:44:43Z

Changed by andrew_b (@aborodin) on Jun 15, 2024 at 14:44 UTC (comment 15)

Type changed from enhancement to defect
Component changed from mc-core to mc-vfs

mc-butler · 2024-06-16T09:00:31Z

Changed by zaytsev (@zyv) on Jun 16, 2024 at 9:00 UTC (comment 16)

I'm a bit concerned about you removing config.h from vfs.h - it means vfs.h must be only included in C-files which include config.h themselves to work correctly. But I guess this is C life :-(

Thank you for the rest of the changes, I think I tried %ju but got some problems on Mac and AIX, and decided to use %llu. I will test again on Mac, AIX and I also wanted to check Solaris. Then report back.

mc-butler · 2024-06-16T10:00:46Z

Changed by andrew_b (@aborodin) on Jun 16, 2024 at 10:00 UTC (comment 16.17)

Replying to zaytsev:

I'm a bit concerned about you removing config.h from vfs.h - it means vfs.h must be only included in C-files which include config.h themselves to work correctly. But I guess this is C life :-(

Yes. We do compile c-files, not h-files. c-file must begin with include of config.h, then include system-wide h-files (from /usr/include), then project h-files.

Thank you for the rest of the changes, I think I tried %ju but got some problems on Mac and AIX, and decided to use %llu. I will test again on Mac, AIX and I also wanted to check Solaris. Then report back.

I get a warning for %llu:

src/vfs/shell/shell.c: In function 'shell_utime':
src/vfs/shell/shell.c:1490:69: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 7 has type 'long unsigned int' [-Wformat=]
 1490 |                               "SHELL_FILENAME=%s SHELL_FILEATIME=%llu SHELL_FILEMTIME=%llu "
      |                                                                  ~~~^
      |                                                                     |
      |                                                                     long long unsigned int
      |                                                                  %lu
 1491 |                               "SHELL_TOUCHATIME=%s SHELL_TOUCHMTIME=%s SHELL_TOUCHATIME_W_NSEC=\"%s\" "
 1492 |                               "SHELL_TOUCHMTIME_W_NSEC=\"%s\";\n", rpath, (uintmax_t) atime.tv_sec,
      |                                                                           ~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                                           |
      |                                                                           long unsigned int
src/vfs/shell/shell.c:1490:90: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 8 has type 'long unsigned int' [-Wformat=]
 1490 |                               "SHELL_FILENAME=%s SHELL_FILEATIME=%llu SHELL_FILEMTIME=%llu "
      |                                                                                       ~~~^
      |                                                                                          |
      |                                                                                          long long unsigned int
      |                                                                                       %lu
......
 1493 |                               (uintmax_t) mtime.tv_sec, utcatime, utcmtime, utcatime_w_nsec,
      |                               ~~~~~~~~~~~~~~~~~~~~~~~~                                    
      |                               |
      |                               long unsigned int

and system_data_types(7) says:

The length modifier for uintmax_t for the printf(3) and the scanf(3)
families of functions is j; resulting commonly in %ju or %jx for printing
uintmax_t values.

Conforming to: C99 and later; POSIX.1-2001 and later.

mc-butler · 2024-07-12T17:41:43Z

Changed by andrew_b (@aborodin) on Jul 12, 2024 at 17:41 UTC (comment 18)

@zaytsev: well? What is your plan here?

mc-butler · 2024-07-12T19:21:41Z

Changed by zaytsev (@zyv) on Jul 12, 2024 at 19:21 UTC (comment 19)

Been busy with other stuff, but probably should just test on Solaris and be done with it. Will see how I can get back to it.

mc-butler · 2024-07-27T19:14:35Z

Changed by zaytsev (@zyv) on Jul 27, 2024 at 19:14 UTC (comment 22)

Started building on Solaris, still struggling to compile the dependencies. Libffi is done:

libffi/libffi#846

MAKE=gmake AR=gar READELF=greadelf \
../configure --disable-static --enable-shared --prefix=/home/zaytsev/opt/libffi

Glib needs more work.

MAKE=gmake AR=gar \
CFLAGS=-D_XPG6 \
LIBFFI_LIBS=$HOME/opt/libffi/lib LIBFFI_CFLAGS=-I$HOME/opt/libffi/include \
../configure \
--disable-static --enable-shared \
--with-libiconv=gnu --disable-dtrace --disable-compile-warnings \
--prefix=/home/zaytsev/opt/glib

Glib is done with lots of makefile editing to remove -Wl,... and disable linking binaries / tests. Maybe newer versions than glib-2.43.92.tar.xz are not as bad, but at some point they started depending on a non-autotools build system... even more pain.

Slang was "easy":

MAKE=gmake CFLAGS=-D_XPG6 ./configure --prefix=$HOME/opt/slang
gmake install

mc was also okay, but patches are needed (see below):

XGETTEXT=gxgettext ./autogen.sh

GLIB_LIBS="-L$HOME/opt/glib/lib -lglib-2.0" \
GLIB_CFLAGS="-I$HOME/opt/glib/include/glib-2.0 -I$HOME/opt/glib/lib/glib-2.0/include" \
SLANG_LIBS="-L$HOME/opt/slang/lib -lslang" SLANG_CFLAGS=-I$HOME/opt/slang/include \
MAKE=gmake \
../configure --prefix=$HOME/opt/mc

mc-butler · 2024-07-27T19:22:08Z

Changed by zaytsev (@zyv) on Jul 27, 2024 at 19:22 UTC (comment 23)

May I commit the following two patches directly to master?

From 1be8bbb4cf3667317f4361c0b749d9ee68032cf1 Mon Sep 17 00:00:00 2001
From: "Yury V. Zaytsev" <yury@shurup.com>
Date: Sat, 27 Jul 2024 21:17:52 +0200
Subject: [PATCH 1/2] buildsys: support bootstrapping on Solaris by using
 backticks in autogen.sh

Signed-off-by: Yury V. Zaytsev <yury@shurup.com>
---
 autogen.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/autogen.sh b/autogen.sh
index ee76efc22..8c06da89c 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -2,7 +2,9 @@
 
 set -e
 
-srcdir="$(cd "$(dirname "$0")" && pwd)"
+# Use backticks to support ksh on Solaris
+basedir=`dirname "$0"`
+srcdir=`cd "$basedir" && pwd`
 
 cd "$srcdir"
 
-- 
2.39.3 (Apple Git-146)

From 889ef3d4a7f3048ff9979e2fc6332d93f0242eed Mon Sep 17 00:00:00 2001
From: "Yury V. Zaytsev" <yury@shurup.com>
Date: Sat, 27 Jul 2024 21:18:48 +0200
Subject: [PATCH 2/2] buildsys: fix ar check by doing it before toolchain
 checks implicitly looking for ar

Signed-off-by: Yury V. Zaytsev <yury@shurup.com>
---
 configure.ac | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 12556f563..21c0ca71b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,6 +36,10 @@ dnl ############################################################################
 dnl Check for compiler
 dnl ############################################################################
 
+dnl This should be checked before toolchain macros, otherwise they will remember
+dnl that ar cannot be found and linking via libtool will fail at a later stage
+AC_CHECK_TOOLS([AR], [ar gar])
+
 AC_PROG_CC
 AM_PROG_CC_C_O
 
@@ -211,8 +215,7 @@ dnl ############################################################################
 dnl Check for other tools
 dnl ############################################################################
 
-AC_CHECK_TOOL(AR, ar, ar)
-AC_CHECK_TOOL(INDENT, gindent, indent)
+AC_CHECK_TOOLS([INDENT], [gindent indent])
 mc_UNIT_TESTS
 
 
-- 
2.39.3 (Apple Git-146)

mc-butler · 2024-07-27T19:23:20Z

Changed by zaytsev (@zyv) on Jul 27, 2024 at 19:23 UTC (comment 24)

N.B. Tested nanosecond timestamps on Solaris, they work now. Solaris supports ls -alsE to show them.

mc-butler · 2024-07-27T20:07:52Z

Changed by zaytsev (@zyv) on Jul 27, 2024 at 20:07 UTC

Attachment Screenshot 2024-07-27 at 22.04.32.png (raw) added

mc-butler · 2024-07-27T20:09:59Z

Changed by zaytsev (@zyv) on Jul 27, 2024 at 20:09 UTC (comment 25)

I've retested on AIX 7.3 and it looks really good. The timestamps are also preserved.

I've looked at your changes, Andrew, and I agree with all of them, thank you for your help! Without you I've probably wouldn't come that far. How do we go about merging this branch? Shall I take yours, rebase and merge? Or you want to do this?

mc-butler · 2024-07-28T05:24:13Z

Changed by zaytsev (@zyv) on Jul 28, 2024 at 5:24 UTC (comment 26)

Rebased - changeset: [bfa0f041cf84ec169c3a72538bddf461dd8bd256] .

mc-butler · 2024-07-28T05:57:19Z

Changed by zaytsev (@zyv) on Jul 28, 2024 at 5:57 UTC (comment 27)

Description edited

mc-butler · 2024-07-28T05:58:34Z

Changed by zaytsev (@zyv) on Jul 28, 2024 at 5:58 UTC (comment 28)

Summary changed from Timestamps with nanosecond precision support is broken on macOS to Nanosecond precision timestamps support broken on macOS / BSD / AIX / Solaris

mc-butler · 2024-07-28T07:16:54Z

Changed by andrew_b (@aborodin) on Jul 28, 2024 at 7:16 UTC (comment 25.29)

Branch state changed from on review to approved
Votes set to andrew_b

Replying to zaytsev:

Shall I take yours, rebase and merge?

Yes, please.

Replying to zaytsev:

May I commit the following two patches directly to master?

Why not to the current branch before merge?

mc-butler · 2024-07-28T07:51:30Z

Changed by zaytsev (@zyv) on Jul 28, 2024 at 7:51 UTC (comment 30)

Why not to the current branch before merge?

No problem, can do that :) Last time you didn't want "unrelated" patches fixing the build system in the topic branches, which is why I asked.

mc-butler · 2024-07-28T08:02:59Z

Changed by zaytsev (@zyv) on Jul 28, 2024 at 8:02 UTC (comment 31)

Resolution set to fixed
Status changed from accepted to testing

Merged to master: [7e2cf63].

mc-butler · 2024-07-28T08:06:30Z

Changed by zaytsev (@zyv) on Jul 28, 2024 at 8:06 UTC (comment 32)

Status changed from testing to closed

mc-butler · 2024-07-28T08:07:17Z

Changed by zaytsev (@zyv) on Jul 28, 2024 at 8:07 UTC (comment 33)

Branch state changed from approved to merged
Votes changed from andrew_b to committed-master

mc-butler · 2025-02-22T11:29:29Z

Changed by zaytsev (@zyv) on Feb 22, 2025 at 11:29 UTC

Blocking changed from #3960 to #3960, #3983

mc-butler · 2025-02-22T11:31:48Z

Changed by zaytsev (@zyv) on Feb 22, 2025 at 11:31 UTC (comment 35)

Blocking changed from #3960, #3983 to #3821, #3927, #3960, #3983

mc-butler · 2025-02-22T11:38:54Z

Changed by zaytsev (@zyv) on Feb 22, 2025 at 11:38 UTC (comment 36)

Blocked by #4542 deleted
Blocking changed from #3821, #3927, #3960, #3983, #4542 to #3821, #3927, #3960, #3983

mc-butler · 2025-02-22T11:40:02Z

Changed by zaytsev (@zyv) on Feb 22, 2025 at 11:40 UTC

Blocking changed from #3821, #3927, #3960, #3983 to #3575, #3821, #3927, #3960, #3983

mc-butler closed this as completed Jul 28, 2024

mc-butler assigned zyv Feb 28, 2025

Nanosecond precision timestamps support broken on macOS / BSD / AIX / Solaris #4542

Nanosecond precision timestamps support broken on macOS / BSD / AIX / Solaris #4542

Comments

mc-butler commented May 21, 2024

mc-butler commented May 21, 2024

mc-butler commented May 31, 2024

mc-butler commented May 31, 2024

mc-butler commented May 31, 2024

mc-butler commented May 31, 2024

mc-butler commented May 31, 2024

mc-butler commented May 31, 2024

mc-butler commented Jun 2, 2024

mc-butler commented Jun 3, 2024

mc-butler commented Jun 3, 2024

mc-butler commented Jun 3, 2024

mc-butler commented Jun 3, 2024

mc-butler commented Jun 3, 2024

mc-butler commented Jun 15, 2024

mc-butler commented Jun 15, 2024

mc-butler commented Jun 16, 2024

mc-butler commented Jun 16, 2024

mc-butler commented Jul 12, 2024

mc-butler commented Jul 12, 2024

mc-butler commented Jul 27, 2024

mc-butler commented Jul 27, 2024

mc-butler commented Jul 27, 2024

mc-butler commented Jul 27, 2024

mc-butler commented Jul 27, 2024

mc-butler commented Jul 28, 2024

mc-butler commented Jul 28, 2024

mc-butler commented Jul 28, 2024

mc-butler commented Jul 28, 2024

mc-butler commented Jul 28, 2024

mc-butler commented Jul 28, 2024

mc-butler commented Jul 28, 2024

mc-butler commented Jul 28, 2024

mc-butler commented Feb 22, 2025

mc-butler commented Feb 22, 2025

mc-butler commented Feb 22, 2025

mc-butler commented Feb 22, 2025