Skip to content

Conversation

@TGNThump
Copy link
Contributor

Fixes #1762

Signed-off-by: Ben Pilgrim <ben@pilgrim.me.uk>
@bootc-bot bootc-bot bot requested a review from gursewak1997 November 13, 2025 23:24
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds the --remove-destination flag to a cp command within the copy_etc_to_state function. The goal is likely to make the copy operation more robust by ensuring destination files are removed before copying. While this is a good intention, my review highlights a potential portability issue. The --remove-destination flag is specific to GNU coreutils' cp and is not universally available, for instance in busybox. This could lead to failures if the code runs in a minimal environment. I've suggested that a more robust, long-term solution would be to replace the external command with a native Rust implementation for file copying, which would also resolve an existing TODO in the code.

Copy link
Contributor

@gursewak1997 gursewak1997 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gursewak1997 gursewak1997 enabled auto-merge (squash) November 14, 2025 01:12
@gursewak1997 gursewak1997 merged commit a0cdb06 into bootc-dev:main Nov 14, 2025
46 of 48 checks passed
@TGNThump TGNThump deleted the fix/cp-remove-destination branch November 14, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

etc write error: not writing through dangling symlink

3 participants