Skip to content

uucore: fall back to '~' when the backup suffix is empty#13274

Open
Adel-Ayoub wants to merge 1 commit into
uutils:mainfrom
Adel-Ayoub:fix/backup-empty-suffix
Open

uucore: fall back to '~' when the backup suffix is empty#13274
Adel-Ayoub wants to merge 1 commit into
uutils:mainfrom
Adel-Ayoub:fix/backup-empty-suffix

Conversation

@Adel-Ayoub

Copy link
Copy Markdown

Problem

GNU coreutils treats an empty backup suffix as "unset" and falls back to the
default ~. Our determine_backup_suffix() returned the empty string instead,
so with --backup and an empty --suffix= (or SIMPLE_BACKUP_SUFFIX=) the
backup path became identical to the destination. As a result cp, install
and mv silently created no backup, and ln failed outright.

Reported in #13168.

Before / after (vs GNU coreutils 9.11, LANG=C)

$ echo new > a; echo old > b
$ cp --backup a b --suffix=
command GNU 9.11 uutils before uutils after
cp --backup a b --suffix= b~ (old) no backup b~ (old)
install --backup a b --suffix= b~ (old) no backup b~ (old)
mv --backup --suffix= a b b~ (old) no backup b~ (old)
ln --backup --suffix= a b b~ + link failed to create hard link 'a' => 'b': Already exists (exit 1) b~ + link

The precedence --suffix > SIMPLE_BACKUP_SUFFIX > ~ was already correct;
only the empty-suffix fallback was missing.

Fix

determine_backup_suffix() already maps a suffix containing / to the default
~. Extend that check to also cover the empty suffix, at the single shared
location used by cp, install, ln and mv.

Tests

  • uucore: test_suffix_empty_defaults_to_tilde
  • cp: test_cp_arg_suffix_empty_defaults_to_tilde
  • install: test_install_backup_empty_suffix_defaults_to_tilde
  • ln: test_ln_backup_empty_suffix_defaults_to_tilde (covers the case that previously failed outright)

Note: the issue also mentions --backup=nil; that part is not a bug (nil is
the alias for existing, and both GNU and uutils already create the backup),
so this PR only addresses the empty-suffix case.

Fixes #13168

GNU coreutils treats an empty backup suffix as unset and falls back to
the default '~'. determine_backup_suffix returned the empty string
instead, so --backup combined with an empty --suffix= (or
SIMPLE_BACKUP_SUFFIX=) produced a backup path equal to the destination:
cp, install and mv silently skipped the backup, and ln failed outright
with "Already exists".

Fall back to the default suffix when the resolved suffix is empty, the
same way a suffix containing a slash is already handled. This is the
single shared spot used by cp, install, ln and mv.

Fixes uutils#13168
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(install): --backup=nil and --backup --suffix= silently skip backup creation

1 participant