Repository navigation
uucore: build safe_traversal on AIX and Hurd - #15030
Conversation
Merging this PR will degrade performance by 21.4%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
|
GNU testsuite comparison: |
|
|
||
| [target.'cfg(all(unix, not(any(target_os = "aix", target_os = "hurd", target_os = "redox"))))'.dependencies] | ||
| [target.'cfg(all(unix, not(target_os = "redox")))'.dependencies] | ||
| uucore = { workspace = true, features = ["safe-traversal"] } |
There was a problem hiding this comment.
looks like the safe-traversal feature is unnecessary for chown
There was a problem hiding this comment.
Right, perms already pulls it in. Dropped it from chown, and from chmod and install too, which had the same redundant section.
55e7c4c to
0df23c8
Compare
|
Please note the removal of the fallback path |
|
Added to the description. |
|
|
||
| // aix and hurd only expose the times as `timespec` fields (`st_atim` and | ||
| // so on), whose types vary between them and with the pointer width. | ||
| #[cfg(any(target_os = "aix", target_os = "hurd"))] |
There was a problem hiding this comment.
almost the same function 6 times plus 6 not(any(aix, hurd)) gates on the existing ones. could you please add an aix/hurd branch inside the existing fns instead, like netbsd does?
There was a problem hiding this comment.
Done: the six extra functions and the not(any(aix, hurd)) gates are gone; AIX and Hurd are a branch inside the existing MetadataExt accessors, as for NetBSD.
| // aix and hurd only expose the times as `timespec` fields (`st_atim` and | ||
| // so on), whose types vary between them and with the pointer width. | ||
| #[cfg(any(target_os = "aix", target_os = "hurd"))] | ||
| #[allow(clippy::unnecessary_cast)] |
There was a problem hiding this comment.
the fields are all i32/i64 here, so .into() should work and we can drop the 6 #[allow(clippy::unnecessary_cast)], no?
There was a problem hiding this comment.
The allows are gone, but I used as _ instead of .into(). On x86_64 Hurd the fields are already i64, so .into() gives six useless_conversion errors there (i686 Hurd was fine). as _ is clean under clippy -D warnings for x86_64 Hurd.
This comment was marked as resolved.
This comment was marked as resolved.
The only thing keeping it off them was the stat time fields, which they expose as timespecs (st_atim and so on). Read those there, and gate safe_traversal and its users on Redox alone, so chmod, du, install and rm walk trees through descriptors there too. The path-based chmod helpers that only AIX still used are gone, as are the target-specific safe-traversal features of chmod, chown and install, which already get it through perms. Redox stays excluded: nix has none of the *at calls there.
0df23c8 to
85d3e76
Compare
|
Yes, with libc 0.2.190 nix fails in |
|
Thanks for your PR |
|
now RUSTC_BOOTSTRAP=1 cargo clippy -q -Zbuild-std -p uucore --features=safe-traversal --all-targets --target powerpc64-ibm-aix |
Map the timespec time fields for AIX and Hurd inside the existing
MetadataExtaccessors, and gate safe_traversal and its users on Redox only, where nix has none of the *at calls.With AIX on the safe_traversal path, chmod's AIX-only path-based symlink handling is no longer used and is removed:
handle_symlink_during_traversaland its wrapperhandle_symlink_during_recursion. If safe_traversal can't be brought to some other platform later, they are in this commit to restore from. The target-specificsafe-traversalfeatures of chmod, chown and install are also dropped, sincepermsalready enables it.Cross-checked with
-Zbuild-stdclippy for x86_64-unknown-hurd-gnu and i686-unknown-hurd-gnu; not run on Hurd. AIX (powerpc64-ibm-aix) doesn't build with libc 0.2.190 on main either (#15083); with libc pinned to 0.2.189 locally, uucore's safe_traversal builds. uu_du still fails to build on Hurd in fsext, as it does on main.Closes #15028