Skip to content

Conversation

@JRF63
Copy link
Contributor

@JRF63 JRF63 commented Jun 21, 2024

This PR introduces a new library for directory tree traversal that will be shared by the cp, mv and rm. This ensures consistent behavior across these utilities and reduces code duplication.

The result is closer to ftw/nftw than the walkdir crate. std::iter::Iterator isn't a good fit for listing the entries because of the need for conditional skipping of directories and contextual error reporting.

One of the design goals was to be able to walk through a file hierarchy in a path prefix/dir without being disrupted by modifications in prefix (no matter how long deep prefix actually is). The copying subroutine for cp and mv was modified to use mkdirat + openat while rm now uses unlinkat so that the utilities themselves will have the same resistance to tampering.

rm's specification requires the ability to walk through arbitrarily deep file hierarchies and handle unlimited path lengths. So as a consequence of using the same library, cp and mv can now do those too.

@JRF63 JRF63 marked this pull request as draft June 21, 2024 16:11
@jgarzik jgarzik added the enhancement New feature or request label Jul 11, 2024
@JRF63 JRF63 marked this pull request as ready for review August 23, 2024 19:49
@jgarzik
Copy link
Contributor

jgarzik commented Aug 26, 2024

It fails here on MacOS 14.6.1:


running 6 tests
test test_walkdir_symlinks ... ok
test test_walkdir_simple ... ok
test test_walkdir_path_prefix_modification ... ok
test test_walkdir_deep_symlinks ... FAILED
test test_walkdir_deep ... FAILED
test test_walkdir_long_filename ... FAILED

failures:

---- test_walkdir_deep_symlinks stdout ----
thread 'test_walkdir_deep_symlinks' panicked at plib/tests/integration.rs:186:16:
Too many open files (os error 24)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_walkdir_deep stdout ----
thread 'test_walkdir_deep' panicked at plib/tests/integration.rs:140:16:
Too many open files (os error 24)

---- test_walkdir_long_filename stdout ----
thread 'test_walkdir_long_filename' panicked at plib/tests/integration.rs:339:34:
called `Result::unwrap()` on an `Err` value: Os { code: 24, kind: Uncategorized, message: "Too many open files" }


failures:
    test_walkdir_deep
    test_walkdir_deep_symlinks
    test_walkdir_long_filename

test result: FAILED. 3 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.48s

error: test failed, to rerun pass `-p plib --test integration`

This left the test system in a state that required manual recovery -- the system rm util would not delete the test dirs in ./target/tmp directory! haha.

@jgarzik
Copy link
Contributor

jgarzik commented Aug 26, 2024

After applying a larger ulimit, the tests proceeded until

