fix(mv): prevent hang when moving directories containing FIFO files

[naoNao89]

Fixes #9656

Adds FIFO detection in copy_dir_contents_recursive() to prevent indefinite hang when moving directories containing FIFO files. Uses make_fifo() instead of fs::copy().

Includes test with timeout protection

[oech3]

Is your test really cross-filesystem? See discussion at #8605 (comment) too.

[github-actions]

GNU testsuite comparison:

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

[naoNao89]

okey
test_mv_dir_containing_fifo_cross_filesystem() moving across filesystems to /dev/shm (the scenario where the bug occurs). #[ignore] since it requires /dev/shm.

test_mv_dir_containing_fifo_same_filesystem()tests the normal case on the same filesystem.

these ensures FIFO handling works in both scenarios

[oech3]

Can we remove ignore and just skip on th system /dev/shm is unusable? #8605 is not merged yet because it has ignore.

[github-actions]

GNU testsuite comparison:

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

[github-actions]

GNU testsuite comparison:

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

[oech3]

I guess non-cross FS test was added at #7241 . So we need test_mv_dir_containing_fifo_cross_filesystem only at here.

[oech3]

Oh it has ignore...

[naoNao89]

heh, you want proper test but that require root. something like:

#[cfg(unix)]
#[ignore = "requires root for mount"]
#[test]
fn test_mv_dir_with_fifo_cross_filesystem() {
    let mut ts = TestScenario::new(util_name!());
    let at = &ts.fixtures;
    
    at.mkdir("source");
    at.mkfifo("source/pipe");
    at.mkdir("mountpoint");
    
    ts.mount_temp_fs(at.plus_as_string("mountpoint")).unwrap();
    
    ts.ucmd()
        .args(&["source", "mountpoint/dest"])
        .timeout(Duration::from_secs(2))
        .succeeds();
}

[oech3]

Merging this might delay... But yeah. And I think we can use GNU >9.9 's test to block regression.

[github-actions]

GNU testsuite comparison:

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

[oech3]

I guess we can reuse cp -a's codebase and fix fifo,symlink,special,etc... without many if (not sure)...

[naoNao89]

But that's a big refactor, reusing cp's file type handling would touch many parts of mv. You should create a new issue as enhancement: "mv: share special file handling logic with cp"

[github-actions]

GNU testsuite comparison:

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

[codspeed-hq]

CodSpeed Performance Report

Merging #9665 will not alter performance

_{Comparing naoNao89:fix/mv-fifo-hang-9656 (68a35de) with main (1fca829)}

Summary

✅ 127 untouched
⏩ 6 skipped¹

Footnotes

1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[oech3]

To simplify tests: #8605 (comment)

[github-actions]

GNU testsuite comparison:

Congrats! The gnu test tests/basenc/bounded-memory is now passing!