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