Skip to content

Conversation

chirsz-ever
Copy link
Contributor

POSIX find do not support -lname: https://pubs.opengroup.org/onlinepubs/9799919799/utilities/find.html

The purpose of this #PR is the same as #1010, which is to use the busybox toolchain to run the script.

It seems a bit too complicated and reduces readability. Feel free to close this PR.

@chirsz-ever chirsz-ever force-pushed the chirsz/posix-compatibility-250729 branch 2 times, most recently from abc8312 to 9f38d5d Compare July 29, 2025 02:24
@p-linnane p-linnane requested a review from Copilot July 29, 2025 02:46
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves POSIX compatibility in the uninstall.sh script by replacing the non-POSIX -lname option in the find command with a POSIX-compatible alternative using -exec and shell commands.

  • Replaces find command's -lname option with -exec and shell command to maintain the same functionality while ensuring POSIX compatibility

@chirsz-ever chirsz-ever force-pushed the chirsz/posix-compatibility-250729 branch from 9f38d5d to be5d14d Compare July 29, 2025 03:27
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

This is significantly less readable. I don't think it's worthwhile to have a bunch of these PRs trying to fix Busybox bit by bit, sorry. If you can open one that entirely fixes it and add a new GitHub Actions target to ensure it doesn't regress, we'll consider that (but may still reject it if too invasive).

@chirsz-ever
Copy link
Contributor Author

The uninstall script is not used often, and it would be easy to edit it before executing, so this PR is closed.

@chirsz-ever chirsz-ever deleted the chirsz/posix-compatibility-250729 branch August 3, 2025 17:35
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.

2 participants