mv: fix invalid numbered backup path by hamflx · Pull Request #6119 · uutils/coreutils

hamflx · 2024-03-24T09:31:08Z

the gnu mv:

hamflx@hamflx-workstation:~/coreutils/test$ find . -delete && mkdir C D E && touch C/c D/d E/e
hamflx@hamflx-workstation:~/coreutils/test$ mv -T --backup=numbered C E/
hamflx@hamflx-workstation:~/coreutils/test$ tree -a
.
├── D
│   └── d
├── E
│   └── c
└── E.~1~
    └── e

3 directories, 3 files

the uu_mv:

hamflx@hamflx-workstation:~/coreutils/test$ find . -delete && mkdir C D E && touch C/c D/d E/e
hamflx@hamflx-workstation:~/coreutils/test$ ../target/debug/mv -T --backup=numbered C E/
hamflx@hamflx-workstation:~/coreutils/test$ tree -a
.
├── D
│   └── d
└── E
    └── c

2 directories, 2 files

src/uucore/src/lib/features/backup_control.rs

sylvestre · 2024-03-31T14:16:02Z

Could you please add an highlevel integration test in https://github.com/uutils/coreutils/blob/main/tests/by-util/test_mv.rs ? thanks

hamflx · 2024-04-03T01:54:09Z

@sylvestre I have added integration test.

github-actions · 2024-04-03T05:48:33Z

GNU testsuite comparison:

Skip an intermittent issue tests/rm/rm1 (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

hamflx · 2024-04-11T07:06:56Z

@sylvestre @tertsdiepraam Is there anything else I need to do if I want this PR to be merged?

sylvestre · 2024-04-11T07:18:45Z

src/uucore/src/lib/features/backup_control.rs

-    let test_path = Path::new(test_path_str);
+    let file_name = path.file_name().unwrap_or_default();
+    let mut numbered_file_name = file_name.to_os_string();
+    numbered_file_name.push(".~1~");


can it be incremental ? like .2, .3, etc

I'm trying to keep the logic the same as before. I think this just checks if the numbered backup exists. Only checking for '.1' is enough. If it exists, then we're still using the numbered backup, else we're using simple backup.

ping ?

sorry, English isn't my native language. What does "ping" mean?

sorry, if you are still working on it :)

Is there anything else I need to do next?

I've noticed that there are some compilation errors being reported currently. If you intend to merge this PR, I can go and handle them.

please don't share screenshot, it is terrible for search and accessibility :)

(and it is fine, it is an intermittent issue)

@hamflx: This is unrelated to your PR.

github-actions · 2024-11-26T08:12:30Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

sylvestre · 2024-11-26T08:48:59Z

src/uucore/src/lib/features/backup_control.rs

Suggested change

match path.file_name() {

Some(file_name) => {

let mut file_name = file_name.to_os_string();

file_name.push(suffix);

path.with_file_name(file_name)

}

None => path.with_file_name(suffix),

}

let mut file_name = path.file_name().unwrap_or_default().to_os_string();

file_name.push(suffix);

path.with_file_name(file_name)

github-actions · 2024-11-26T10:14:22Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

github-actions · 2024-12-02T18:52:11Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

tertsdiepraam reviewed Mar 24, 2024

View reviewed changes

src/uucore/src/lib/features/backup_control.rs Outdated Show resolved Hide resolved

hamflx force-pushed the fix/invalid-backup-numbered-path branch from f38fc1b to d4bc4cd Compare March 24, 2024 12:20

hamflx mentioned this pull request Mar 24, 2024

Make mv command fallback to copy only if the src and dst are on different device #6040

Merged

sylvestre reviewed Mar 25, 2024

View reviewed changes

src/uucore/src/lib/features/backup_control.rs Outdated Show resolved Hide resolved

sylvestre force-pushed the fix/invalid-backup-numbered-path branch from d4bc4cd to 8d5334a Compare March 30, 2024 21:51

hamflx force-pushed the fix/invalid-backup-numbered-path branch from 8d5334a to 5f2f7dc Compare April 3, 2024 01:52

sylvestre reviewed Apr 11, 2024

View reviewed changes

sylvestre force-pushed the fix/invalid-backup-numbered-path branch from 5f2f7dc to 9e4566b Compare November 25, 2024 09:26

hamflx force-pushed the fix/invalid-backup-numbered-path branch from 9e4566b to 4b7b26f Compare November 26, 2024 07:45

sylvestre reviewed Nov 26, 2024

View reviewed changes

hamflx force-pushed the fix/invalid-backup-numbered-path branch from 4b7b26f to d32823a Compare November 26, 2024 09:47

mv: fix invalid numbered backup path

6b32c30

sylvestre force-pushed the fix/invalid-backup-numbered-path branch from d32823a to 6b32c30 Compare December 2, 2024 09:22

sylvestre merged commit 843c0c2 into uutils:main Dec 3, 2024

moonfruit mentioned this pull request Jan 20, 2025

uutils-selected 0.0.29 moonfruit/homebrew-tap#200

Closed

-    match path.file_name() {
-        Some(file_name) => {
-            let mut file_name = file_name.to_os_string();
-            file_name.push(suffix);
-            path.with_file_name(file_name)
-        }
-        None => path.with_file_name(suffix),
-    }
+    let mut file_name = path.file_name().unwrap_or_default().to_os_string();
+    file_name.push(suffix);
+    path.with_file_name(file_name)

Uh oh!

Conversation

hamflx commented Mar 24, 2024

Uh oh!

Uh oh!

Uh oh!

sylvestre commented Mar 31, 2024

Uh oh!

hamflx commented Apr 3, 2024

Uh oh!

github-actions bot commented Apr 3, 2024

Uh oh!

hamflx commented Apr 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 26, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 26, 2024

Uh oh!

github-actions bot commented Dec 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments