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 back code that sets MSBuildSDKsPath environment variable for each project #1021

Merged

Conversation

DustinCampbell
Copy link
Contributor

Unfortunately, it seems that there are some reliability issues with the MSBuild SDK Resolvers that we haven't gotten to the bottom of yet. So, I'm adding back code to set the MSBuildSDKsPath environment variable per project based on running the dotnet in that project's directory.

This should address issues like dotnet/vscode-csharp#1849 and dotnet/vscode-csharp#1846.

Note that this is not a long term solution. It will work for now, but there are several future concerns that we need to be mindful of:

  1. Setting an environment variable will be problematic if we ever want to run design-time builds in parallel. Currently, that's blocked by several issues, but maybe someday.
  2. If the dotnet --info output changes such that Base Path no longer appears in the text, this will break and need to be updated.
  3. This requires a process to be launched for every project. This isn't out biggest performance issue at the moment and it's what we were doing before, but this is something to keep in mind.

cc @nguerrera, @livarcocc

@DustinCampbell DustinCampbell merged commit 4b389b0 into OmniSharp:release/1.26.2 Nov 10, 2017
@DustinCampbell
Copy link
Contributor Author

Thanks!

DustinCampbell added a commit that referenced this pull request Nov 10, 2017
@haifengmeiITC
Copy link

@DustinCampbell can you also apply this change to the omnisharp-vscode project?

@DustinCampbell
Copy link
Contributor Author

Yes. It will take time though. These builds go through some process and get uploaded to a couple of different CDNs first.

@nguerrera
Copy link

Have you ever been able to repro the reliability issue?

@DustinCampbell
Copy link
Contributor Author

I haven't been able to repro it at all, but there are enough reports that I want to add this back in the meantime.

@nguerrera
Copy link

I suspect dotnet/msbuild#2532 is different by the way. See my comments there.

@DustinCampbell
Copy link
Contributor Author

Yeah, I saw that. This issue has me a bit mystified though. We don't do anything special with the SdkResolvers -- just copy them in place. Then, we just use standard MSBuild APIs to evaluate and build a project. No funny business. 😀

Users are reporting problems on multiple OSs.

@DustinCampbell DustinCampbell deleted the fix-sdks-path branch April 11, 2018 21:12
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.

7 participants