On 16/09/15 15:29, Chris Severance wrote:
> The following script of 4 equivalent installs does not complete.
> 
> #!/usr/bin/bash
> 
> set -e
> set -u
> 
> # Bug report for install (GNU coreutils) 8.24
> # Arch Linux gq 4.1.6-1-ARCH #1 SMP PREEMPT Mon Aug 17 08:52:28 CEST
> 2015 x86_64 GNU/Linux
> # The following 4 install do the same thing. One will fail.
> 
> rm -rf 'testin' 'testout'
> mkdir -p 'testin/d2'
> touch 'testin/f1' 'testin/d2/f2' 'f3'
> echo 'Try 1'
> install -Dpm644 'testin/f1' 'testin/d2/f2' 'f3' -t "${PWD}/testout/"
> rm -rf 'testout'
> echo 'Try 2'
> install -Dpm644 "${PWD}/testin/f1" "${PWD}/testin/d2/f2" 'f3' -t
> "testout/"
> rm -rf 'testout'
> echo 'Try 3'
> install -Dpm644 'f3' "${PWD}/testin/f1" "${PWD}/testin/d2/f2" -t
> "${PWD}/testout/"
> rm -rf 'testout'
> echo 'Try 4 will fail'
> install -Dpm644 "${PWD}/testin/f1" "${PWD}/testin/d2/f2" 'f3' -t
> "${PWD}/testout/"
> # install: cannot stat 'f3': No such file or directory

Thanks for the excellent summary.

The first attached patch should fix this.
Also included are another 2 improvements
I noticed while implementing the fix.

thanks,
Pádraig.

>From ef4a57cd52a976d82336938ca04bc8af4b5151be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Sat, 2 Jan 2016 18:38:37 +0000
Subject: [PATCH 1/3] install: fix relative copies to absolute directory with
 -D

* src/install.c (mkancesdirs_safe_wd): Unconditionally
restore the current working directory when possible called
multiple times (from install_file_in_dir()).
* tests/install/create-leading.sh: Add a test case.
* NEWS: Mention the fix.
Fixes http://bugs.gnu.org/21497
---
 NEWS                            |  4 ++++
 src/install.c                   | 13 ++++++++-----
 tests/install/create-leading.sh |  9 ++++++++-
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 863be18..72d69b8 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   cut --fields no longer outputs extraneous characters on some uClibc configs.
   [bug introduced in coreutils-6.11]
 
+  install -D again copies relative file names when absolute file names
+  are also specified along with an absolute destination directory name.
+  [bug introduced in coreutils-6.2]
+
   ls no longer prematurely wraps lines when printing short file names.
   [bug introduced in coreutils-5.1.0]
 
diff --git a/src/install.c b/src/install.c
index 1bdb757..6efb29e 100644
--- a/src/install.c
+++ b/src/install.c
@@ -704,14 +704,17 @@ install_file_in_file (const char *from, const char *to,
   return change_attributes (to);
 }
 
-/* Copy file FROM onto file TO, creating any missing parent directories of TO.
+/* Create any missing parent directories of TO,
+   while maintaining the current Working Directory.
    Return true if successful.  */
 
 static bool
