On 19/08/2024 03:44, Frank Heckenbach wrote:
install dereferences symlinks given as sources (and I think that's good). The "-C" option, however, uses lstat rather than stat in need_copy and returns true if !S_ISREG.So the (dereferenced) file will always be copied despite "-C". Even worse, since "-p" cannot be combined with "-C", the installed file will always get a new timestamp which may trigger rebuilds (e.g. if the file is a header) even if nothing was changed, e.g.: % touch a % ln -s a b % install -C b c % ls --time-style=+%T -l total 0 -rw------- 1 frank frank 0 23:46:51 a lrwxrwxrwx 1 frank frank 1 23:46:54 b -> a -rwxr-xr-x 1 frank frank 0 23:46:58 c % install -C b c % ls --time-style=+%T -l total 0 -rw------- 1 frank frank 0 23:46:51 a lrwxrwxrwx 1 frank frank 1 23:46:54 b -> a -rwxr-xr-x 1 frank frank 0 23:47:06 c % install -C b c % ls --time-style=+%T -l total 0 -rw------- 1 frank frank 0 23:46:51 a lrwxrwxrwx 1 frank frank 1 23:46:54 b -> a -rwxr-xr-x 1 frank frank 0 23:47:08 c % I asked Kamil Dudka who implemented "-C" originally and he pointed out that requiring both the files to be regular files was suggested by Jim Meyering in the review: https://lists.gnu.org/archive/html/bug-coreutils/2009-02/msg00106.html However, I don't see a real reason there, only his question: "Have you considered requiring that both files be `regular', too?" I don't know if install didn't dereference symlinks back then, but now that it does, it seems consistent to dereference them in the check as well, i.e. use stat rather than lstat.
I agree with your arguments to change this. I'm not sure if the suggestion above about regular files, was meant to preclude symlinks. Marking this as done, and I'll apply the attached later. thanks, Pádraig
From 692ae740939cb356d7a16cb1e17d959e68d1fa91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com> Date: Mon, 19 Aug 2024 12:43:09 +0100 Subject: [PATCH] install: dereference source symlinks when comparing * NEWS: Mention the change in behavior. * src/install.c (need_copy): s/lstat/stat/ for the source. * tests/install/install-C.sh: Add test cases (and improve existing test case which wan't valid due to the existing non standard modes on test files). Addresses https://bugs.gnu.org/72707 --- NEWS | 3 +++ src/install.c | 2 +- tests/install/install-C.sh | 12 +++++++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 65cc3fde0..e1d3f82d1 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,9 @@ GNU coreutils NEWS -*- outline -*- ** Changes in behavior + install -C now dereferences symlink sources when comparing, + rather than always treating as different and performing the copy. + ls's -f option now simply acts like -aU, instead of also ignoring some earlier options. For example 'ls -fl' and 'ls -lf' are now equivalent because -f no longer ignores an earlier -l. The new diff --git a/src/install.c b/src/install.c index 939db9cd4..85f2d9e11 100644 --- a/src/install.c +++ b/src/install.c @@ -177,7 +177,7 @@ need_copy (char const *src_name, char const *dest_name, return true; /* compare files using stat */ - if (lstat (src_name, &src_sb) != 0) + if (stat (src_name, &src_sb) != 0) return true; if (fstatat (dest_dirfd, dest_relname, &dest_sb, AT_SYMLINK_NOFOLLOW) != 0) diff --git a/tests/install/install-C.sh b/tests/install/install-C.sh index a6b332e14..ee3ee2467 100755 --- a/tests/install/install-C.sh +++ b/tests/install/install-C.sh @@ -79,8 +79,18 @@ ginstall -Cv -m$mode3 a b > out || fail=1 compare out out_installed_second || fail=1 # files are not regular files +ginstall -v -m$mode1 a b > out || fail=1 # reset to regular mode +compare out out_installed_second || fail=1 +# symlink source is always dereferenced (and so regular here) +ginstall -v -m$mode1 a d > out || fail=1 # create regular dest +echo "'a' -> 'd'" > out_installed_first_ad || framework_failure_ +compare out out_installed_first_ad || fail=1 ln -s a c || framework_failure_ -ln -s b d || framework_failure_ +ginstall -Cv -m$mode1 c d > out || fail=1 +compare out out_empty || fail=1 +# symlink (non regular) dest is always replaced +ln -nsf b d || framework_failure_ +ls -l a b c d > /tmp/pb.out ginstall -Cv -m$mode1 c d > out || fail=1 echo "removed 'd' 'c' -> 'd'" > out_installed_second_cd -- 2.45.2
