Skip to content

import&copy: keep the original access and modified timestamps#12357

Merged
TurboGit merged 1 commit intodarktable-org:masterfrom
phweyland:import-copy-timestamps
Aug 22, 2022
Merged

import&copy: keep the original access and modified timestamps#12357
TurboGit merged 1 commit intodarktable-org:masterfrom
phweyland:import-copy-timestamps

Conversation

@phweyland
Copy link
Copy Markdown
Contributor

@phweyland phweyland commented Aug 19, 2022

fixes #12355

EDIT: The original issue is common to all OS, but the solution may be different. I've tested on Linux Debian.

@phweyland phweyland added the bugfix pull request fixing a bug label Aug 20, 2022
@phweyland phweyland force-pushed the import-copy-timestamps branch from 48dfe2c to 0a92ab0 Compare August 20, 2022 02:22
@phweyland phweyland force-pushed the import-copy-timestamps branch from 0a92ab0 to 87c67b4 Compare August 20, 2022 02:24
@TurboGit TurboGit added this to the 4.0.1 milestone Aug 22, 2022
Copy link
Copy Markdown
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@TurboGit TurboGit merged commit 7603720 into darktable-org:master Aug 22, 2022
@gi-man
Copy link
Copy Markdown
Contributor

gi-man commented Aug 22, 2022

I'm getting a failure trying to build this PR on windows:

C:/msys64/home/gman/test/darktable/src/control/jobs/control_jobs.c:2109:37: error: 'struct stat' has no member named 'st_atim'; did you mean 'st_atime'?
 2109 |     times[0].tv_sec = (long)statbuf.st_atim.tv_sec;
      |                                     ^~~~~~~
      |                                     st_atime

3 min too late.

@parafin
Copy link
Copy Markdown
Member

parafin commented Aug 22, 2022

This failure was caught by CI, so I wouldn't say anyone was too late, but the opposite...

@gi-man
Copy link
Copy Markdown
Contributor

gi-man commented Aug 22, 2022

@TurboGit It is now failing on master. I built master 15min ago (before the merge), without a problem. I was testing the PR and noticed the problem.

@TurboGit
Copy link
Copy Markdown
Member

This failure was caught by CI, so I wouldn't say anyone was too late, but the opposite...

Sorry CI is failing on all PR so I did not bother looking on this one. My bad.

@TurboGit
Copy link
Copy Markdown
Member

I have now reverted this change until the Windows issue is solved. I have tried looking for something equivalent to tv_sec but it seems not that simple and I know next to nothing about Windows. So please, can a Windows dev shine in there? TIA.

@TurboGit
Copy link
Copy Markdown
Member

@phweyland : Reverted as breaking build on Windows.

@phweyland
Copy link
Copy Markdown
Contributor Author

From https://www.man7.org/linux/man-pages/man2/stat.2.html:

       struct stat {
           dev_t     st_dev;         /* ID of device containing file */
           ino_t     st_ino;         /* Inode number */
           mode_t    st_mode;        /* File type and mode */
           nlink_t   st_nlink;       /* Number of hard links */
           uid_t     st_uid;         /* User ID of owner */
           gid_t     st_gid;         /* Group ID of owner */
           dev_t     st_rdev;        /* Device ID (if special file) */
           off_t     st_size;        /* Total size, in bytes */
           blksize_t st_blksize;     /* Block size for filesystem I/O */
           blkcnt_t  st_blocks;      /* Number of 512B blocks allocated */

           /* Since Linux 2.6, the kernel supports nanosecond
              precision for the following timestamp fields.
              For the details before Linux 2.6, see NOTES. */

           struct timespec st_atim;  /* Time of last access */
           struct timespec st_mtim;  /* Time of last modification */
           struct timespec st_ctim;  /* Time of last status change */

       #define st_atime st_atim.tv_sec      /* Backward compatibility */
       #define st_mtime st_mtim.tv_sec
       #define st_ctime st_ctim.tv_sec
       };

The "notes' say:

   Since kernel 2.5.48, the stat structure supports nanosecond
   resolution for the three file timestamp fields.  The nanosecond
   components of each timestamp are available via names of the form
   st_atim.tv_nsec, if suitable feature test macros are defined.
   Nanosecond timestamps were standardized in POSIX.1-2008, and,
   starting with version 2.12, glibc exposes the nanosecond
   component names if _POSIX_C_SOURCE is defined with the value
   200809L or greater, or _XOPEN_SOURCE is defined with the value
   700 or greater.  Up to and including glibc 2.19, the definitions
   of the nanoseconds components are also defined if _BSD_SOURCE or
   _SVID_SOURCE is defined.  If none of the aforementioned macros
   are defined, then the nanosecond values are exposed with names of
   the form st_atimensec.

I haven't found version requirement for darktable...
Any direction is welcome.

@TurboGit
Copy link
Copy Markdown
Member

Yes for Linux no issue. Looking at MSDN it looks like we need GetFileTime() and FileTimeToSystemTime().

The later will give us:

typedef struct _SYSTEMTIME {
  WORD wYear;
  WORD wMonth;
  WORD wDayOfWeek;
  WORD wDay;
  WORD wHour;
  WORD wMinute;
  WORD wSecond;
  WORD wMilliseconds;
} SYSTEMTIME, *PSYSTEMTIME, *LPSYSTEMTIME;

And hopefully wSecond/wMilliseconds are what we are looking for.

@phweyland
Copy link
Copy Markdown
Contributor Author

Looking at MSDN it looks like we need GetFileTime() and FileTimeToSystemTime()

Thanks! Making a try.
I continue with the same PR or I create a new one ?

@TurboGit
Copy link
Copy Markdown
Member

A new PR please, I cannot reopen this one. TIA.

@WMAM
Copy link
Copy Markdown

WMAM commented Sep 5, 2022

Sorry if I duplicate this report.
This is to confirm that this bug has not been fixed in the latest development builds... at least not in Windows 11. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix pull request fixing a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The "copy&import" operation results in a change in the date and time of the raw file.

5 participants