-mkancesdirs_safe_wd (char const *from, char *to, struct cp_options *x)
+mkancesdirs_safe_wd (char const *from, char *to, struct cp_options *x,
+                     bool save_always)
 {
   bool save_working_directory =
-    ! (IS_ABSOLUTE_FILE_NAME (from) && IS_ABSOLUTE_FILE_NAME (to));
+    save_always
+    || ! (IS_ABSOLUTE_FILE_NAME (from) && IS_ABSOLUTE_FILE_NAME (to));
   int status = EXIT_SUCCESS;
 
   struct savewd wd;
@@ -749,7 +752,7 @@ static bool
 install_file_in_file_parents (char const *from, char *to,
                               const struct cp_options *x)
 {
-  return (mkancesdirs_safe_wd (from, to, (struct cp_options *)x)
+  return (mkancesdirs_safe_wd (from, to, (struct cp_options *)x, false)
           && install_file_in_file (from, to, x));
 }
 
@@ -766,7 +769,7 @@ install_file_in_dir (const char *from, const char *to_dir,
   bool ret = true;
 
   if (mkdir_and_install)
-    ret = mkancesdirs_safe_wd (from, to, (struct cp_options *)x);
+    ret = mkancesdirs_safe_wd (from, to, (struct cp_options *)x, true);
 
   ret = ret && install_file_in_file (from, to, x);
   free (to);
diff --git a/tests/install/create-leading.sh b/tests/install/create-leading.sh
index 8971d5f..62f74f5 100755
--- a/tests/install/create-leading.sh
+++ b/tests/install/create-leading.sh
@@ -24,7 +24,7 @@ print_ver_ ginstall
 
 
 file=file
-echo foo > $file
+echo foo > $file || framework_failure_
 
 # Before 4.0q, this would mistakenly create $file, not 'dest'
 # in no-dir1/no-dir2/.
@@ -32,4 +32,11 @@ ginstall -D $file no-dir1/no-dir2/dest || fail=1
 test -d no-dir1/no-dir2 || fail=1
 test -r no-dir1/no-dir2/dest || fail=1
 
+# Between 6.1 and 8.24, this would not copy $file
+# due to incorrectly modified working directory
+mkdir dir1 || framework_failure_
+touch dir1/file1 || framework_failure_
+ginstall -D $PWD/dir1/file1 $file -t $PWD/no-dir2/ || fail=1
+test -r no-dir2/$file && test -r no-dir2/file1 || fail=1
+
 Exit $fail
-- 
2.5.0


>From 17c2ec8ee87ac1e736341d97945b991dc3b48da5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Sat, 2 Jan 2016 22:14:41 +0000
Subject: [PATCH 2/3] install: only attempt to create a target dir once

* src/install.c (main): As an optimization, when calling
install_file_in_dir() for each file, only attempt to create
the target directory once, as this is invariant over the loop.
---
 src/install.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/install.c b/src/install.c
index 6efb29e..3f0ca74 100644
--- a/src/install.c
+++ b/src/install.c
@@ -1043,7 +1043,7 @@ main (int argc, char **argv)
           dest_info_init (&x);
           for (i = 0; i < n_files; i++)
             if (! install_file_in_dir (file[i], target_directory, &x,
-                                       mkdir_and_install))
+                                       i == 0 && mkdir_and_install))
               exit_status = EXIT_FAILURE;
         }
     }
-- 
2.5.0


>From adb2e560f736dae95640fd9bf0d6faa9cbfcd217 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Sat, 2 Jan 2016 22:22:55 +0000
Subject: [PATCH 3/3] doc: mention in more places that -D will create
 --target-directory

* src/install.c (usage): Mention this commonly required functionality
in the -D option description.
* doc/coreutils.texi (install invocation): Likewise for the
--target-directory description.
---
 doc/coreutils.texi | 1 +
 src/install.c      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 62be66a..76e4182 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -9024,6 +9024,7 @@ Program used to strip binaries.
 @optBackupSuffix
 
 @optTargetDirectory
+Also specifying the @option{-D} option will ensure the directory is present.
 
 @optNoTargetDirectory
 
diff --git a/src/install.c b/src/install.c
index 3f0ca74..2ff279c 100644
--- a/src/install.c
+++ b/src/install.c
@@ -632,6 +632,7 @@ In the 4th form, create all components of the given DIRECTORY(ies).\n\
 "), stdout);
       fputs (_("\
   -D                  create all leading components of DEST except the last,\n\
+                        or all components of --target-directory,\n\
                         then copy SOURCE to DEST\n\
   -g, --group=GROUP   set group ownership, instead of process' current group\n\
   -m, --mode=MODE     set permission mode (as in chmod), instead of rwxr-xr-x\n\
-- 
2.5.0

Reply via email to