Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(docs): Specify full path to xattr for macOS, fixes #2583 #2586

Merged
merged 2 commits into from
Nov 10, 2024

Conversation

peterkaminski
Copy link
Contributor

On some Macs, Homebrew installs its own xattr which doesn't have the -r option. See #2583.

This PR just adds the full path to the system xattr in README.md, so the -r in the instructions doesn't cause a problem.

It's not clear to me whether or not the -r is required; this PR does not address that question, it just routes around it.

I have tested the instruction on my Mac which has the Homebrew xattr, and with the added path, it works.

I would assume this change is low risk; it's just to documentation, and the change points to a system command that should always be there. However, it's possible I'm missing corner cases that further testing and validation on more machines would uncover.

…ixes th-ch#2583

On some Macs, Homebrew installs its own `xattr` which doesn't have the `-r` option. See th-ch#2583.

This PR just adds the full path to the system `xattr` in `README.md`, so the `-r` in the instructions doesn't cause a problem.

It's not clear to me whether or not the `-r` is required; this PR does not address that question, it just routes around it.

I have tested the instruction on my Mac which has the Homebrew `xattr`, and with the added path, it works.

I would assume this change is low risk; it's just to documentation, and the change points to a system command that should always be there. However, it's possible I'm missing corner cases that further testing and validation on more machines would uncover.
Copy link
Owner

@th-ch th-ch left a comment

Choose a reason for hiding this comment

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

Overall the change looks good, we just need to apply to other READMEs (docs/readme/) as well!

Changed `xattr` instruction to `/usr/bin/xattr` in non-en README.md files. Fix for th-ch#2583.

Done by recursive grep, so this should be all of them. Command line used:

```shell
perl -p -i -e 's/^xattr/\/usr\/bin\/xattr/' `egrep -rl xattr .`
```
@peterkaminski
Copy link
Contributor Author

Overall the change looks good, we just need to apply to other READMEs (docs/readme/) as well!

@th-ch, now applied to the other READMEs as well: es, fr, is, ko, ru.

@JellyBrick JellyBrick merged commit 3b7697a into th-ch:master Nov 10, 2024
5 checks passed
@JellyBrick JellyBrick changed the title (doc-only change) Specify full path to xattr for macOS, fixes #2583 chore(docs): Specify full path to xattr for macOS, fixes #2583 Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants