[Bug-tar] Tar 1.23 patch proposed to enable symlink timestamp restoration

[MELENNEC Ronan DGAC/CEDRe]

Hi, hello,

I would like to propose a change (see attachment)
to Gnu Tar 1.23 in order to allow symbolic links to be restored
with their tar-archived timestamps restored as well
(on systems where the kernel and the C library support this).

I have failed to enable this functionality
by compiling tar-1.23 without source code changes,
even though I know for sure that my kernel
and C library support it (this can be proven
using "touch" compiled from Gnu coreutils 8.4).

I have noticed a patch to this effect
proposed last year by Carl Worth,
(see <http://www.mail-archive.com/bug-tar@gnu.org/msg02281.html>)
but apparently this patch is no longer applicable
to last released version (tar 1.23, March 2010) ;
source file having substantially changed since.

My patch only changes tar-1.23/src/extract.c :
- a test on file type barring symlink timestamps
from being restored is removed ;
- a call to utimens() is replaced by a call to lutimens().

The configuration scripts do not need to be changed.
The base code (typically providing lutimens()
as a wrapper around syscall function(s) such as utimensat())
is already functional ands does not need to be changed.

The tar -m option ("touch" option) still works as expected.

Limitations :
- as of today I can only test on Linux Ubuntu 9.10
(using a 2.6.31 kernel) ;
- I have not thoroughly analysed the relevant configuration
scripts.
Therefore I cannot make any assessment of portability.

My advice :
- this functionality is desirable as it aligns
tar with other tools such as
rsync and touch (the latter from Gnu coreutils 8.4).
- source code reuse from coreutils touch or possibly rsync
is suggested (as the configuration logic used in tar sources
remains somewhat complicated).

Cordially,
--
Ronan Melennec
<ronan.melennec "at" aviation-civile.gouv.fr>
--

--- tar-1.23/src/extract.c-orig	2010-01-26 12:28:09.000000000 +0100
+++ tar-1.23/src/extract.c	2010-04-21 12:40:51.000000000 +0200
@@ -253,34 +253,32 @@
 	  mode_t invert_permissions, enum permstatus permstatus,
 	  char typeflag)
 {
-  if (typeflag != SYMTYPE)
+  /* We do the utime before the chmod because some versions of utime are
+     broken and trash the modes of the file.  */
+
+  if (! touch_option && permstatus != INTERDIR_PERMSTATUS)
     {
-      /* We do the utime before the chmod because some versions of utime are
-	 broken and trash the modes of the file.  */
+      /* We set the accessed time to `now', which is really the time we
+         started extracting files, unless incremental_option is used, in
+         which case .st_atime is used.  */
+
+      /* FIXME: incremental_option should set ctime too, but how?  */
+
+      struct timespec ts[2];
+      if (incremental_option)
+	ts[0] = st->atime;
+      else
+	ts[0] = start_time;
+      ts[1] = st->mtime;
 
-      if (! touch_option && permstatus != INTERDIR_PERMSTATUS)
+      if (lutimens (file_name, ts) != 0)
+	utime_error (file_name);
+      else
 	{
-	  /* We set the accessed time to `now', which is really the time we
-	     started extracting files, unless incremental_option is used, in
-	     which case .st_atime is used.  */
-
-	  /* FIXME: incremental_option should set ctime too, but how?  */
-
-	  struct timespec ts[2];
-	  if (incremental_option)
-	    ts[0] = st->atime;
-	  else
-	    ts[0] = start_time;
-	  ts[1] = st->mtime;
-
-	  if (utimens (file_name, ts) != 0)
-	    utime_error (file_name);
-	  else
-	    {
-	      check_time (file_name, ts[0]);
-	      check_time (file_name, ts[1]);
-	    }
+	  check_time (file_name, ts[0]);
+	  check_time (file_name, ts[1]);
 	}
+    }
 
       /* Some systems allow non-root users to give files away.  Once this
 	 done, it is not possible anymore to change file permissions.
@@ -288,7 +286,6 @@
 	 they would apply to the wrong user, and there would be a race
 	 condition.  So, don't use systems that allow non-root users to
 	 give files away.  */
-    }
 
   if (0 < same_owner_option && permstatus != INTERDIR_PERMSTATUS)
     {