GNU bug report logs - #72707
install: need_copy (for "-C") should use stat instead of lstat

Previous Next

Package: coreutils;

Reported by: Frank Heckenbach <f.heckenbach <at> fh-soft.de>

Date: Mon, 19 Aug 2024 04:40:01 UTC

Severity: normal

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 72707 in the body.
You can then email your comments to 72707 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-coreutils <at> gnu.org:
bug#72707; Package coreutils. (Mon, 19 Aug 2024 04:40:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Frank Heckenbach <f.heckenbach <at> fh-soft.de>:
New bug report received and forwarded. Copy sent to bug-coreutils <at> gnu.org. (Mon, 19 Aug 2024 04:40:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Frank Heckenbach <f.heckenbach <at> fh-soft.de>
To: bug-coreutils <at> gnu.org
Subject: install: need_copy (for "-C") should use stat instead of lstat
Date: Mon, 19 Aug 2024 04:44:52 +0200
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.




Reply sent to Pádraig Brady <P <at> draigBrady.com>:
You have taken responsibility. (Mon, 19 Aug 2024 11:51:02 GMT) Full text and rfc822 format available.

Notification sent to Frank Heckenbach <f.heckenbach <at> fh-soft.de>:
bug acknowledged by developer. (Mon, 19 Aug 2024 11:51:02 GMT) Full text and rfc822 format available.

Message #10 received at 72707-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Frank Heckenbach <f.heckenbach <at> fh-soft.de>, 72707-done <at> debbugs.gnu.org
Subject: Re: bug#72707: install: need_copy (for "-C") should use stat instead
 of lstat
Date: Mon, 19 Aug 2024 12:48:37 +0100
[Message part 1 (text/plain, inline)]
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
[install-C-deref.patch (text/x-patch, attachment)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 17 Sep 2024 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 276 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.