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

Fix build on macOS Sierra #644

Merged
merged 6 commits into from
Sep 26, 2016

Conversation

DustinCampbell
Copy link
Contributor

@DustinCampbell DustinCampbell commented Sep 23, 2016

Fixes #643

This should fix builds on macOS Sierra.

There were a couple of problems that needed fixing due to runtime packages not being available for macOS Sierra (osx.10.12-x64).

  1. The build script specified dotnet restore --infer-runtimes. Since there aren't runtime packages for macOS Sierra, it failed. We actually don't to specify this any longer, so I removed the flag.
  2. OmniSharp.Tests was defined in a way that was pretty broken and impacted the build in a nasty way. Essentially, it tries to be both a library (because it contains test helper code), and an app (because it contains tests). This change makes it strictly an app, which fixes "dotnet build". A future change will refactoring the test helper code out into a different library.
  3. dotnet publish needs to specify a runtime on macOS Sierra. Otherwise, it will specify the current runtime (osx.10.12-x64), which doesn't exist. So, the build script now detects the current RID by looking at the output of dotnet --info. Then, in the publish step, the build script checks to see if the current RID is osx.10.12-x64 when attempting to publish for "default" and uses osx.10.11-x64 instead.

Copy link

@wjk wjk left a comment

Choose a reason for hiding this comment

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

👎 No good. It still breaks in the same way as before this change was applied.

@DustinCampbell
Copy link
Contributor Author

really? hmmm...

@DustinCampbell
Copy link
Contributor Author

i think i see where it's failing.

@DustinCampbell
Copy link
Contributor Author

@wjk: I'm installing Sierra now. Want to try again after my last tweak?

@DustinCampbell
Copy link
Contributor Author

nevermind. this won't fix it yet.

The problem is that OmniSharp is using dotnet restore --infer-runtimes to run restore for unit tests. On macOS sierra, that means it'll try to restore for osx.10.12-x64, which won't work until there are new packages published that support that. I'll need to take a closer look at the build script to clean this up.

@DustinCampbell
Copy link
Contributor Author

OK. Fingers crossed. Works on my (macOS Sierra) machine.

@DustinCampbell
Copy link
Contributor Author

cc @david-driscoll

@DustinCampbell
Copy link
Contributor Author

Note: Adding all of the runtimes to the test projects (+ an extra one for AppVeyor) feels just awful. Does anyone have any better ideas here?

Copy link

@wjk wjk left a comment

Choose a reason for hiding this comment

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

👍 Works great! Builds on my macOS Sierra machine with no issues.

@d12frosted
Copy link

Thanks, that fixes my issue! 😸 ❤️

It turned out that the OmniSharp.Tests was defined incorrectly. It was a netcoreapp1.0, but wanted to act like a library.
@DustinCampbell DustinCampbell changed the title Specify osx.10.11-x64 RID in build.cake Fix build on macOS Sierra Sep 26, 2016
Copy link
Member

@david-driscoll david-driscoll left a comment

Choose a reason for hiding this comment

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

Without a mac I can't say it works for me (:sheep:) but changes look good.

@DustinCampbell DustinCampbell merged commit 1fe1df2 into OmniSharp:dev Sep 26, 2016
@DustinCampbell DustinCampbell deleted the fix-macos-build branch November 10, 2016 09:33
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