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 Mac OS build setup steps #287

Merged
merged 12 commits into from
Oct 4, 2018

Conversation

kyle-rader
Copy link
Contributor

These steps are the outcome of getting a working build on a Mac pro (Late 2013) with a freshly installed

macOS High Sierra 10.13.6

Updates:

  • Add a Build setup step to the main README.md
  • Remove the macOS sdk version from the ProjFS build scripts. (Because XCode merrrrrrr removes SDKs at will...)

Questions:

  • What to say about the Apple Developer/Cert Signing step?

Copy link
Member

@nickgra nickgra left a comment

Choose a reason for hiding this comment

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

I'm not sure what to say about developer certificates, I find most open source projects skim over how to handle getting a certificate so this might be fine?

Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Show resolved Hide resolved
Readme.md Outdated

VFS for macOS is still in progress. You can build it, but this will not create a macOS VFS installer the same way the current Windows build will.

* Ensure you have Xcode installed and have accepted the terms of use and launched Xcode at least once.
Copy link
Member

Choose a reason for hiding this comment

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

nit: Only use and one in this step. replace the first and with ,

Copy link
Member

@nickgra nickgra left a comment

Choose a reason for hiding this comment

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

LGTM now, I'm curious to hear what Saeed has to say about the level of detail and certificate handling.

Copy link
Contributor

@sanoursa sanoursa left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together! Mostly minor cleanup though we need to make sure we validate the build script changes a bit more. And keep in mind that you still have to kick off the Mac functional tests manually.

ProjFS.Mac/Scripts/Build.sh Show resolved Hide resolved
Readme.md Outdated
build will fail, and the second and subsequent builds will succeed. This is because the build requires a prebuild code generation step.
For details, see the build script in the previous step.

The installer can now be found at `C:\Repos\VFSForGit\BuildOutput\GVFS.Installer\bin\x64\[Debug|Release]\SetupGVFS.<version>.exe`

## Building VFS for Git on Mac

VFS for macOS is still in progress. You can build it, but this will not create a macOS VFS installer the same way the current Windows build will.
Copy link
Contributor

Choose a reason for hiding this comment

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

We've been saying "VFS for Git on Mac", so stick with that for consistency. I'm open to feedback if it would be better to say "VFS for Git on macOS", but "VFS for macOS" is a whole different thing (https://developer.apple.com/library/archive/documentation/Darwin/Conceptual/KernelProgramming/Filesystem/Filesystem.html)

Readme.md Outdated

* If you still do not have the `dotnet` cli `>= v2.1.300` installed [manually install it](https://www.microsoft.com/net/download/dotnet-core/2.1)

* If you are not currently an Apple Developer you will need to become one so that you can either use Microsoft certs or create your own certs for signing purposes. (If you are a Microsoft employee you must use your alias@microsoft.com when creating your Apple account.)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove all of this and instead point to a link for managing certs in Xcode. There's no reason for anyone to use Microsoft certs, nor do we want to open that up. For local development, people just need to generate their own cert and sign their kext, and that is all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How is this as the resource to link to?

Copy link
Member

Choose a reason for hiding this comment

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

That page seems too high level on what a certificate is (we should point them to codesigning resources using certificates), this might be a better starting point https://developer.apple.com/support/code-signing/

In general, Xcode should be able to do all the right things for them once they're signed in.

Readme.md Outdated

* If you are not currently an Apple Developer you will need to become one so that you can either use Microsoft certs or create your own certs for signing purposes. (If you are a Microsoft employee you must use your alias@microsoft.com when creating your Apple account.)

* Create a `VFS` directory and Clone the VFS repo into a directory called `src` inside it:
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no VFS repo :-). It's called VFSForGit.

Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated
gvfs clone URL_TO_REPOSITORY --local-cache-path ~/.gvfsCache
```

Note the current use of `--local-cache-path`. That argument prevent gvfs from running into a permissions error trying to put the cache at the root of your Mac's hard-drive.
Copy link
Contributor

Choose a reason for hiding this comment

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

... because porting of this feature is still in progress

Scripts/Mac/PrepFunctionalTests.sh Show resolved Hide resolved
Readme.md Outdated

* Ensure you have Xcode installed and have accepted the terms of use and launched Xcode at least once.

* Disable "System Integrity Protection" (for loading unsigned kexts) by booting into recovery mode (`[Win/⌘] + R` while booting).
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't load an unsigned kext, period. Our kexts are not unsigned, they are just signed with developer certs.

Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated

* Create a `VFS` directory and Clone the VFS repo into a directory called `src` inside it:
```
mkdir VFS && cd VFS && git clone https://github.com/Microsoft/VFSForGit.git src && cd src
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow a similar style as the Windows section for the steps on how to clone and build.

Actually I'm not picky on what the style is, just that they be consistent.

Copy link
Contributor

@sanoursa sanoursa left a comment

Choose a reason for hiding this comment

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

I started reviewing this again but it looks like some of the earlier comments are still in progress. I'll stop here, and please let me know when you're ready for another look!

Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
@kyle-rader
Copy link
Contributor Author

@sanoursa @wilbaker Updated and rebased on master. Ready for re-review

Copy link
Contributor

@sanoursa sanoursa left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup, the steps look much clearer now. A few more style things, but most importantly, we need to be super clear about the dangers of disabling SIP. This is not something we are encouraging anyone to do, and it's just for now while we're doing active development that it's a necessary step, but it is most definitely a dangerous thing to do to any production machine.

Readme.md Outdated

* Ensure you have Xcode installed, have accepted the terms of use, and have launched Xcode at least once.

* Disable the "System Integrity Protection" (for loading unsigned Kexts) by booting into recovery mode (`[Win/⌘] + R` while booting).
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no such thing as an unsigned kext. You can't even build it if you don't have your cert set up. What we do have are dev-signed kexts and production-signed kexts.

Also please don't capitalize kext.

Readme.md Outdated

* Ensure you have Xcode installed, have accepted the terms of use, and have launched Xcode at least once.

* Disable the "System Integrity Protection" (for loading unsigned Kexts) by booting into recovery mode (`[Win/⌘] + R` while booting).
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want a giant warning message here that this is a Very Bad Thing for the security of your machine. We are not recommending this on any production machine, ever. This is for development and testing only. Etc etc.

Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
@kyle-rader
Copy link
Contributor Author

@nickgra @sanoursa I've pushed resolutions to all remaining comments

Copy link
Contributor

@sanoursa sanoursa left a comment

Choose a reason for hiding this comment

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

Thanks for all the cleanup, I think this looks good!

Copy link
Member

@nickgra nickgra left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me minus some phrasing nits.

Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
Readme.md Outdated Show resolved Hide resolved
@kyle-rader kyle-rader merged commit b613d4f into microsoft:master Oct 4, 2018
@kyle-rader kyle-rader deleted the chore/add-mac-build-steps branch October 4, 2018 22:22
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