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

Several updates to build #924

Merged
merged 13 commits into from
Jul 24, 2017
Merged

Conversation

DustinCampbell
Copy link
Contributor

@DustinCampbell DustinCampbell commented Jul 23, 2017

This PR cherry-picks several changes from #915.

  • We're moving to the very latest Mono. This is necessary to take advantage of the new --assembly-loader flag for the Mono runtime, which does a better job of simulating the .NET Framework assembly loading behavior on Mono. In addition, the msbuild that ships with Mono is much more mature, so we can call it directly.
  • Install the .NET Core SDK during Travis runs. This allows the sanity runs of OmniSharp that occur at the end of the build to work properly, which has been broken since the Sdks were removed from OmniSharp.
  • Platform detection code to determine whether we're on Windows, OSX, Linux, and whether the current platform is 32- or 64-bit. This is needed for pulling the correct Mono runtime.
  • Validation step that tests the installed Mono to ensure it's the correct version.

I structured each commit to be digestible.


using System;

public class Platform
Copy link
Member

Choose a reason for hiding this comment

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

it's nice that we have robust platform information captured in one place. Maybe for consistency we can replace the calls to the built-in Cake IsRunningOnWindows() global methods used in several places with platform.IsWindows. This way we have a guarantee the two logics would not diverge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll do that.

@filipw
Copy link
Member

filipw commented Jul 24, 2017

it's a shame Cake is on a very old version of Roslyn (1.0.0-rc2). In the newer versions of Roslyn scripting, top level extension methods are supported in scripts which would be perfect for complex build script like this one

Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

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

The commit messages are 🍻

@DustinCampbell
Copy link
Contributor Author

it's a shame Cake is on a very old version of Roslyn (1.0.0-rc2).

Agreed, though it's worse than that. AFAICT, Cake only seems to use Roslyn Scripting on Windows. It seems to use the Mono scripting dialect on OSX/Linux, which has very different semantics.

@bjorkstromm
Copy link
Member

@filipw @DustinCampbell we are updating Roslyn and unifying script engines in next release 😉

PR here cake-build/cake#1645

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.

LGTM

@DustinCampbell DustinCampbell merged commit d3d1915 into OmniSharp:master Jul 24, 2017
@DustinCampbell DustinCampbell deleted the update-build branch August 30, 2017 19:13
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