From debbugs-submit-bounces@debbugs.gnu.org Mon Apr 04 06:48:58 2011 Received: (at submit) by debbugs.gnu.org; 4 Apr 2011 10:48:58 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q6hL7-0005rm-KT for submit@debbugs.gnu.org; Mon, 04 Apr 2011 06:48:58 -0400 Received: from eggs.gnu.org ([140.186.70.92]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Q6g64-00047n-5T for submit@debbugs.gnu.org; Mon, 04 Apr 2011 05:29:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q6g5y-0004cT-0R for submit@debbugs.gnu.org; Mon, 04 Apr 2011 05:29:14 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RFC_ABUSE_POST autolearn=unavailable version=3.3.1 Received: from lists.gnu.org ([199.232.76.165]:47152) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q6g5x-0004bJ-OO for submit@debbugs.gnu.org; Mon, 04 Apr 2011 05:29:13 -0400 Received: from [140.186.70.92] (port=56759 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q6g5s-00026o-A3 for bug-coreutils@gnu.org; Mon, 04 Apr 2011 05:29:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q6g5p-0004Yi-NC for bug-coreutils@gnu.org; Mon, 04 Apr 2011 05:29:06 -0400 Received: from hol-smtp31.benteler.de ([194.246.46.251]:52468 helo=HOL-SMTP11.benteler-alu.com) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1Q6g5p-0004X0-Cb for bug-coreutils@gnu.org; Mon, 04 Apr 2011 05:29:05 -0400 Received: from (unknown [172.30.4.18]) by HOL-SMTP11.benteler-alu.com with smtp id 521b_f964_f96b1138_5e9d_11e0_9a34_00219b9e4c98; Mon, 04 Apr 2011 11:29:01 +0200 Subject: cp -au : New hard links in source becomes new files at destination when using cp -au X-KeepSent: F7AA4B2C:3BBF4643-C1257868:0030B81E; type=4; name=$KeepSent To: bug-coreutils@gnu.org X-Mailer: Lotus Notes Release 7.0.3 HF1091 June 18, 2009 Message-ID: From: Odd.Harry.Mannsverk@benteler-alu.com Date: Mon, 4 Apr 2011 11:29:00 +0200 X-MIMETrack: Serialize by Router on RFS01/Spoke/Benteler(Release 8.5.2 HF16|August 30, 2010) at 04.04.2011 11:29:00 MIME-Version: 1.0 Content-type: text/plain; charset=US-ASCII X-NAI-Spam-Flag: NO X-NAI-Spam-Threshold: 6 X-NAI-Spam-Score: 0 X-NAI-Spam-Version: 2.2.0.9286 : core <3816> : streams <616773> : uri <842711> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 199.232.76.165 X-Spam-Score: -6.6 (------) X-Debbugs-Envelope-To: submit X-Mailman-Approved-At: Mon, 04 Apr 2011 06:48:56 -0400 Cc: Hans.Kristian.Nordengen@ergo.no X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.6 (------) Hi I have tried to use the command cp combining the -a and the -u options. I had to stop the copying process midways and restarted it again, and to my suprice the diskusage at the destination was 10 -20 % larger than the diskusage at the source and my disks ran full even though the destination disks was the same size as the source disks. My disks contained rsync snapshot backup data so every inod had typically 15 hard links to it. I found out that when I ran cp -au to update the target, cp -au creates new files in the destination when there is a new hard link in the source to a file already existing in the destination directory. This is not the behaviour I expected. I got the same behavior on: SUSE Linux Enterprise Server 11 (x86_64) and Ubuntu 10.04 I have included a series of commands to: create a source directory with hard linked files, make folder target, and copy the directory to this folder add a new hard link in the source folder update the target folder with the command cp -au listing the directory contents of source and target to show that the source contains 3 hard links to the same file, in the target directory hardlink2 has become a separate identical file. If you disagree with me that this is not the desired behaviour of cp -au, please let me know why. If you agree, I hope the behaviour of cp -au will change some time in the future. regards Odd Harry Mannsverk > mkdir source > touch source/file1 > ln source/file1 source/hardlink1 > mkdir target > cp -a source target > ln source/file1 source/hardlink2 > cp -au source target > ls -1iR source source: 229418 file1 229418 hardlink1 229418 hardlink2 > ls -1iR target target: 229414 source target/source: 229415 file1 229415 hardlink1 229416 hardlink2 > From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 07:42:33 2011 Received: (at 8419-done) by debbugs.gnu.org; 25 Jul 2011 11:42:34 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlJYP-0007hI-39 for submit@debbugs.gnu.org; Mon, 25 Jul 2011 07:42:33 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlJYM-0007h5-1u for 8419-done@debbugs.gnu.org; Mon, 25 Jul 2011 07:42:32 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 346BC6007C; Mon, 25 Jul 2011 13:42:24 +0200 (CEST) From: Jim Meyering To: Odd.Harry.Mannsverk@benteler-alu.com Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au In-Reply-To: (Odd Harry Mannsverk's message of "Mon, 4 Apr 2011 11:29:00 +0200") References: Date: Mon, 25 Jul 2011 13:42:24 +0200 Message-ID: <874o2a60yn.fsf@rho.meyering.net> Lines: 147 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: 8419-done Cc: 8419-done@debbugs.gnu.org, Hans.Kristian.Nordengen@ergo.no X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) Odd.Harry.Mannsverk@benteler-alu.com wrote: > I have tried to use the command cp combining the -a and the -u options. > I had to stop the copying process midways and restarted it again, and to my > suprice the diskusage at the destination was 10 -20 % larger than the > diskusage at the source and my disks ran full even though the destination > disks was the same size as the source disks. ... Thank you for a fine bug report. That is indeed a bug, and it affects the latest release, coreutils-8.12. I confirmed that it afflicts fileutils-3.16 too, so this bug has probably been present since the initial implementation. Here's the fix I expect to push: >From 3095daab7f6d7980b77e01d97d75e702ce4a2e63 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 25 Jul 2011 11:31:01 +0200 Subject: [PATCH] cp -up: preserve all hard links * src/copy.c (copy_internal): With --update (-u), this function would return early once it found that the destination is not older than the source, *without* recording the source-dev/ino--to--dest_name mapping. That mapping is required in order to preserve src hard links in the destination tree, so when using cp with --update and --preserve=links (perhaps via -p or -a), cp could fail to preserve one hard link per inode when at least one of the hard-linked names already exists in the destination tree. Reported by Odd Harry Mannsverk in http://debbugs.gnu.org/8419. * tests/cp/preserve-link: New file. Exercise the flaw/fix. * tests/Makefile.am (TESTS): Add it. * NEWS (Bug fixes): Mention it. --- NEWS | 6 ++++++ src/copy.c | 12 ++++++++++++ tests/Makefile.am | 1 + tests/cp/preserve-link | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 0 deletions(-) create mode 100755 tests/cp/preserve-link diff --git a/NEWS b/NEWS index 0720719..416060f 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,12 @@ GNU coreutils NEWS -*- outline -*- I.E. for skipped files, the original ownership is output, not the new one. [bug introduced in sh-utils-2.0g] + cp -u -p would fail to preserve one hard link for each up-to-date copy + of a src-hard-linked name in the destination tree. I.e., if s/a and s/b + are hard-linked and dst/s/a is up to date, "cp -up s dst" would copy s/b + to dst/s/b rather than simply linking dst/s/b to dst/s/a. + [This bug appears to have been present in "the beginning".] + printf '%d' '"' no longer accesses out-of-bounds memory in the diagnostic. [bug introduced in sh-utils-1.16] diff --git a/src/copy.c b/src/copy.c index c17b942..df8b1db 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1628,6 +1628,17 @@ copy_internal (char const *src_name, char const *dst_name, end up removing the source file. */ if (rename_succeeded) *rename_succeeded = true; + + /* However, we still must record that we've processed + this src/dest pair, in case this source file is + hard-linked to another one. In that case, we'll use + the mapping information to link the corresponding + destination names. */ + earlier_file = remember_copied (dst_name, src_sb.st_ino, + src_sb.st_dev); + if (earlier_file) + goto create_hard_link; + return true; } } @@ -1948,6 +1959,7 @@ copy_internal (char const *src_name, char const *dst_name, } else { + create_hard_link:; /* We want to guarantee that symlinks are not followed. */ bool link_failed = (linkat (AT_FDCWD, earlier_file, AT_FDCWD, dst_name, 0) != 0); diff --git a/tests/Makefile.am b/tests/Makefile.am index ebd1b11..0a83dae 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -341,6 +341,7 @@ TESTS = \ cp/parent-perm-race \ cp/perm \ cp/preserve-2 \ + cp/preserve-link \ cp/preserve-slink-time \ cp/proc-short-read \ cp/proc-zero-len \ diff --git a/tests/cp/preserve-link b/tests/cp/preserve-link new file mode 100755 index 0000000..d0da873 --- /dev/null +++ b/tests/cp/preserve-link @@ -0,0 +1,40 @@ +#!/bin/sh +# Exercise the fix for http://debbugs.gnu.org/8419 + +# Copyright (C) 2011 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +. "${srcdir=.}/init.sh"; path_prepend_ ../src +print_ver_ cp + +same_inode() +{ + local u v + u=$(stat --format %i "$1") && + v=$(stat --format %i "$2") && test "$u" = "$v" +} + +mkdir -p s t/s || framework_failure_ +touch s/f t/s/f || framework_failure_ +ln s/f s/link || framework_failure_ + +# This must create a hard link, t/s/link, to the existing file, t/s/f. +# With cp from coreutils-8.12 and prior, it would mistakenly copy +# the file rather than creating the link. +cp -au s t || fail=1 + +same_inode t/s/f t/s/link || fail=1 + +Exit $fail -- 1.7.6.609.gbf6a9 From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 09:18:25 2011 Received: (at 8419-done) by debbugs.gnu.org; 25 Jul 2011 13:18:25 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlL3A-0001NN-Ow for submit@debbugs.gnu.org; Mon, 25 Jul 2011 09:18:25 -0400 Received: from senmx12-mx.siemens-enterprise.com ([62.134.46.10]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlL38-0001NA-NH for 8419-done@debbugs.gnu.org; Mon, 25 Jul 2011 09:18:23 -0400 Received: from MCHP064A.global-ad.net (unknown [172.29.37.63]) by senmx12-mx.siemens-enterprise.com (Server) with ESMTP id 91A5423F04AC; Mon, 25 Jul 2011 15:18:16 +0200 (CEST) Received: from MCHP058A.global-ad.net ([172.29.37.57]) by MCHP064A.global-ad.net ([172.29.37.63]) with mapi; Mon, 25 Jul 2011 15:18:13 +0200 From: "Voelker, Bernhard" To: Jim Meyering , "Odd.Harry.Mannsverk@benteler-alu.com" Date: Mon, 25 Jul 2011 15:18:13 +0200 Subject: RE: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au Thread-Topic: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au Thread-Index: AcxKwAkysFUqvCTcTeONBYsIoaojSwADQ3EQ Message-ID: <7856072A9D04C24B82DFE2B1112FE38A08FE2D4179@MCHP058A.global-ad.net> References: <874o2a60yn.fsf@rho.meyering.net> In-Reply-To: <874o2a60yn.fsf@rho.meyering.net> Accept-Language: de-DE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: de-DE, en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Spam-Score: -3.4 (---) X-Debbugs-Envelope-To: 8419-done Cc: "8419-done@debbugs.gnu.org" <8419-done@debbugs.gnu.org>, "Hans.Kristian.Nordengen@ergo.no" X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.4 (---) Jim Meyering wrote: + create_hard_link:; Is there a specific reason for the additional empty statement? Also seen in: find . -name '*.c' | xargs grep ':;' ./src/head.c: free_mem:; ./src/kill.c: no_more_options:; ./src/od.c:cleanup:; ./src/paste.c: done:; ./src/printf.c: no_more_flag_characters:; Berny= From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 09:20:54 2011 Received: (at 8419-done) by debbugs.gnu.org; 25 Jul 2011 13:20:54 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlL5a-0001Qw-5F for submit@debbugs.gnu.org; Mon, 25 Jul 2011 09:20:54 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlL5X-0001Qf-W3 for 8419-done@debbugs.gnu.org; Mon, 25 Jul 2011 09:20:52 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 1059C60092; Mon, 25 Jul 2011 15:20:46 +0200 (CEST) From: Jim Meyering To: "Voelker\, Bernhard" Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au In-Reply-To: <7856072A9D04C24B82DFE2B1112FE38A08FE2D4179@MCHP058A.global-ad.net> (Bernhard Voelker's message of "Mon, 25 Jul 2011 15:18:13 +0200") References: <874o2a60yn.fsf@rho.meyering.net> <7856072A9D04C24B82DFE2B1112FE38A08FE2D4179@MCHP058A.global-ad.net> Date: Mon, 25 Jul 2011 15:20:45 +0200 Message-ID: <87ipqq4hua.fsf@rho.meyering.net> Lines: 17 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: 8419-done Cc: "8419-done@debbugs.gnu.org" <8419-done@debbugs.gnu.org>, "Hans.Kristian.Nordengen@ergo.no" , "Odd.Harry.Mannsverk@benteler-alu.com" X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) Voelker, Bernhard wrote: > Jim Meyering wrote: > + create_hard_link:; > > Is there a specific reason for the additional empty statement? > > Also seen in: > find . -name '*.c' | xargs grep ':;' > ./src/head.c: free_mem:; > ./src/kill.c: no_more_options:; > ./src/od.c:cleanup:; > ./src/paste.c: done:; > ./src/printf.c: no_more_flag_characters:; Yes. It won't compile without that, since the thing right after it is a declaration, and not a statement. From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 09:38:31 2011 Received: (at 8419) by debbugs.gnu.org; 25 Jul 2011 13:38:31 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlLMd-0001oA-Cz for submit@debbugs.gnu.org; Mon, 25 Jul 2011 09:38:31 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1QlLMa-0001nx-Hr for 8419@debbugs.gnu.org; Mon, 25 Jul 2011 09:38:29 -0400 Received: (qmail 45196 invoked from network); 25 Jul 2011 13:38:22 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 25 Jul 2011 13:38:22 -0000 Message-ID: <4E2D712B.5030004@draigBrady.com> Date: Mon, 25 Jul 2011 14:35:39 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: 8419@debbugs.gnu.org, jim@meyering.net Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au References: <874o2a60yn.fsf@rho.meyering.net> In-Reply-To: <874o2a60yn.fsf@rho.meyering.net> X-Enigmail-Version: 1.0.1 Content-Type: multipart/mixed; boundary="------------040501040608000109000506" X-Spam-Score: -2.6 (--) X-Debbugs-Envelope-To: 8419 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.6 (--) This is a multi-part message in MIME format. --------------040501040608000109000506 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit On 25/07/11 12:42, Jim Meyering wrote: > Odd.Harry.Mannsverk@benteler-alu.com wrote: >> I have tried to use the command cp combining the -a and the -u options. >> I had to stop the copying process midways and restarted it again, and to my >> suprice the diskusage at the destination was 10 -20 % larger than the >> diskusage at the source and my disks ran full even though the destination >> disks was the same size as the source disks. > ... > > Thank you for a fine bug report. > That is indeed a bug, and it affects the latest release, coreutils-8.12. > I confirmed that it afflicts fileutils-3.16 too, so this bug has probably > been present since the initial implementation. > > Here's the fix I expect to push: That looks good. Should we fold the attached patch in too? It refactors out a create_hard_link() function, so we wouldn't be adding any new labels. cheers, Pádraig. --------------040501040608000109000506 Content-Type: text/x-patch; name="create_hl.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="create_hl.diff" diff --git a/src/copy.c b/src/copy.c index c17b942..95d05f0 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1488,6 +1488,37 @@ restore_default_fscreatecon_or_die (void) _("failed to restore the default file creation context")); } +static bool +create_hard_link (char const *src_name, char const *dst_name, + bool replace, bool verbose) +{ + /* We want to guarantee that symlinks are not followed. */ + bool link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0); + + /* If the link failed because of an existing destination, + remove that file and then call link again. */ + if (link_failed && errno == EEXIST && replace) + { + if (unlink (dst_name) != 0) + { + error (0, errno, _("cannot remove %s"), quote (dst_name)); + return false; + } + if (verbose) + printf (_("removed %s\n"), quote (dst_name)); + link_failed = (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0) != 0); + } + + if (link_failed) + { + error (0, errno, _("cannot create hard link %s to %s"), + quote_n (0, dst_name), quote_n (1, src_name)); + return false; + } + + return true; +} + /* Copy the file SRC_NAME to the file DST_NAME. The files may be of any type. NEW_DST should be true if the file DST_NAME cannot exist because its parent directory was just created; NEW_DST should @@ -1948,31 +1979,8 @@ copy_internal (char const *src_name, char const *dst_name, } else { - /* We want to guarantee that symlinks are not followed. */ - bool link_failed = (linkat (AT_FDCWD, earlier_file, AT_FDCWD, - dst_name, 0) != 0); - - /* If the link failed because of an existing destination, - remove that file and then call link again. */ - if (link_failed && errno == EEXIST) - { - if (unlink (dst_name) != 0) - { - error (0, errno, _("cannot remove %s"), quote (dst_name)); - goto un_backup; - } - if (x->verbose) - printf (_("removed %s\n"), quote (dst_name)); - link_failed = (linkat (AT_FDCWD, earlier_file, AT_FDCWD, - dst_name, 0) != 0); - } - - if (link_failed) - { - error (0, errno, _("cannot create hard link %s to %s"), - quote_n (0, dst_name), quote_n (1, earlier_file)); - goto un_backup; - } + if (!create_hard_link (earlier_file, dst_name, true, x->verbose)) + goto un_backup; return true; } @@ -2274,11 +2282,8 @@ copy_internal (char const *src_name, char const *dst_name, && !(LINK_FOLLOWS_SYMLINKS && S_ISLNK (src_mode) && x->dereference == DEREF_NEVER)) { - if (linkat (AT_FDCWD, src_name, AT_FDCWD, dst_name, 0)) - { - error (0, errno, _("cannot create link %s"), quote (dst_name)); - goto un_backup; - } + if (!create_hard_link (src_name, dst_name, false, false)) + goto un_backup; } else if (S_ISREG (src_mode) || (x->copy_as_regular && !S_ISLNK (src_mode))) --------------040501040608000109000506-- From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 10:00:46 2011 Received: (at 8419) by debbugs.gnu.org; 25 Jul 2011 14:00:46 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlLiA-0002O1-9h for submit@debbugs.gnu.org; Mon, 25 Jul 2011 10:00:46 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlLi8-0002Np-3w for 8419@debbugs.gnu.org; Mon, 25 Jul 2011 10:00:45 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 0789360092; Mon, 25 Jul 2011 16:00:37 +0200 (CEST) From: Jim Meyering To: =?iso-8859-1?Q?P=E1draig?= Brady Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au In-Reply-To: <4E2D712B.5030004@draigBrady.com> (=?iso-8859-1?Q?=22P=E1drai?= =?iso-8859-1?Q?g?= Brady"'s message of "Mon, 25 Jul 2011 14:35:39 +0100") References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> Date: Mon, 25 Jul 2011 16:00:37 +0200 Message-ID: <87d3gy4fzu.fsf@rho.meyering.net> Lines: 43 MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) P=E1draig Brady wrote: > On 25/07/11 12:42, Jim Meyering wrote: >> Odd.Harry.Mannsverk@benteler-alu.com wrote: >>> I have tried to use the command cp combining the -a and the -u options. >>> I had to stop the copying process midways and restarted it again, and t= o my >>> suprice the diskusage at the destination was 10 -20 % larger than the >>> diskusage at the source and my disks ran full even though the destinati= on >>> disks was the same size as the source disks. >> ... >> >> Thank you for a fine bug report. >> That is indeed a bug, and it affects the latest release, coreutils-8.12. >> I confirmed that it afflicts fileutils-3.16 too, so this bug has probably >> been present since the initial implementation. >> >> Here's the fix I expect to push: > > That looks good. > Should we fold the attached patch in too? > It refactors out a create_hard_link() function, > so we wouldn't be adding any new labels. Thanks for the review. Factoring out that function is definitely worth doing. However, I'd prefer to keep that clean-up change separate from the bug-fixing one. Would you please add a comment for the new function? Maybe something as simple as this: /* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE and VERBOSE settings. Return true upon success. Otherwise, diagnose the failure and return false. */ > diff --git a/src/copy.c b/src/copy.c > index c17b942..95d05f0 100644 > --- a/src/copy.c > +++ b/src/copy.c > @@ -1488,6 +1488,37 @@ restore_default_fscreatecon_or_die (void) > _("failed to restore the default file creation context")); > } > > +static bool > +create_hard_link (char const *src_name, char const *dst_name, > + bool replace, bool verbose) From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 10:03:21 2011 Received: (at 8419) by debbugs.gnu.org; 25 Jul 2011 14:03:21 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlLke-0002Rf-KD for submit@debbugs.gnu.org; Mon, 25 Jul 2011 10:03:20 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1QlLkc-0002RT-JD for 8419@debbugs.gnu.org; Mon, 25 Jul 2011 10:03:19 -0400 Received: (qmail 49295 invoked from network); 25 Jul 2011 14:03:12 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 25 Jul 2011 14:03:12 -0000 Message-ID: <4E2D76FD.5060909@draigBrady.com> Date: Mon, 25 Jul 2011 15:00:29 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> In-Reply-To: <87d3gy4fzu.fsf@rho.meyering.net> X-Enigmail-Version: 1.0.1 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.6 (--) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.6 (--) On 25/07/11 15:00, Jim Meyering wrote: > Thanks for the review. > Factoring out that function is definitely worth doing. > However, I'd prefer to keep that clean-up change separate > from the bug-fixing one. Would you please add a comment for > the new function? Maybe something as simple as this: > > /* Create a hard link DST_NAME to SRC_NAME, honoring the REPLACE and > VERBOSE settings. Return true upon success. Otherwise, diagnose > the failure and return false. */ OK I'll cleanup and apply the refactoring after yours. cheers, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 12:29:46 2011 Received: (at 8419) by debbugs.gnu.org; 25 Jul 2011 16:29:46 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlO2L-0006Um-QK for submit@debbugs.gnu.org; Mon, 25 Jul 2011 12:29:46 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1QlO2J-0006UZ-M7 for 8419@debbugs.gnu.org; Mon, 25 Jul 2011 12:29:44 -0400 Received: (qmail 75712 invoked from network); 25 Jul 2011 16:29:37 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 25 Jul 2011 16:29:37 -0000 Message-ID: <4E2D994D.706@draigBrady.com> Date: Mon, 25 Jul 2011 17:26:53 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> In-Reply-To: <4E2D76FD.5060909@draigBrady.com> X-Enigmail-Version: 1.0.1 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-Spam-Score: -2.6 (--) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.6 (--) Actually I'm wondering now whether the new code should be unconditionally replacing the dest? What if the dest is a separate newer file? I.E. I think the following amended test should pass? Also what about backups if the separate file is older? diff --git a/tests/cp/preserve-link b/tests/cp/preserve-link index d0da873..cee5ec0 100755 --- a/tests/cp/preserve-link +++ b/tests/cp/preserve-link @@ -28,13 +28,26 @@ same_inode() mkdir -p s t/s || framework_failure_ touch s/f t/s/f || framework_failure_ + +# a non existing link must be linked in the dest tree ln s/f s/link || framework_failure_ +# an existing link must be updated +ln s/f s/linke || framework_failure_ +ln t/s/f t/s/linke || framework_failure_ + +# a updated link must not be overwritten +ln s/f s/linku || framework_failure_ +touch -d "+2 hour" t/s/linku || framework_failure_ + # This must create a hard link, t/s/link, to the existing file, t/s/f. # With cp from coreutils-8.12 and prior, it would mistakenly copy # the file rather than creating the link. +touch -d "+1 hour" s/f || framework_failure_ cp -au s t || fail=1 same_inode t/s/f t/s/link || fail=1 +same_inode t/s/f t/s/linke || fail=1 +same_inode t/s/f t/s/linku && fail=1 Exit $fail From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 14:01:35 2011 Received: (at 8419-done) by debbugs.gnu.org; 25 Jul 2011 18:01:36 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlPTD-00006L-EA for submit@debbugs.gnu.org; Mon, 25 Jul 2011 14:01:35 -0400 Received: from senmx11-mx.siemens-enterprise.com ([62.134.46.9]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlPTB-000069-Px for 8419-done@debbugs.gnu.org; Mon, 25 Jul 2011 14:01:34 -0400 Received: from MCHP063A.global-ad.net (unknown [172.29.37.61]) by senmx11-mx.siemens-enterprise.com (Server) with ESMTP id C5EA51EB843C; Mon, 25 Jul 2011 20:01:27 +0200 (CEST) Received: from MCHP058A.global-ad.net ([172.29.37.57]) by MCHP063A.global-ad.net ([172.29.37.61]) with mapi; Mon, 25 Jul 2011 20:01:27 +0200 From: "Voelker, Bernhard" To: Jim Meyering Date: Mon, 25 Jul 2011 20:01:29 +0200 Subject: RE: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au Thread-Topic: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au Thread-Index: AcxKzapNp003SYfWT+OGaqbo8OIuQQAHh/Ug Message-ID: <7856072A9D04C24B82DFE2B1112FE38A08FE32AF01@MCHP058A.global-ad.net> References: <874o2a60yn.fsf@rho.meyering.net> <7856072A9D04C24B82DFE2B1112FE38A08FE2D4179@MCHP058A.global-ad.net> <87ipqq4hua.fsf@rho.meyering.net> In-Reply-To: <87ipqq4hua.fsf@rho.meyering.net> Accept-Language: de-DE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: de-DE, en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Spam-Score: -3.4 (---) X-Debbugs-Envelope-To: 8419-done Cc: "8419-done@debbugs.gnu.org" <8419-done@debbugs.gnu.org>, "Hans.Kristian.Nordengen@ergo.no" , "Odd.Harry.Mannsverk@benteler-alu.com" X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.4 (---) Jim Meyering wrote: >Voelker, Bernhard wrote: >> Jim Meyering wrote: >> + create_hard_link:; >> >> Is there a specific reason for the additional empty statement? >> >> Also seen in: >> find . -name '*.c' | xargs grep ':;' >> ./src/head.c: free_mem:; >> ./src/kill.c: no_more_options:; >> ./src/od.c:cleanup:; >> ./src/paste.c: done:; >> ./src/printf.c: no_more_flag_characters:; > > Yes. > It won't compile without that, since the thing right after > it is a declaration, and not a statement. ah, yes, there's a bool following - it's not my best day. But it's not necessary in the other cases. I've sent a patch for this in a separate mail to coreutils@gnu.org. Have a nice day, Berny From debbugs-submit-bounces@debbugs.gnu.org Mon Jul 25 20:36:26 2011 Received: (at 8419) by debbugs.gnu.org; 26 Jul 2011 00:36:27 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QlVdK-0001Q0-7Y for submit@debbugs.gnu.org; Mon, 25 Jul 2011 20:36:26 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1QlVdI-0001Po-8o for 8419@debbugs.gnu.org; Mon, 25 Jul 2011 20:36:25 -0400 Received: (qmail 38202 invoked from network); 26 Jul 2011 00:36:17 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 26 Jul 2011 00:36:17 -0000 Message-ID: <4E2E0B5C.5010606@draigBrady.com> Date: Tue, 26 Jul 2011 01:33:32 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> In-Reply-To: <4E2D994D.706@draigBrady.com> X-Enigmail-Version: 1.0.1 Content-Type: multipart/mixed; boundary="------------010207080306090109040805" X-Spam-Score: -2.6 (--) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.6 (--) This is a multi-part message in MIME format. --------------010207080306090109040805 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit On 25/07/11 17:26, Pádraig Brady wrote: > Actually I'm wondering now whether the new code > should be unconditionally replacing the dest? > What if the dest is a separate newer file? > I.E. I think the following amended test should pass? > Also what about backups if the separate file is older? Well backups take a different path, as do older or non existing destination paths. So how about this change to just remove the new create_hard_link: call and beef up the test? cheers, Pádraig. --------------010207080306090109040805 Content-Type: text/x-patch; name="cp-update-link-new.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="cp-update-link-new.diff" diff --git a/src/copy.c b/src/copy.c index df8b1db..d6a0d1a 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1633,11 +1633,11 @@ copy_internal (char const *src_name, char const *dst_name, this src/dest pair, in case this source file is hard-linked to another one. In that case, we'll use the mapping information to link the corresponding - destination names. */ - earlier_file = remember_copied (dst_name, src_sb.st_ino, - src_sb.st_dev); - if (earlier_file) - goto create_hard_link; + destination names. Note we don't hard link DST_NAME + here, because it may be a separate file with newer + or same timestamp. If it's older than SRC_NAME, + then this path is not taken. */ + remember_copied (dst_name, src_sb.st_ino, src_sb.st_dev); return true; } @@ -1959,7 +1959,6 @@ copy_internal (char const *src_name, char const *dst_name, } else { - create_hard_link:; /* We want to guarantee that symlinks are not followed. */ bool link_failed = (linkat (AT_FDCWD, earlier_file, AT_FDCWD, dst_name, 0) != 0); diff --git a/tests/cp/preserve-link b/tests/cp/preserve-link index d0da873..6beae02 100755 --- a/tests/cp/preserve-link +++ b/tests/cp/preserve-link @@ -28,13 +28,31 @@ same_inode() mkdir -p s t/s || framework_failure_ touch s/f t/s/f || framework_failure_ + +# a non existing link must be linked in the dest tree ln s/f s/link || framework_failure_ +# an existing link must be updated +ln s/f s/linke || framework_failure_ +ln t/s/f t/s/linke || framework_failure_ + +# an updated older file must be overwritten +ln s/f s/fileo || framework_failure_ +touch -d "-1 hour" t/s/fileo || framework_failure_ + +# an updated non linked file must not be overwritten +ln s/f s/fileu || framework_failure_ +touch -d "+1 hour" t/s/fileu || framework_failure_ + # This must create a hard link, t/s/link, to the existing file, t/s/f. # With cp from coreutils-8.12 and prior, it would mistakenly copy # the file rather than creating the link. +touch -d "+1 hour" t/s/f || framework_failure_ cp -au s t || fail=1 same_inode t/s/f t/s/link || fail=1 +same_inode t/s/f t/s/linke || fail=1 +same_inode t/s/f t/s/fileo || fail=1 +same_inode t/s/f t/s/fileu && fail=1 Exit $fail --------------010207080306090109040805-- From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 26 05:36:56 2011 Received: (at 8419) by debbugs.gnu.org; 26 Jul 2011 09:36:57 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qle4O-000579-5f for submit@debbugs.gnu.org; Tue, 26 Jul 2011 05:36:56 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qle4L-00056v-Qg for 8419@debbugs.gnu.org; Tue, 26 Jul 2011 05:36:55 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 25CBA601CD; Tue, 26 Jul 2011 11:36:48 +0200 (CEST) From: Jim Meyering To: =?iso-8859-1?Q?P=E1draig?= Brady Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au In-Reply-To: <4E2E0B5C.5010606@draigBrady.com> (=?iso-8859-1?Q?=22P=E1drai?= =?iso-8859-1?Q?g?= Brady"'s message of "Tue, 26 Jul 2011 01:33:32 +0100") References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> <4E2E0B5C.5010606@draigBrady.com> Date: Tue, 26 Jul 2011 11:36:48 +0200 Message-ID: <87zkk11iz3.fsf@rho.meyering.net> Lines: 106 MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) P=E1draig Brady wrote: > On 25/07/11 17:26, P=E1draig Brady wrote: >> Actually I'm wondering now whether the new code >> should be unconditionally replacing the dest? >> What if the dest is a separate newer file? >> I.E. I think the following amended test should pass? >> Also what about backups if the separate file is older? > > Well backups take a different path, as do > older or non existing destination paths. > So how about this change to just remove > the new create_hard_link: call and beef up the test? Hi P=E1draig, The link-creation code there cannot be removed. Consider this scenario: src/{a,b} # a and b are linked dst/src/a If cp -au encounters src/a first, then the new "remember_copied" call records the required info so when it encounters src/b it can make the desired link from the preexisting dst/src/a to dst/src/b. In that case, the link-creation code is indeed unnecessary. However, what if it encounters src/b first? In that case, it must copy src/b to dst/src/b (new inode!) Then, when it encounters src/a, it must *remove* the preexisting dst/src/a and replace it with a hard link to dst/src/b. As an alternative, cp could conceivably remove the just-copied dst/src/b, replacing it with a hard-link to dst/src/a. The trouble I have with all of this is that we can see two different outcomes, depending on the order in which "a" and "b" (in the src directory) are processed by cp. In one case, the preexisting and up-to-date dst/src/a is not modified. In the other, it is removed, and replaced by a hard-link to potentially-differing-content "dst/src/b". Test comments below. > diff --git a/src/copy.c b/src/copy.c > index df8b1db..d6a0d1a 100644 > --- a/src/copy.c > +++ b/src/copy.c > @@ -1633,11 +1633,11 @@ copy_internal (char const *src_name, char const *= dst_name, > this src/dest pair, in case this source file is > hard-linked to another one. In that case, we'll use > the mapping information to link the corresponding > - destination names. */ > - earlier_file =3D remember_copied (dst_name, src_sb.st_= ino, > - src_sb.st_dev); > - if (earlier_file) > - goto create_hard_link; > + destination names. Note we don't hard link DST_NAME > + here, because it may be a separate file with newer > + or same timestamp. If it's older than SRC_NAME, > + then this path is not taken. */ > + remember_copied (dst_name, src_sb.st_ino, src_sb.st_de= v); ... > diff --git a/tests/cp/preserve-link b/tests/cp/preserve-link > index d0da873..6beae02 100755 > --- a/tests/cp/preserve-link > +++ b/tests/cp/preserve-link > @@ -28,13 +28,31 @@ same_inode() > > mkdir -p s t/s || framework_failure_ > touch s/f t/s/f || framework_failure_ > + > +# a non existing link must be linked in the dest tree > ln s/f s/link || framework_failure_ > > +# an existing link must be updated > +ln s/f s/linke || framework_failure_ > +ln t/s/f t/s/linke || framework_failure_ > + > +# an updated older file must be overwritten > +ln s/f s/fileo || framework_failure_ > +touch -d "-1 hour" t/s/fileo || framework_failure_ Since this "fileo" is hard-linked to f (and f to "linke"), changing its mtime with touch changes those of the other files, too. > +# an updated non linked file must not be overwritten > +ln s/f s/fileu || framework_failure_ > +touch -d "+1 hour" t/s/fileu || framework_failure_ Doesn't this undo the effects of the preceding touch? You might want to use a separate "cp" command, with separate inputs -- or even a separate test script, moving "same_inode" into a init.cfg. > # This must create a hard link, t/s/link, to the existing file, t/s/f. > # With cp from coreutils-8.12 and prior, it would mistakenly copy > # the file rather than creating the link. > +touch -d "+1 hour" t/s/f || framework_failure_ > cp -au s t || fail=3D1 > > same_inode t/s/f t/s/link || fail=3D1 > +same_inode t/s/f t/s/linke || fail=3D1 > +same_inode t/s/f t/s/fileo || fail=3D1 > +same_inode t/s/f t/s/fileu && fail=3D1 > > Exit $fail From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 26 05:50:36 2011 Received: (at 8419) by debbugs.gnu.org; 26 Jul 2011 09:50:36 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QleHb-0005UC-Mh for submit@debbugs.gnu.org; Tue, 26 Jul 2011 05:50:36 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QleHZ-0005Ty-KV for 8419@debbugs.gnu.org; Tue, 26 Jul 2011 05:50:34 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id CDD2960211; Tue, 26 Jul 2011 11:50:27 +0200 (CEST) From: Jim Meyering To: =?iso-8859-1?Q?P=E1draig?= Brady Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au In-Reply-To: <4E2E0B5C.5010606@draigBrady.com> (=?iso-8859-1?Q?=22P=E1drai?= =?iso-8859-1?Q?g?= Brady"'s message of "Tue, 26 Jul 2011 01:33:32 +0100") References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> <4E2E0B5C.5010606@draigBrady.com> Date: Tue, 26 Jul 2011 11:50:27 +0200 Message-ID: <87tya91icc.fsf@rho.meyering.net> Lines: 62 MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) P=E1draig Brady wrote: > On 25/07/11 17:26, P=E1draig Brady wrote: >> Actually I'm wondering now whether the new code >> should be unconditionally replacing the dest? >> What if the dest is a separate newer file? >> I.E. I think the following amended test should pass? >> Also what about backups if the separate file is older? > > Well backups take a different path, as do > older or non existing destination paths. > So how about this change to just remove > the new create_hard_link: call and beef up the test? > > cheers, > P=E1draig. > > diff --git a/src/copy.c b/src/copy.c > index df8b1db..d6a0d1a 100644 > --- a/src/copy.c > +++ b/src/copy.c > @@ -1633,11 +1633,11 @@ copy_internal (char const *src_name, char const *= dst_name, > this src/dest pair, in case this source file is > hard-linked to another one. In that case, we'll use > the mapping information to link the corresponding > - destination names. */ > - earlier_file =3D remember_copied (dst_name, src_sb.st_= ino, > - src_sb.st_dev); > - if (earlier_file) > - goto create_hard_link; > + destination names. Note we don't hard link DST_NAME > + here, because it may be a separate file with newer > + or same timestamp. If it's older than SRC_NAME, > + then this path is not taken. */ > + remember_copied (dst_name, src_sb.st_ino, src_sb.st_de= v); BTW, I made exactly that mistake for the first iteration of the patch I committed yesterday. Obviously (to me, now), I should have written more in that commit to justify the link-creating code. But it's only this morning that I realized the non-determinism and understood well enough to write coherently about it. For the record, if I apply that change and run the existing test, it fails like this (using ext4 and Fedora 15): $ make check -C tests TESTS=3Dcp/preserve-link VERBOSE=3Dyes ... + mkdir -p s t/s + touch s/f t/s/f + ln s/f s/link + cp -au s t + same_inode t/s/f t/s/link + local u v ++ stat --format %i t/s/f + u=3D1573873 ++ stat --format %i t/s/link + v=3D1573874 + test 1573873 =3D 1573874 + fail=3D1 + Exit 1 ... =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 1 of 1 test failed From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 26 06:07:18 2011 Received: (at 8419) by debbugs.gnu.org; 26 Jul 2011 10:07:19 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QleXm-0005vU-9T for submit@debbugs.gnu.org; Tue, 26 Jul 2011 06:07:18 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QleXk-0005vE-DY for 8419@debbugs.gnu.org; Tue, 26 Jul 2011 06:07:17 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id AD8CB600D0; Tue, 26 Jul 2011 12:07:10 +0200 (CEST) From: Jim Meyering To: =?iso-8859-1?Q?P=E1draig?= Brady Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au In-Reply-To: <87zkk11iz3.fsf@rho.meyering.net> (Jim Meyering's message of "Tue, 26 Jul 2011 11:36:48 +0200") References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> <4E2E0B5C.5010606@draigBrady.com> <87zkk11iz3.fsf@rho.meyering.net> Date: Tue, 26 Jul 2011 12:07:10 +0200 Message-ID: <87oc0h1hkh.fsf@rho.meyering.net> Lines: 44 MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) Jim Meyering wrote: > P=E1draig Brady wrote: >> On 25/07/11 17:26, P=E1draig Brady wrote: >>> Actually I'm wondering now whether the new code >>> should be unconditionally replacing the dest? >>> What if the dest is a separate newer file? >>> I.E. I think the following amended test should pass? >>> Also what about backups if the separate file is older? >> >> Well backups take a different path, as do >> older or non existing destination paths. >> So how about this change to just remove >> the new create_hard_link: call and beef up the test? > > Hi P=E1draig, > > The link-creation code there cannot be removed. > Consider this scenario: > > src/{a,b} # a and b are linked > dst/src/a > > If cp -au encounters src/a first, then the new "remember_copied" > call records the required info so when it encounters src/b it can > make the desired link from the preexisting dst/src/a to dst/src/b. > In that case, the link-creation code is indeed unnecessary. > > However, what if it encounters src/b first? > In that case, it must copy src/b to dst/src/b (new inode!) > Then, when it encounters src/a, it must *remove* the preexisting dst/src/a This "must" is my interpretation that --preserve=3Dlink (implied by -a and -p) has a higher priority than the --update (-u) option. > and replace it with a hard link to dst/src/b. > As an alternative, cp could conceivably remove the just-copied dst/src/b, > replacing it with a hard-link to dst/src/a. > > The trouble I have with all of this is that we can see two different > outcomes, depending on the order in which "a" and "b" (in the src > directory) are processed by cp. In one case, the preexisting and > up-to-date dst/src/a is not modified. In the other, it is removed, > and replaced by a hard-link to potentially-differing-content "dst/src/b". ... From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 26 07:42:35 2011 Received: (at 8419) by debbugs.gnu.org; 26 Jul 2011 11:42:35 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qlg1z-0000rW-BO for submit@debbugs.gnu.org; Tue, 26 Jul 2011 07:42:35 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1Qlg1w-0000rF-LQ for 8419@debbugs.gnu.org; Tue, 26 Jul 2011 07:42:33 -0400 Received: (qmail 22028 invoked from network); 26 Jul 2011 11:42:26 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 26 Jul 2011 11:42:26 -0000 Message-ID: <4E2EA77A.6040207@draigBrady.com> Date: Tue, 26 Jul 2011 12:39:38 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> <4E2E0B5C.5010606@draigBrady.com> <87zkk11iz3.fsf@rho.meyering.net> <87oc0h1hkh.fsf@rho.meyering.net> In-Reply-To: <87oc0h1hkh.fsf@rho.meyering.net> X-Enigmail-Version: 1.0.1 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.6 (--) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.6 (--) On 26/07/11 11:07, Jim Meyering wrote: > Jim Meyering wrote: >> Pádraig Brady wrote: >>> On 25/07/11 17:26, Pádraig Brady wrote: >>>> Actually I'm wondering now whether the new code >>>> should be unconditionally replacing the dest? >>>> What if the dest is a separate newer file? >>>> I.E. I think the following amended test should pass? >>>> Also what about backups if the separate file is older? >>> >>> Well backups take a different path, as do >>> older or non existing destination paths. >>> So how about this change to just remove >>> the new create_hard_link: call and beef up the test? >> >> Hi Pádraig, >> >> The link-creation code there cannot be removed. >> Consider this scenario: >> >> src/{a,b} # a and b are linked >> dst/src/a >> >> If cp -au encounters src/a first, then the new "remember_copied" >> call records the required info so when it encounters src/b it can >> make the desired link from the preexisting dst/src/a to dst/src/b. >> In that case, the link-creation code is indeed unnecessary. >> >> However, what if it encounters src/b first? >> In that case, it must copy src/b to dst/src/b (new inode!) >> Then, when it encounters src/a, it must *remove* the preexisting dst/src/a > > This "must" is my interpretation that --preserve=link (implied > by -a and -p) has a higher priority than the --update (-u) option. That's a bit surprising, thought understandable I think. If the separate file in the dest is newer it will be replaced by an older link from the source. That could legitimately happen I suppose if the original copy was with -rp, then the timestamps would be newer than those in dest. I'll update the test to enforce the replacement of newer files in dest >> and replace it with a hard link to dst/src/b. >> As an alternative, cp could conceivably remove the just-copied dst/src/b, >> replacing it with a hard-link to dst/src/a. >> >> The trouble I have with all of this is that we can see two different >> outcomes, depending on the order in which "a" and "b" (in the src >> directory) are processed by cp. In one case, the preexisting and >> up-to-date dst/src/a is not modified. In the other, it is removed, >> and replaced by a hard-link to potentially-differing-content "dst/src/b". TBH I don't understand all the implications. Note the `touch` commands in the test operate on the separate inodes in dest. Note also your original test didn't fail for me on ext4 on F15. cheers, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 26 09:23:46 2011 Received: (at 8419) by debbugs.gnu.org; 26 Jul 2011 13:23:47 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qlhbu-0003Yy-Bx for submit@debbugs.gnu.org; Tue, 26 Jul 2011 09:23:46 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qlhbr-0003Yl-Nk for 8419@debbugs.gnu.org; Tue, 26 Jul 2011 09:23:44 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 1B0D160092; Tue, 26 Jul 2011 15:23:37 +0200 (CEST) From: Jim Meyering To: =?iso-8859-1?Q?P=E1draig?= Brady Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au In-Reply-To: <4E2EA77A.6040207@draigBrady.com> (=?iso-8859-1?Q?=22P=E1drai?= =?iso-8859-1?Q?g?= Brady"'s message of "Tue, 26 Jul 2011 12:39:38 +0100") References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> <4E2E0B5C.5010606@draigBrady.com> <87zkk11iz3.fsf@rho.meyering.net> <87oc0h1hkh.fsf@rho.meyering.net> <4E2EA77A.6040207@draigBrady.com> Date: Tue, 26 Jul 2011 15:23:36 +0200 Message-ID: <87ei1d18h3.fsf@rho.meyering.net> Lines: 87 MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) P=E1draig Brady wrote: ... >>> The link-creation code there cannot be removed. >>> Consider this scenario: >>> >>> src/{a,b} # a and b are linked >>> dst/src/a >>> >>> If cp -au encounters src/a first, then the new "remember_copied" >>> call records the required info so when it encounters src/b it can >>> make the desired link from the preexisting dst/src/a to dst/src/b. >>> In that case, the link-creation code is indeed unnecessary. >>> >>> However, what if it encounters src/b first? >>> In that case, it must copy src/b to dst/src/b (new inode!) >>> Then, when it encounters src/a, it must *remove* the preexisting dst/sr= c/a >> >> This "must" is my interpretation that --preserve=3Dlink (implied >> by -a and -p) has a higher priority than the --update (-u) option. > > That's a bit surprising, thought understandable I think. > If the separate file in the dest is newer it will be replaced > by an older link from the source. That could legitimately happen > I suppose if the original copy was with -rp, then the timestamps > would be newer than those in dest. > I'll update the test to enforce the replacement of newer files in dest > >>> and replace it with a hard link to dst/src/b. >>> As an alternative, cp could conceivably remove the just-copied dst/src/= b, >>> replacing it with a hard-link to dst/src/a. >>> >>> The trouble I have with all of this is that we can see two different >>> outcomes, depending on the order in which "a" and "b" (in the src >>> directory) are processed by cp. In one case, the preexisting and >>> up-to-date dst/src/a is not modified. In the other, it is removed, >>> and replaced by a hard-link to potentially-differing-content "dst/src/b= ". > > TBH I don't understand all the implications. > Note the `touch` commands in the test operate on the separate inodes in d= est. Oh! Sorry I missed that. > Note also your original test didn't fail for me on ext4 on F15. When I apply the change below, recompile "cp" and run make check -C tests TESTS=3Dcp/preserve-link VERBOSE=3Dyes with the test tweaked to run cp via strace, strace -e stat,lstat -o /tmp/cp-strace cp -au s t || fail=3D1 I get this, which shows it's processing s/link before s/f. stat("t", {st_mode=3DS_IFDIR|0700, st_size=3D4096, ...}) =3D 0 lstat("s", {st_mode=3DS_IFDIR|0700, st_size=3D4096, ...}) =3D 0 lstat("t/s", {st_mode=3DS_IFDIR|0700, st_size=3D4096, ...}) =3D 0 lstat("s/link", {st_mode=3DS_IFREG|0600, st_size=3D0, ...}) =3D 0 stat("t/s/link", 0x7fffa85ca0a0) =3D -1 ENOENT (No such file or di= rectory) lstat("s/f", {st_mode=3DS_IFREG|0600, st_size=3D0, ...}) =3D 0 stat("t/s/f", {st_mode=3DS_IFREG|0600, st_size=3D0, ...}) =3D 0 I suspect you'll see that it's processing those two files in the reverse order on your system. In case it's kernel-related, I'm using this: 2.6.38.8-32.fc15.x86_64 and the disk is an SSD. diff --git a/src/copy.c b/src/copy.c index aaf7e79..f4b6587 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1636,10 +1636,7 @@ copy_internal (char const *src_name, char const *dst= _name, hard-linked to another one. In that case, we'll use the mapping information to link the corresponding destination names. */ - earlier_file =3D remember_copied (dst_name, src_sb.st_in= o, - src_sb.st_dev); - if (earlier_file) - goto create_hard_link; + remember_copied (dst_name, src_sb.st_ino, src_sb.st_dev); return true; } @@ -1961,7 +1958,6 @@ copy_internal (char const *src_name, char const *dst_= name, } else { - create_hard_link:; /* We want to guarantee that symlinks are not followed. */ bool link_failed =3D (linkat (AT_FDCWD, earlier_file, AT_FDCWD, dst_name, 0) !=3D 0); From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 26 09:46:25 2011 Received: (at 8419) by debbugs.gnu.org; 26 Jul 2011 13:46:26 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qlhxp-00051S-7l for submit@debbugs.gnu.org; Tue, 26 Jul 2011 09:46:25 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1Qlhxn-00051H-Ud for 8419@debbugs.gnu.org; Tue, 26 Jul 2011 09:46:24 -0400 Received: (qmail 42932 invoked from network); 26 Jul 2011 13:46:17 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 26 Jul 2011 13:46:17 -0000 Message-ID: <4E2EC482.5020901@draigBrady.com> Date: Tue, 26 Jul 2011 14:43:30 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> <4E2E0B5C.5010606@draigBrady.com> <87zkk11iz3.fsf@rho.meyering.net> <87oc0h1hkh.fsf@rho.meyering.net> <4E2EA77A.6040207@draigBrady.com> <87ei1d18h3.fsf@rho.meyering.net> In-Reply-To: <87ei1d18h3.fsf@rho.meyering.net> X-Enigmail-Version: 1.0.1 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.6 (--) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.6 (--) On 26/07/11 14:23, Jim Meyering wrote: > Pádraig Brady wrote: > ... >>>> The link-creation code there cannot be removed. >>>> Consider this scenario: >>>> >>>> src/{a,b} # a and b are linked >>>> dst/src/a >>>> >>>> If cp -au encounters src/a first, then the new "remember_copied" >>>> call records the required info so when it encounters src/b it can >>>> make the desired link from the preexisting dst/src/a to dst/src/b. >>>> In that case, the link-creation code is indeed unnecessary. >>>> >>>> However, what if it encounters src/b first? >>>> In that case, it must copy src/b to dst/src/b (new inode!) >>>> Then, when it encounters src/a, it must *remove* the preexisting dst/src/a >>> >>> This "must" is my interpretation that --preserve=link (implied >>> by -a and -p) has a higher priority than the --update (-u) option. >> >> That's a bit surprising, thought understandable I think. >> If the separate file in the dest is newer it will be replaced >> by an older link from the source. That could legitimately happen >> I suppose if the original copy was with -rp, then the timestamps >> would be newer than those in dest. >> I'll update the test to enforce the replacement of newer files in dest >> >>>> and replace it with a hard link to dst/src/b. >>>> As an alternative, cp could conceivably remove the just-copied dst/src/b, >>>> replacing it with a hard-link to dst/src/a. >>>> >>>> The trouble I have with all of this is that we can see two different >>>> outcomes, depending on the order in which "a" and "b" (in the src >>>> directory) are processed by cp. In one case, the preexisting and >>>> up-to-date dst/src/a is not modified. In the other, it is removed, >>>> and replaced by a hard-link to potentially-differing-content "dst/src/b". >> >> TBH I don't understand all the implications. >> Note the `touch` commands in the test operate on the separate inodes in dest. > > Oh! Sorry I missed that. > >> Note also your original test didn't fail for me on ext4 on F15. > > When I apply the change below, recompile "cp" and run > make check -C tests TESTS=cp/preserve-link VERBOSE=yes > with the test tweaked to run cp via strace, > strace -e stat,lstat -o /tmp/cp-strace cp -au s t || fail=1 > I get this, which shows it's processing s/link before s/f. > > stat("t", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0 > lstat("s", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0 > lstat("t/s", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0 > lstat("s/link", {st_mode=S_IFREG|0600, st_size=0, ...}) = 0 > stat("t/s/link", 0x7fffa85ca0a0) = -1 ENOENT (No such file or directory) > lstat("s/f", {st_mode=S_IFREG|0600, st_size=0, ...}) = 0 > stat("t/s/f", {st_mode=S_IFREG|0600, st_size=0, ...}) = 0 confirmed. stat("t", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 lstat("s", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 lstat("t/s", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 lstat("s/f", {st_mode=S_IFREG|0664, st_size=0, ...}) = 0 stat("t/s/f", {st_mode=S_IFREG|0664, st_size=0, ...}) = 0 lstat("s/link", {st_mode=S_IFREG|0664, st_size=0, ...}) = 0 stat("t/s/link", 0x7fff731e22c0) = -1 ENOENT (No such file or directory) stat("s", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 stat("s", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 stat("t/s", {st_mode=S_IFDIR|0775, st_size=4096, ...}) = 0 cheers, Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Wed Jul 27 05:38:09 2011 Received: (at 8419) by debbugs.gnu.org; 27 Jul 2011 09:38:09 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qm0Z7-00011e-H4 for submit@debbugs.gnu.org; Wed, 27 Jul 2011 05:38:09 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1Qm0Z5-00011W-Dc for 8419@debbugs.gnu.org; Wed, 27 Jul 2011 05:38:08 -0400 Received: (qmail 96768 invoked from network); 27 Jul 2011 09:38:05 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 27 Jul 2011 09:38:05 -0000 Message-ID: <4E2FDBD2.3030303@draigBrady.com> Date: Wed, 27 Jul 2011 10:35:14 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> <4E2E0B5C.5010606@draigBrady.com> <87zkk11iz3.fsf@rho.meyering.net> <87oc0h1hkh.fsf@rho.meyering.net> <4E2EA77A.6040207@draigBrady.com> <87ei1d18h3.fsf@rho.meyering.net> In-Reply-To: <87ei1d18h3.fsf@rho.meyering.net> X-Enigmail-Version: 1.0.1 Content-Type: multipart/mixed; boundary="------------090203070902090808060400" X-Spam-Score: -2.6 (--) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.6 (--) This is a multi-part message in MIME format. --------------090203070902090808060400 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit On 26/07/11 14:23, Jim Meyering wrote: > Pádraig Brady wrote: > ... > >> Note also your original test didn't fail for me on ext4 on F15. > > I suspect you'll see that it's processing those two files in the reverse > order on your system. In case it's kernel-related, I'm using this: > 2.6.38.8-32.fc15.x86_64 > and the disk is an SSD. My guess here is there is inode sorting going on, which is unstable and returning matching inodes in a non deterministic order? Anyway I've updated the test (attached) to try both ways. cheers, Pádraig. --------------090203070902090808060400 Content-Type: text/x-patch; name="cp-preserve-link.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="cp-preserve-link.diff" >From 2aea1828a1aab158f68cccf3eac408203889021e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Wed, 27 Jul 2011 09:32:39 +0100 Subject: [PATCH] tests: cp/preserve-link: test all relevant paths * tests/cp/preserve-link: Add test cases for when a missing link in the destination tree is encountered first and second. Also add cases for old and new separate files in the destination tree, both to make the clobbering behavior explicit, and to test any changes in this area in future. --- tests/cp/preserve-link | 68 ++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 60 insertions(+), 8 deletions(-) diff --git a/tests/cp/preserve-link b/tests/cp/preserve-link index d0da873..e3c31f9 100755 --- a/tests/cp/preserve-link +++ b/tests/cp/preserve-link @@ -26,15 +26,67 @@ same_inode() v=$(stat --format %i "$2") && test "$u" = "$v" } -mkdir -p s t/s || framework_failure_ -touch s/f t/s/f || framework_failure_ -ln s/f s/link || framework_failure_ +create_source_tree() +{ + rm -Rf s + mkdir s || framework_failure_ + + # a missing link in dest will be created + touch s/f || framework_failure_ + ln s/f s/linkm || framework_failure_ + + # an existing link in dest will be maintained + ln s/f s/linke || framework_failure_ + + # a separate older file in dest will be overwritten + ln s/f s/fileo || framework_failure_ + + # a separate newer file in dest will be overwritten! + ln s/f s/fileu || framework_failure_ +} + +create_target_tree() +{ + f=$1 # which of f or linkm to create in t/ + + rm -Rf t + mkdir -p t/s/ || framework_failure_ + + # a missing link in dest must be created + touch t/s/$f || framework_failure_ + + # an existing link must be maintained + ln t/s/$f t/s/linke || framework_failure_ + + # a separate older file in dest will be overwritten + touch -d '-1 hour' t/s/fileo || framework_failure_ + + # a separate newer file in dest will be overwritten! + touch -d '+1 hour' t/s/fileu || framework_failure_ +} + + +# Note we repeat this, creating either one of +# two hard linked files from source in the dest, so as to +# test both paths in `cp` for creating the hard links. +# The path taken by cp is dependent on which cp encounters +# first in the source, which is non deterministic currently +# (I'm guessing that results are sorted by inode and +# beauses they're the same here, and due to the sort +# being unstable, either can be processed first). +create_source_tree + +for f in f linkm; do + create_target_tree $f -# This must create a hard link, t/s/link, to the existing file, t/s/f. -# With cp from coreutils-8.12 and prior, it would mistakenly copy -# the file rather than creating the link. -cp -au s t || fail=1 + # Copy all the hard links across. With cp from coreutils-8.12 + # and prior, it would sometimes mistakenly copy rather than link. + cp -au s t || fail=1 -same_inode t/s/f t/s/link || fail=1 + same_inode t/s/f t/s/linkm || fail=1 + same_inode t/s/f t/s/linke || fail=1 + same_inode t/s/f t/s/fileo || fail=1 + same_inode t/s/f t/s/fileu || fail=1 +done Exit $fail -- 1.7.5.2 --------------090203070902090808060400-- From debbugs-submit-bounces@debbugs.gnu.org Wed Jul 27 08:10:35 2011 Received: (at 8419) by debbugs.gnu.org; 27 Jul 2011 12:10:35 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qm2wc-0005TF-If for submit@debbugs.gnu.org; Wed, 27 Jul 2011 08:10:35 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qm2wa-0005T7-CY for 8419@debbugs.gnu.org; Wed, 27 Jul 2011 08:10:33 -0400 Received: from rho.meyering.net (localhost.localdomain [127.0.0.1]) by rho.meyering.net (Acme Bit-Twister) with ESMTP id 990506005B; Wed, 27 Jul 2011 14:10:31 +0200 (CEST) From: Jim Meyering To: =?iso-8859-1?Q?P=E1draig?= Brady Subject: Re: bug#8419: cp -au : New hard links in source becomes new files at destination when using cp -au In-Reply-To: <4E2FDBD2.3030303@draigBrady.com> (=?iso-8859-1?Q?=22P=E1drai?= =?iso-8859-1?Q?g?= Brady"'s message of "Wed, 27 Jul 2011 10:35:14 +0100") References: <874o2a60yn.fsf@rho.meyering.net> <4E2D712B.5030004@draigBrady.com> <87d3gy4fzu.fsf@rho.meyering.net> <4E2D76FD.5060909@draigBrady.com> <4E2D994D.706@draigBrady.com> <4E2E0B5C.5010606@draigBrady.com> <87zkk11iz3.fsf@rho.meyering.net> <87oc0h1hkh.fsf@rho.meyering.net> <4E2EA77A.6040207@draigBrady.com> <87ei1d18h3.fsf@rho.meyering.net> <4E2FDBD2.3030303@draigBrady.com> Date: Wed, 27 Jul 2011 14:10:31 +0200 Message-ID: <87mxfzx6tk.fsf@rho.meyering.net> Lines: 28 MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -6.1 (------) X-Debbugs-Envelope-To: 8419 Cc: 8419@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -6.1 (------) P=E1draig Brady wrote: > On 26/07/11 14:23, Jim Meyering wrote: >> P=E1draig Brady wrote: >> ... >> >>> Note also your original test didn't fail for me on ext4 on F15. >> > >> I suspect you'll see that it's processing those two files in the reverse >> order on your system. In case it's kernel-related, I'm using this: >> 2.6.38.8-32.fc15.x86_64 >> and the disk is an SSD. > > My guess here is there is inode sorting going on, > which is unstable and returning matching inodes in > a non deterministic order? > > Anyway I've updated the test (attached) to try both ways. ... > Subject: [PATCH] tests: cp/preserve-link: test all relevant paths > > * tests/cp/preserve-link: Add test cases for when a missing > link in the destination tree is encountered first and second. > Also add cases for old and new separate files in the destination > tree, both to make the clobbering behavior explicit, and to > test any changes in this area in future. Thanks! From unknown Mon Jun 23 23:54:25 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Thu, 25 Aug 2011 11:24:03 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator