Re: [PATCH] in place option for unexpand

[Pádraig Brady]

Pádraig Brady wrote:
> Sami Kerola wrote:
>> Good weekend,
>>
>> If you consider in-line option is useful for unexpand I could wrote
>> another patch for expand command which will do the same thing.
> 
> Thanks for this. I'm wondering about usefulness to other coreutils.
> Perhaps fmt,fold,nl,tac,tr would also benefit in addition to expand & 
> unexpand?
> On the other hand it's simple enough to achieve this using existing commands:
> 
> tf=$(mktemp)
> cleanup() { rm -f "$tf"; }
> trap "cleanup" EXIT
> for file in *.whatever; do
>     touch --reference="$file" "$tf"
>     chmod --reference="$file" "$tf"
>     unexpand "$file" > "$tf" &&
>     mv "$tf" "$file"
> done
> 
> That as well as being more flexible will also give you
> auto cleanup of the temp file on Ctrl-C for example.
> You would have to add signal handling to your patch to do that.
> So what do people think about bringing the above logic
> internal to certain utils?
> 
> BTW, I like the way you use $TMPDIR etc. contrary to the `sed` --in-place
> option which uses a tmp file in the same dir as the original.
> Using the same dir will fail in the case you've a file in a readonly dir
> bind mounted to a writeable location for example.
> On the otherhand it requires more code to support. For example
> I think your rename() will fail if $TMPDIR is on
> a different filesystem to the original file?
> 
> cheers,
> Pádraig.
> 
> p.s. wouldn't it be useful to give cp an option to copy attributes
> but not data, i.e. instead of the touch and chmod in the example above,
> one could do: cp --attributes-only --preserve=all "$file" "$tf"
> or alternatively give a --preserve=all option to `touch`?

Attached is a simple patch to support `cp --attributes-only`.
Please be extra critical of this since it's a new interface.
For example, perhaps it would be better to extend the --copy-contents
option with a "never" parameter? Perhaps we don't need this at all?

Our "replace" script exapmle would now be of the form:

tf=$(mktemp)
cleanup() { rm -f "$tf"; }
trap "cleanup" EXIT
for file in *.whatever; do
  cp --attr "$file" "$tf"
  unexpand "$file" > "$tf" &&
  mv "$tf" "$file"
done

cheers,
Pádraig.

>From a414bfbb4802649f768e3d222e542962cd038530 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Mon, 21 Sep 2009 08:43:03 +0100
Subject: [PATCH] cp: add an option to only copy the file attributes

* src/copy.c (copy_reg): Skip copying the file contents if specified
* src/copy.h (struct cp_options): Add a data_copy_required boolean
* src/cp.c (main): Default to copying data but don't if specified
* src/install.c: Default to copying data
* src/mv.c: Likewise
---
 src/copy.c    |    2 +-
 src/copy.h    |    4 ++++
 src/cp.c      |   10 +++++++++-
 src/install.c |    1 +
 src/mv.c      |    1 +
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index b7d113f..d19be04 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -490,7 +490,7 @@ copy_reg (char const *src_name, char const *dst_name,
   struct stat sb;
   struct stat src_open_sb;
   bool return_val = true;
-  bool data_copy_required = true;
+  bool data_copy_required = x->data_copy_required;
 
   source_desc = open (src_name,
                       (O_RDONLY | O_BINARY
diff --git a/src/copy.h b/src/copy.h
index 745533a..d931599 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -170,6 +170,10 @@ struct cp_options
      will be hard links to the same file (a copy of F).  */
   bool preserve_links;
 
+  /* Optionally copy the data either with CoW reflink files or
+     explicitly with the --attributes-only option.  */
+  bool data_copy_required;
+
   /* If true and any of the above (for preserve) file attributes cannot
      be applied to a destination file, treat it as a failure and return
      nonzero immediately.  E.g. for cp -p this must be true, for mv it
diff --git a/src/cp.c b/src/cp.c
index 2ba1dbf..909c7e4 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -72,7 +72,8 @@ struct dir_attr
    non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
 enum
 {
-  COPY_CONTENTS_OPTION = CHAR_MAX + 1,
+  ATTRIBUTES_ONLY_OPTION = CHAR_MAX + 1,
+  COPY_CONTENTS_OPTION,
   NO_PRESERVE_ATTRIBUTES_OPTION,
   PARENTS_OPTION,
   PRESERVE_ATTRIBUTES_OPTION,
@@ -115,6 +116,7 @@ ARGMATCH_VERIFY (reflink_type_string, reflink_type);
 static struct option const long_opts[] =
 {
   {"archive", no_argument, NULL, 'a'},
+  {"attributes-only", no_argument, NULL, ATTRIBUTES_ONLY_OPTION},
   {"backup", optional_argument, NULL, 'b'},
   {"copy-contents", no_argument, NULL, COPY_CONTENTS_OPTION},
   {"dereference", no_argument, NULL, 'L'},
@@ -167,6 +169,7 @@ Mandatory arguments to long options are mandatory for short 
options too.\n\
 "), stdout);
       fputs (_("\
   -a, --archive                same as -dR --preserve=all\n\
+      --attributes-only        don't copy the file data, just the attributes\n\
       --backup[=CONTROL]       make a backup of each existing destination 
file\n\
   -b                           like --backup but does not accept an argument\n\
       --copy-contents          copy contents of special files when recursive\n\
@@ -781,6 +784,7 @@ cp_option_init (struct cp_options *x)
   x->reduce_diagnostics = false;
   x->require_preserve_xattr = false;
 
+  x->data_copy_required = true;
   x->require_preserve = false;
   x->recursive = false;
   x->sparse_mode = SPARSE_AUTO;
@@ -962,6 +966,10 @@ main (int argc, char **argv)
             version_control_string = optarg;
           break;
 
+        case ATTRIBUTES_ONLY_OPTION:
+          x.data_copy_required = false;
+          break;
+
         case COPY_CONTENTS_OPTION:
           copy_contents = true;
           break;
diff --git a/src/install.c b/src/install.c
index fafa21a..6b9b7a4 100644
--- a/src/install.c
+++ b/src/install.c
@@ -282,6 +282,7 @@ cp_option_init (struct cp_options *x)
   x->preserve_mode = false;
   x->preserve_timestamps = false;
   x->reduce_diagnostics=false;
+  x->data_copy_required = true;
   x->require_preserve = false;
   x->require_preserve_context = false;
   x->require_preserve_xattr = false;
diff --git a/src/mv.c b/src/mv.c
index 97e9dd4..5e51202 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -119,6 +119,7 @@ cp_option_init (struct cp_options *x)
   x->preserve_timestamps = true;
   x->preserve_security_context = selinux_enabled;
   x->reduce_diagnostics = false;
+  x->data_copy_required = true;
   x->require_preserve = false;  /* FIXME: maybe make this an option */
   x->require_preserve_context = false;
   x->preserve_xattr = true;
-- 
1.6.2.5