Re: [Bug-tar] --one-file-system and "file changed" warning for mountpoint

[Sergey Poznyakoff]

Hi Nathan,

> Does anyone know of a reason that tar needs to perform the file-changed
> check on directories that it has skipped due to --one-file-system...?

There's no compelling reason, it's just an oversight. Please try
the attached patch.

Regards,
Sergey

PS: I'm going to release v.1.30 today. It looks a bit risky to apply
a patch of this size to about-to-be-released code, so I'm going to
postpone it until it is well tested.

>From e08d7825df0967edf5ea31fdb2a0269a3368f594 Mon Sep 17 00:00:00 2001
From: Sergey Poznyakoff <address@hidden>
Date: Sun, 17 Dec 2017 12:04:13 +0200
Subject: [PATCH] Don't do post-dumping checks on files that have been skipped.

* src/common.h (dump_status_skip): New status code.
* src/create.c (dump_dir0): Return enum dump_status.
(dump_dir): Likewise.
(dump_file0): Don't do additional checks (timestamp, etc)
if the file has been skipped.
---
 src/common.h |  3 ++-
 src/create.c | 63 ++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/src/common.h b/src/common.h
index bbe167e..275252d 100644
--- a/src/common.h
+++ b/src/common.h
@@ -492,7 +492,8 @@ enum dump_status
     dump_status_ok,
     dump_status_short,
     dump_status_fail,
-    dump_status_not_implemented
+    dump_status_not_implemented,
+    dump_status_skip
   };

 void add_exclusion_tag (const char *name, enum exclusion_tag_type type,
diff --git a/src/create.c b/src/create.c
index 35bcf5b..1537fb4 100644
--- a/src/create.c
+++ b/src/create.c
@@ -1119,9 +1119,11 @@ dump_regular_file (int fd, struct tar_stat_info *st)

 
 /* Copy info from the directory identified by ST into the archive.
-   DIRECTORY contains the directory's entries.  */
+   DIRECTORY contains the directory's entries. Return dump_status_ok
+   on success, dump_status_skip if the directory was not dumped, and
+   dump_status_fail on error. */

-static void
+static enum dump_status
 dump_dir0 (struct tar_stat_info *st, char const *directory)
 {
   bool top_level = ! st->parent;
@@ -1133,7 +1135,7 @@ dump_dir0 (struct tar_stat_info *st, char const 
*directory)

   blk = start_header (st);
   if (!blk)
-    return;
+    return dump_status_fail;

   info_attach_exclist (st);

@@ -1188,11 +1190,11 @@ dump_dir0 (struct tar_stat_info *st, char const 
*directory)
              set_next_block_after (blk + (bufsize - 1) / BLOCKSIZE);
            }
        }
-      return;
+      return dump_status_ok;
     }

   if (!recursion_option)
-    return;
+    return dump_status_skip;

   if (one_file_system_option
       && !top_level
@@ -1203,6 +1205,7 @@ dump_dir0 (struct tar_stat_info *st, char const 
*directory)
                 (0, 0,
                  _("%s: file is on a different filesystem; not dumped"),
                  quotearg_colon (st->orig_file_name)));
+      return dump_status_skip;
     }
   else
     {
@@ -1259,6 +1262,7 @@ dump_dir0 (struct tar_stat_info *st, char const 
*directory)
          break;
        }
     }
+  return dump_status_ok;
 }

 /* Ensure exactly one trailing slash.  */
@@ -1315,24 +1319,28 @@ get_directory_entries (struct tar_stat_info *st)
   return streamsavedir (st->dirstream, savedir_sort_order);
 }

-/* Dump the directory ST.  Return true if successful, false (emitting
-   diagnostics) otherwise.  Get ST's entries, recurse through its
-   subdirectories, and clean up file descriptors afterwards.  */
-static bool
+/* Dump the directory ST.  Return dump_status_ok if successful,
+   dump_status_skip if the directory was not eligible for dumping (e.g.
+   located on another file system while --one-file-system is in effect), and
+   dump_status_fail (emitting diagnostics) on error.  Get ST's entries,
+   recurse through its subdirectories, and clean up file descriptors
+   afterwards.  */
+static enum dump_status
 dump_dir (struct tar_stat_info *st)
 {
+  enum dump_status status;
   char *directory = get_directory_entries (st);
   if (! directory)
     {
       savedir_diag (st->orig_file_name);
-      return false;
+      return dump_status_fail;
     }

-  dump_dir0 (st, directory);
+  status = dump_dir0 (st, directory);

   restore_parent_fd (st);
   free (directory);
-  return true;
+  return status;
 }

 
@@ -1755,7 +1763,7 @@ dump_file0 (struct tar_stat_info *st, char const *name, 
char const *p)

   if (is_dir || S_ISREG (st->stat.st_mode) || S_ISCTG (st->stat.st_mode))
     {
-      bool ok;
+      enum dump_status status;
       struct stat final_stat;

       xattrs_acls_get (parentfd, name, st, 0, !is_dir);
@@ -1775,15 +1783,13 @@ dump_file0 (struct tar_stat_info *st, char const *name, 
char const *p)
              return;
            }

-         ok = dump_dir (st);
+         status = dump_dir (st);

          fd = st->fd;
          parentfd = top_level ? chdir_fd : parent->fd;
        }
       else
        {
-         enum dump_status status;
-
          if (fd && sparse_option && ST_IS_SPARSE (st->stat))
            {
              status = sparse_dump_file (fd, st);
@@ -1801,40 +1807,42 @@ dump_file0 (struct tar_stat_info *st, char const *name, 
char const *p)
              break;

            case dump_status_fail:
+           case dump_status_skip:
              break;

            case dump_status_not_implemented:
              abort ();
            }
-
-         ok = status == dump_status_ok;
        }

-      if (ok)
+      if (status == dump_status_ok)
        {
          if (fd < 0)
            {
              errno = - fd;
-             ok = false;
+             status = dump_status_fail;
            }
          else if (fd == 0)
            {
              if (parentfd < 0 && ! top_level)
                {
                  errno = - parentfd;
-                 ok = false;
+                 status = dump_status_fail;
                }
              else
-               ok = fstatat (parentfd, name, &final_stat, fstatat_flags) == 0;
+               status =
+                 fstatat (parentfd, name, &final_stat, fstatat_flags) == 0
+                 ? dump_status_ok : dump_status_fail;
            }
          else
-           ok = fstat (fd, &final_stat) == 0;
+           status = fstat (fd, &final_stat) == 0
+                      ? dump_status_ok : dump_status_fail;

-         if (! ok)
+         if (status == dump_status_fail)
            file_removed_diag (p, top_level, stat_diag);
        }

-      if (ok)
+      if (status == dump_status_ok)
        {
          if ((timespec_cmp (get_stat_ctime (&final_stat), original_ctime) != 0
               /* Original ctime will change if the file is a directory and
@@ -1853,8 +1861,9 @@ dump_file0 (struct tar_stat_info *st, char const *name, 
char const *p)
            utime_error (p);
        }

-      ok &= tar_stat_close (st);
-      if (ok && remove_files_option)
+      if (! tar_stat_close (st))
+       status = dump_status_fail;
+      if (status == dump_status_ok && remove_files_option)
        queue_deferred_unlink (p, is_dir);

       return;
--
1.8.4