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

Add APFS snapshot support #415

Merged
merged 9 commits into from
Jul 2, 2018
Merged

Add APFS snapshot support #415

merged 9 commits into from
Jul 2, 2018

Conversation

amarcu5
Copy link
Contributor

@amarcu5 amarcu5 commented Apr 28, 2018

Add's VSS support for macOS using APFS snapshot

Addresses issue #341

Add's VSS support for macOS using APFS snapshot's
@CLAassistant
Copy link

CLAassistant commented Apr 28, 2018

CLA assistant check
All committers have signed the CLA.

Restricts VSS under macOS to version 10.13 High Sierra or higher and local volume only
VSS support now determined by:
1) Checking that repository filesystem is APFS rather than by inspecting macOS version
2) Checking that repository resides on local device (as external APFS formatted drives are not supported) rather than crudely by path name
Fixed tabbing
Minor improvement to error handling and small formatting changes
@leftytennis
Copy link
Contributor

I haven't patched my copy yet, but will do so and test. Thanks for doing this, too bad Apple has decided to make the API for APFS snapshots so convoluted to use. The API itself seems simple, minus the criterial for the special entitlement...

Copy link
Contributor

@leftytennis leftytennis left a comment

Choose a reason for hiding this comment

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

I have tested successfully on my iMac and Macbook Pro, both using APFS. Please update duplicacy_main.go to reflect that -vss is a valid option for windows or macos/apfs. Thanks!

VSS flag usage now indicates support for macOS using APFS in addition to Windows
@amarcu5
Copy link
Contributor Author

amarcu5 commented Apr 29, 2018

@jt70471 All done, thanks, I'd forgotten about that

Copy link
Contributor

@leftytennis leftytennis left a comment

Choose a reason for hiding this comment

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

One more item, I manually tested OK, however, when I updated my cron scripts to include the -vss option for backup, mount and umount commands were not found.

/sbin/ isn't in the PATH by default via cron. I can update that, but might make sense to change your execution of mount and umount to /sbin/mount and /sbin/umount respectively.

Now calls mount and unmount using absolute path
@amarcu5
Copy link
Contributor Author

amarcu5 commented Apr 29, 2018

@jt70471 agreed, keep the suggestions coming!

While I was at it I noticed I inadvertently broke Windows builds but all fixed now

@leftytennis
Copy link
Contributor

This looks great, the only thing I couldn't test was the check that the filesystem is indeed APFS, because I have already converted all of my macs to APFS!

@leftytennis
Copy link
Contributor

@gilbertchen I've been running with this patch/feature for a month w/o issues. Any reason you haven't included yet?

@gilbertchen
Copy link
Owner

Thanks for your contribution and testing! I can't test it myself but if it has been running good for you guys that it should be safe to merge it.

@gilbertchen gilbertchen merged commit 0af7461 into gilbertchen:master Jul 2, 2018
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.

4 participants