running 111 tests
test cp::test_cp_part_symlink ... ignored
test cp::test_cp_proc_short_read ... ignored
test cp::test_cp_dir_slash ... ok
test cp::test_cp_special_bits ... ignored
test cp::test_cp_childproof ... ok
test cp::test_cp_dir_vs_file ... ok
test cp::test_cp_into_self ... ok
test cp::test_cp_hl ... ok
test cp::test_cp_r_vs_symlink ... ok
test cp::test_cp_deref ... ok
test cp::test_cp_fail_perm ... ok
test cp::test_cp_thru_dangling ... ok
test cp::test_cp_i_2 ... ok
test cp::test_cp_special_f ... ok
test cp::test_cp_trailing_slash ... ok
test cp::test_cp_i ... ok
test cp::test_cp_same_file ... ok
test link::link_already_exists ... ok
test link::link_create ... ok
test ls::test_ls_empty_directory ... ok
test ls::test_ls_infloop ... ok
test ls::test_ls_size_align ... ok
test ls::test_ls_rt_1 ... ok
test ls::test_ls_m_option ... ok
test ls::test_ls_no_arg ... ok
test ls::test_ls_file_type ... ok
test ls::test_ls_recursive ... ok
test ls::test_ls_dangle ... ok
test ls::test_ls_inode ... ok
test mkdir::test_directory_already_exists ... ok
test mkdir::test_set_directory_mode ... ok
test mkdir::test_invalid_mode ... ok
test mkdir::test_create_single_directory ... ok
test mv::test_mv_hard_link_1 ... ignored
test mv::test_mv_hardlink_case ... ignored
test mv::test_mv_i_1 ... ok
test mv::test_mv_hard_4 ... ok
test mv::test_mv_atomic ... ok
test mv::test_mv_atomic2 ... ok
test mv::test_mv_i_link_no ... ignored
test mv::test_mv_dir2dir ... ok
test mv::test_mv_into_self_2 ... ignored
test mv::test_mv_hard_2 ... ok
test mv::test_mv_force ... ok
test mv::test_mv_leak_fd ... ignored
test mv::test_mv_part_fail ... ignored
test mv::test_mv_part_hardlink ... ignored
test mv::test_mv_part_rename ... ignored
test mv::test_mv_part_symlink ... ignored
test mv::test_mv_partition_perm ... ignored
test mv::test_mv_dir_file ... ok
test mv::test_mv_special ... ignored
test mv::test_mv_sticky_to_xpart ... ignored
test mv::test_mv_dup_source ... ok
test mv::test_mv_i_5 ... ok
test mv::test_mv_to_symlink ... ignored
test mv::test_mv_into_self ... ok
test mv::test_mv_into_self_3 ... ok
test mv::test_mv_into_self_4 ... ok
test mv::test_mv_perm_1 ... ok
test mv::test_mv_symlink_onto_hardlink_to_self ... ok
test mv::test_mv_childproof ... ok
test rm::test_rm_dangling_symlink ... ignored
test mv::test_mv_i_2 ... ok
test mv::test_mv_i_4 ... ok
test mv::test_mv_symlink_onto_hardlink ... ok
test mv::test_mv_trailing_slash ... ok
test rm::test_rm_dir_no_w ... ok
test rm::test_rm_dir_nonrecur ... ok
test rm::test_rm_cycle ... ok
test rm::test_rm_fail_2eperm ... ignored
test rm::test_rm_f_1 ... ok
test rm::test_rm_dot_rel ... ok
test rm::test_rm_empty_name ... ok
test rm::test_rm_empty_inacc ... ok
test rm::test_rm_i_no_r ... ok
test rm::test_rm_i_1 ... ok
test rm::test_rm_fail_eacces ... ok
test rm::test_rm_ignorable ... ok
test rm::test_rm_no_give_up ... ignored
test rm::test_rm_ir_1 ... ok
test rm::test_rm_inaccessible ... ok
test rm::test_rm_r_4 ... ok
test rm::test_rm_r_3 ... FAILED
test rm::test_rm_rm1 ... ok
test rm::test_rm_rm2 ... ok
test rm::test_rm_rm3 ... ok
test rm::test_rm_rm4 ... ok
test rm::test_rm_readdir_bug ... ok
test rm::test_rm_rm5 ... ok
test rm::test_rm_sunos_1 ... ok
test rm::test_rm_unread2 ... ok
test rm::test_rm_r_root ... ok
test rm::test_rm_unread3 ... ok
test rm::test_rm_unreadable ... ok
test readlink::test_readlink_not_a_symlink ... ok
test readlink::test_readlink_non_existent_file ... ok
test readlink::test_readlink_valid_symlink ... ok
test readlink::test_readlink_valid_symlink_no_newline ... ok
test cp::test_cp_preserve_slink_time ... ok
test rm::test_rm_deep_2 ... ok
test rmdir::rmdir_remove_non_existent_directory ... ok
test rmdir::rmdir_remove_existing_directory ... ok
test rmdir::rmdir_remove_directory_with_parents ... ok
test rmdir::rmdir_remove_non_empty_directory ... ok
test ls::test_ls_time ... ok
test rm::test_rm_isatty ... ok
test unlink::unlink_remove_non_existing_file ... ok
test unlink::unlink_remove_existing_file ... ok
test unlink::unlink_remove_directory ... ok
test rm::test_rm_hash ... ok

failures:

---- rm::test_rm_r_3 stdout ----
thread 'rm::test_rm_r_3' panicked at plib/src/testing.rs:68:5:
assertion `left == right` failed
  left: "thread 'main' panicked at library/core/src/panicking.rs:219:5:\nunsafe precondition(s) violated: ptr::copy_nonoverlapping requires that both pointer arguments are aligned and non-null and the specified memory ranges do not overlap\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\nthread caused non-unwinding panic. aborting.\n"
 right: ""
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    rm::test_rm_r_3

test result: FAILED. 91 passed; 1 failed; 19 ignored; 0 measured; 0 filtered out; finished in 4.04s

error: test failed, to rerun pass `-p posixutils-tree --test tree-tests`

@JRF63 JRF63 marked this pull request as draft August 31, 2024 07:43
@JRF63
Copy link
Contributor Author

JRF63 commented Aug 31, 2024

Looks like I need to dynamically detect the ulimit. It's currently hardcoded to 1024.

@JRF63 JRF63 marked this pull request as ready for review September 11, 2024 12:40
@jgarzik jgarzik merged commit fe084ef into rustcoreutils:main Sep 11, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants