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

Stop deploying SDKs with OmniSharp #847

Merged
merged 5 commits into from
May 4, 2017

Conversation

DustinCampbell
Copy link
Contributor

Fixes #765

This change stops deploying the .NET Core MSBuild SDKs with OmniSharp. That was a stop gap for .NET Core 1.0. However, with .NET Core 2.0 around the corner, it's time to stop deploying them and rely on the .NET Core SDK installed on the user's machine.

To locate the correct .NET Core SDK, we simply launch dotnet --info in the working directory of the project. That should properly handle the case where the SDK is specified in a global.json file. Then, we process the text returned by dotnet --info and locate the "Base Path" value. Combining this with "Sdks" should be the correct value.

@DustinCampbell
Copy link
Contributor Author

Note, this also shrinks the drops by a good bit.

@filipw
Copy link
Member

filipw commented May 4, 2017

very nice! also the build script should be faster

On a side note, I think we should also review what is the role of OmniSharp.Abstractions. At the moment it's a mix of a little of everything i.e. config class with hardcoded Roslyn versions or endpoint paths, all the request/response models, a bunch of interfaces, a bunch of extension methods and utilities, CLI wrapper like theDotNetCliService etc.

@DustinCampbell
Copy link
Contributor Author

Yes. I've been going through Abstractions and moving things out. For example, a lot of implementations have moved to OmniSharp.Host and OmniSharp.Roslyn.

@DustinCampbell DustinCampbell merged commit 0e8f170 into OmniSharp:dev May 4, 2017
@DustinCampbell
Copy link
Contributor Author

also the build script should be faster

Not by much. Restoring these packages and copying their contents was a fairly small amount of time compared to other steps, like publish.

@juergenhoetzel
Copy link
Contributor

I use -use-global-dotnet-sdk but the non-existing local SDK path is still referenced in the msbuild.sh script:
msbuild.sh: line 3: cd: ./.dotnet/sdk/1.0.1/: No such file or directory

@juergenhoetzel
Copy link
Contributor

There is an Enviroment property in RunOptions:

    public IDictionary<string, string> Environment { get; }

using this property, the actual SDK_DIR could be passed to msbuild.sh?

@DustinCampbell
Copy link
Contributor Author

Yes, this should be done. I'm not sure what your comment has to do with this particular PR though. msbuild.sh was added awhile ago.

@juergenhoetzel
Copy link
Contributor

juergenhoetzel commented May 31, 2017

Yes, this should be done. I'm not sure what your comment has to do with this particular PR though. msbuild.sh was added awhile ago.

I thought the functionality of passing the correct SDK_DIR to msbuild.sh should have been part of this PR, because building on GNU/Linux systems fails if you use this new feature (dont deploy SDK).

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.

3 participants