On 02/04/2023 13:40, Pádraig Brady wrote:
On 01/04/2023 23:40, Kristian Klausen via GNU coreutils Bug Reports wrote:
Hi
After upgrading to coreutils 9.2-2 on Arch Linux the following:
mkdir -p src dst
touch {src,dst}/bar
cp --recursive --backup src/* dst
fails with:
cp: cannot create regular file 'dst/foo/bar': File exists
Running strace on cp I noticed:
renameat2(4, "foo/bar", 4, "foo/bar~", 0) = -1 ENOENT (No such file or
directory)
In coreutils 9.1-3 the syscall succeeds:
renameat2(4, "bar", 4, "bar~", 0) = 0
I assume renameat2 is called with the wrong oldpath and newpath in 9.2
and that it should just be the basename and not the full relative path.
Cheers
Kristian Klausen
Your analysis is correct wrt the wrong paths being given to the renameat2().
This is related to https://bugs.gnu.org/55029
For completeness the correct repro is:
mkdir -p {src,dst}/foo
touch {src,dst}/foo/bar
cp --recursive --backup src/* dst
The attached two patches should address this.
The first fixes the bug in gnulib (cc'd),
while the second adds a test to coreutils.
thanks,
Pádraig
From 418aa564ebff70c1d118a5d3307a6d0b147ff7a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Mon, 3 Apr 2023 18:06:22 +0100
Subject: [PATCH] backupfile: fix bug when renaming from subdirectory
* lib/backupfile.c (backup_internal): Ensure we use the
appropriate offset if operating on a subdirectory,
i.e., on an updated sdir.
Fixes https://bugs.gnu.org/62607
---
ChangeLog | 8 ++++++++
lib/backupfile.c | 7 ++++---
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ae851ed8aa..69c19b3ebe 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2023-04-03 Pádraig Brady <p...@draigbrady.com>
+
+ backupfile: fix bug when renaming from subdirectory
+ * lib/backupfile.c (backup_internal): Ensure we use the
+ appropriate offset if operating on a subdirectory,
+ i.e., on an updated sdir.
+ Fixes https://bugs.gnu.org/62607
+
2023-04-03 Bruno Haible <br...@clisp.org>
vasnprintf-posix: Fix harmless mistake (regression 2023-03-24).
diff --git a/lib/backupfile.c b/lib/backupfile.c
index 9cca271343..5bcf924414 100644
--- a/lib/backupfile.c
+++ b/lib/backupfile.c
@@ -331,7 +331,7 @@ backupfile_internal (int dir_fd, char const *file,
return s;
DIR *dirp = NULL;
- int sdir = dir_fd;
+ int sdir = -1;
idx_t base_max = 0;
while (true)
{
@@ -370,9 +370,10 @@ backupfile_internal (int dir_fd, char const *file,
if (! rename)
break;
- idx_t offset = backup_type == simple_backups ? 0 : base_offset;
+ dir_fd = sdir < 0 ? dir_fd : sdir;
+ idx_t offset = sdir < 0 ? 0 : base_offset;
unsigned flags = backup_type == simple_backups ? 0 : RENAME_NOREPLACE;
- if (renameatu (sdir, file + offset, sdir, s + offset, flags) == 0)
+ if (renameatu (dir_fd, file + offset, dir_fd, s + offset, flags) == 0)
break;
int e = errno;
if (! (e == EEXIST && extended))
--
2.26.2
From 1a80fab339d52db7e284b4f2f41068d5d8dd7e4e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Mon, 3 Apr 2023 18:12:33 +0100
Subject: [PATCH] tests: cp: test --backup with subdirectories
* tests/cp/backup-dir.sh: Add a test to ensure
we rename appropriately when backing up through subdirs.
* NEWS: Mention the bug fix.
Addresses https://bugs.gnu.org/62607
---
NEWS | 5 +++++
tests/cp/backup-dir.sh | 8 +++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index f53adab6f..59c6da5a7 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,11 @@ GNU coreutils NEWS -*- outline -*-
more restricted systems like android or containers etc.
[bug introduced in coreutils-9.2]
+ cp --recursive --backup will again operate correctly.
+ Previousy it may have hit "File exists" errors when
+ it failed to appropriately rename files being replaced.
+ [bug introduced in coreutils-9.2]
+
date --file and dircolors will now diagnose a failure to read a file.
Previously they would have silently ignored the failure.
[This bug was present in "the beginning".]
diff --git a/tests/cp/backup-dir.sh b/tests/cp/backup-dir.sh
index 6573d58e0..5c17498cf 100755
--- a/tests/cp/backup-dir.sh
+++ b/tests/cp/backup-dir.sh
@@ -1,5 +1,5 @@
#!/bin/sh
-# Ensure that cp -b doesn't back up directories.
+# Ensure that cp -b handles directories appropriately
# Copyright (C) 2006-2023 Free Software Foundation, Inc.
@@ -29,4 +29,10 @@ cp -ab x y || fail=1
test -d y/x || fail=1
test -d y/x~ && fail=1
+# Bug 62607.
+# This would fail to backup using rename, and thus fail to replace the file
+mkdir -p {src,dst}/foo || framework_failure_
+touch {src,dst}/foo/bar || framework_failure_
+cp --recursive --backup src/* dst || fail=1
+
Exit $fail
--
2.26.2
