Fix install SIGPIPE handling and enable Windows build support #9012
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Moved Unix-specific features like mode, perms, entries, and process from main uucore dependencies to cfg(unix) target in Cargo.toml - Added platform-specific error messages for unsupported options in locales - Implemented conditional compilation in install.rs for Unix-only behaviors, such as user/group handling and umask, with fallbacks for non-Unix systems - This refactor allows the install utility to run on Windows and other non-Unix platforms while preserving Unix functionality
- Reformatted the assignment of specified_mode for better readability - Added line breaks and indentation to the mode::parse call to enhance code structure without altering functionality
…ations - Add conditional compilation for Unix-like systems in install.rs and mode.rs - Update imports to remove unused and add necessary ones based on platform - Enhance path handling in standard() with safe byte-to-string conversion - Restructure copy_file() to handle special file types on Unix and add fallback for non-Unix - Stub chmod() on Windows for consistency
Handle Cow<OsStr> to avoid unnecessary allocations when converting to PathBuf, improving efficiency in path handling.
Add #[cfg(unix)] to various tests and imports in test_install.rs to ensure they only compile and run on Unix systems, as they rely on Unix-specific file permissions and ownership features.
Updated tests in test_install.rs to use MAIN_SEPARATOR for path separators, ensuring consistent behavior across Windows and Unix. Adjusted error messages for clarity and added #[cfg(unix)] attributes to platform-specific tests, improving portability and reducing OS-dependent failures.
Remove assertion for dir2 in test_install_dir_dot as dir2/.. resolves to parent and doesn't create dir2 on Windows, ensuring no panic. Use MAIN_SEPARATOR in test_install_dir_req_verbose for consistent output formatting across platforms.
Replaced dynamic format string with hardcoded path in the test assertion to simplify the check for expected output, removing dependency on MAIN_SEPARATOR.
Reorganized import statements and improved code formatting in install.rs and test_install.rs for better consistency and readability.
Removed the unnecessary `#[cfg(unix)] use std::os::unix::prelude::OsStrExt;` import to clean up duplicate functionality already covered by the existing `use std::os::unix::ffi::OsStrExt;`.
Simplify test assertions by using &format! instead of format! with sep parameter for path separators, improving code conciseness and consistency.
- Add #[cfg(unix)] to ChownFailed, InvalidUser, and InvalidGroup errors - Add #[cfg(not(windows))] to StripProgramFailed error and strip_file function - Remove unnecessary non-Unix stub for needs_copy_for_ownership - Improve test formatting for better readability This ensures proper handling of Unix-specific functionalities and removes redundant code for non-Unix platforms.
- Add #[cfg(not(windows))] to std::process import for platform-specific handling - Refactor test assertions to remove unnecessary string references in format! calls for cleaner code
Removed unused std::fs import, reorganized utility imports for better structure, and added #[cfg(not(windows))] guards to constants and functions to ensure compatibility across platforms.
Restrict std::fs usage to Unix systems to ensure compatibility and avoid potential issues on non-Unix platforms where certain filesystem operations may not be applicable.
Enable pipe error handling in the install command to prevent crashes when the output pipe is closed prematurely. Added a test to verify the command fails silently while still performing the installation.
Added "signals" to the features list in the uucore dependency to support signal handling functionality in the install utility, allowing proper interruption and process management during file operations.
Replaces silent failure check with explicit assertion to ensure no stderr is produced on broken pipe, improving test reliability and debugging clarity.
CodSpeed Performance ReportMerging #9012 will improve performances by 2.08%Comparing Summary
Benchmarks breakdown
Footnotes
|
|
GNU testsuite comparison: |
Would you move |
It is necessary to check whether install is supported in the Windows environment for GNU coreutils. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
reworks install so it builds and runs on Windows: platform-conditional imports, uucore feature gating, and locale tweaks for unsupported options
restores the default SIGPIPE behavior on Unix via uucore::signals::enable_pipe_errors()
adjusts test_install_broken_pipe to assert that stderr stays empty even when the consumer closes early
adds the broken-pipe regression test and updates existing tests for cross-platform behaviour
Testing carried out:
cargo check -p uu_install --target x86_64-unknown-linux-gnu
cargo check -p uu_install --target x86_64-pc-windows-msvc
cargo test --test tests test_install::test_install_broken_pipe