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

Support MacOS #24

Merged
merged 8 commits into from
Dec 6, 2021
Merged

Support MacOS #24

merged 8 commits into from
Dec 6, 2021

Conversation

montaguegabe
Copy link
Contributor

This issue fixes #23, a problem apparently appearing on MacOS 10.8 and above. The diff command has been changed as well on Mac. Added instructions on how to install the matching grep and diff commands, and modified the code to use them.

README.md Outdated

| Mathod | Command |
|--------|---------------------------------------------------------------------------------------------------------------|
| curl | `sudo sh -c "$(curl -fsSL https://raw.githubusercontent.com/sp1thas/dropboxignore/master/utils/install.sh)"` |
| wget | `sudo sh -c "$(wget -qO- https://raw.githubusercontent.com/sp1thas/dropboxignore/master/utils/install.sh)"` |
| fetch | `sudo sh -c "$(fetch -o - https://raw.githubusercontent.com/sp1thas/dropboxignore/master/utils/install.sh)"` |

### Installation notes for MacOS
Copy link
Owner

Choose a reason for hiding this comment

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

I propose to add these brew install commands directly into the utils/install.sh file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good - I forgot that users of dropboxignore are pretty much guaranteed to be developers so they will definitely have homebrew

@sp1thas
Copy link
Owner

sp1thas commented Dec 4, 2021

Thank you @montaguegabe for your contribution. Your documentation-related changes should also be applied in docs/installation.md (I know... duplicated docs.. we should find a better approach on this)

@montaguegabe
Copy link
Contributor Author

I can update so that the brew commands are part of the install script and modify the docs in both places

(PyCharm gives warnings for removing quotes around "diff" so I left all the quotes after the equals signs for consistency - feel free to add commit to change further)
@montaguegabe
Copy link
Contributor Author

One thing to note actually: at least on my Mac running a brew install command will automatically update homebrew, which can sometimes take up to ~3 minutes of somewhat verbose output (maybe even longer if it hasn't been updated in a while). An unwary user might think all this heavy lifting is to install dropboxignore. But I guess we can circumvent the auto-update with the HOMEBREW_NO_AUTO_UPDATE=1 prefix so maybe I'll do that. I'm not exactly sure what the rationale is behind Homebrew trying to update each usage

@montaguegabe
Copy link
Contributor Author

Sorry don't merge this yet - I hadn't tested install.sh and brew doesn't work with sudo

@montaguegabe
Copy link
Contributor Author

Ok all set! I figured out how to drop sudo for the brew commands as per the second answer on https://unix.stackexchange.com/questions/353206/is-there-a-reverse-sudo (I actually tested a mock brew command to make sure it wasn't secretly executing with root permissions). Also added a fix for the few people who have brew install packages to a different location. Have tested the last commit: both the install script and the installed command itself.

Copy link
Owner

@sp1thas sp1thas left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks :)

@sp1thas sp1thas merged commit f662ba8 into sp1thas:master Dec 6, 2021
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.

Broken on later versions of OS X
2 participants