Skip to content

install: print the OS error, not its Debug form, when removing the destination fails - #15015

Merged
sylvestre merged 2 commits into
uutils:mainfrom
koopatroopa787:install-failed-remove-error-message
Oct 11, 2026
Merged

sylvestre merged 2 commits into
uutils:mainfrom
koopatroopa787:install-failed-remove-error-message

Conversation

@koopatroopa787

@koopatroopa787 koopatroopa787 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

When install could not remove an existing destination file, the message ended with the Rust Debug form of the error (Error: Os { code: 13, kind: PermissionDenied, ... }). Both removal sites now format it with strip_errno, and the redundant Error: prefix is dropped from the message, so it reads Failed to remove existing file 'r--/f'. Permission denied.

This does not change the separate second line (install: Permission denied) mentioned in the issue.

Related to #15003

Copilot AI balanced review requested due to automatic review settings October 1, 2026 14:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Congrats! The gnu test tests/id/setgid is no longer failing!

@xtqqczze

xtqqczze commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

please rebase for CI

show_error!(
"{}",
translate!("install-error-failed-to-remove", "path" => target.quote(), "error" => format!("{e:?}"))
translate!("install-error-failed-to-remove", "path" => target.quote(), "error" => strip_errno(&e))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, do you think that we should remove "Error: " from install-error-failed-to-remove? So that it becomes Failed to remove existing file { $path }. { $error } as the GNU error message is install: cannot create directory 'r--': Permission denied

@xtqqczze

xtqqczze commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

@koopatroopa787 Please fully resolve #15003, or remove Closes #15003 from the PR description. Additionally, the test test_install_failed_remove_existing_error_is_plain seems unnecessary.

@koopatroopa787
koopatroopa787 force-pushed the install-failed-remove-error-message branch from 648eb98 to 6df30fd Compare October 4, 2026 12:29
Copilot AI balanced review requested due to automatic review settings October 4, 2026 12:29
@koopatroopa787

Copy link
Copy Markdown
Contributor Author

Rebased, dropped the test and the Error: prefix, and removed Closes since this only addresses the Debug-format part of #15003.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sylvestre
sylvestre merged commit c5a3ce5 into uutils:main Oct 11, 2026
97 checks passed
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.

5 participants