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

Feature Request: Use regular MSBuild Binary Location #35

Open
MeikTranel opened this issue Feb 21, 2018 · 19 comments
Open

Feature Request: Use regular MSBuild Binary Location #35

MeikTranel opened this issue Feb 21, 2018 · 19 comments

Comments

@MeikTranel
Copy link

Using this VSCode Extension for opening some CSProj Files gives out errors because the well-known MSBuild variables link to very different locations than the build system actually uses.

Example:
When editing CSProj Files for Portable Class Libraries you'll see that the line
<Import Project="$(MSBuildExtensionsPath32)\Microsoft\Portable\$(TargetFrameworkVersion)\Microsoft.Portable.CSharp.targets" />

shows an error, because it relies on the fact that PCL Supports Libraries and Targets are available using the provided path from the given default MSBuild binary directory: in this case: $(MSBuildExtensionsPath32)

When actually validating the csproj (through building e.g.) MSBuild provides this variable as part of "well
-known MSBuild Properties" that are preset by the binary itself.

But because the vscode extension provides the msbuild binary itself as part of the language server, those well-known Properties are skewed towards

....\extensions\tintoy-msbuild-project-tools-0.2.27\out\language-server\

where there are no extensions for msbuild.
Most pre-installed extensions for MSBuild, like WiX Toolset or PCL-Support are installed next to the visual studio / msbuild binaries, where they can then use well-known MSBuild properties to move towards their relative directory.

I would request that the vscode-extension lets me override the default msbuild binary location via settings, so it can use my Visual Studio install MSBuild binaries instead of the binaries pre-deployed with the language-server components.

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

Hey, thanks for the detailed report! I'm pretty sure we can find a way to make it work correctly for your use-case. It's 11pm here in AU, but I'll look into this first thing tomorrow :)

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

Just one thing to be aware of; task assemblies that truly require the full framework rather than netcoreapp2.0 probably won't work because the language service has to target that for cross-platform support (but let's see what we can do)

@MeikTranel
Copy link
Author

No problem. Take your time. This is nothing stopping me from work.

As for the netcoreapp2.0 thing; Indeed that could be an issue, although....

all the extension needs is the targets/props files right? It doesn't try to load the DLL to read properties right? The target file itself should be framework independent. The DLLs are only required at runtime, at which point the whole thing isn't in your hand anymore?
Or am i wrong?

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

No, you're correct :)

It's only if you have targets that compute items dynamically using inline tasks (or netfx ones) that you'd even notice I think.

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

We deliberately use the MSBuild equivalent of the ReflectionOnly assembly-load context (i.e. can't invoke targets).

@MeikTranel
Copy link
Author

MeikTranel commented Feb 21, 2018 via email

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018 via email

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

Here's where we set up the project collection:

https://github.com/tintoy/msbuild-project-tools-server/blob/master/src/LanguageServer.Common/Utilities/MSBuildHelper.cs

I just need to make the extensions directory customisable (and make sure that doesn't break anything by making the .net core targets inaccessible).

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

For example, SDK imports

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

Ok, so having had a look at the code, we default to using %DOTNET_SDK_DIR% (e.g. C:\Program Files\dotnet\sdk\2.1.4 for $(MSBuildExtensionPath).

This directory does contain the targets required to make dotnet SDK-style projects work, but they don't play so well with anything designed to build on the full framework or using Windows-only project types such as PCL projects (PCL projects can target other platforms, but have to be built on Windows).

I can provide settings to override $(MSBuildExtensionsPath) and $(MSBuildExtensionsPath32), but these will be per-workspace; if you have both styles of project in the same solution then one or the other would still be broken (depending on whether or not you choose to override them).

I might be able to detect whether the root <Project> element has an Sdk attribute and let that drive whether the overrides take effect or not, but perhaps I should ask the MSBuild folks whether that's the most correct approach.

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

Just out of curiosity, if you run "dotnet build" on your PCL project, does it build correctly?

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

This issue makes me think that perhaps it might all work if you only override MSBuildExtensionsPath32 since that's what PCL use (whereas other projects use the non-32 version):

dotnet/project-system#1369

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

If I can get you a custom build would you be willing to try it out?

@MeikTranel
Copy link
Author

MeikTranel commented Feb 21, 2018

At purely windows shops you don't actually run the dotnet cli all that much.
Let's do it this way. I'm gonna fork this on the weekend and fiddle a little bit, because there's really not many people out there who could give a more diverse reallife testing scenario for weird msbuild things than my current job 😄 i always wanted to do some typescript stuff. Let me throw you a pull request this weekend or maybe next week and then you have a look?

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

@MeikTranel that would be fantastic!

If it helps, here's a branch where I was playing around with the ideas we've been discussing:

https://github.com/tintoy/msbuild-project-tools-server/tree/feature/msbuild-extensions-directory

(the language server is in a separate repository because other editors, such as Eclipse, use the language service without the VSCode extension)

It might be worth making the property overrides more generic, though - perhaps if the configuration looked something like this:

"msbuild": {
    "overrideGlobalProperties": {
        "MSBuildExtensionsPath32": "C:\\Program Files (x86)\\MSBuild"
    }
}

What do you think?

Anyway, feel free to have a play around, open a PR and we can discuss further. Thanks for getting involved!

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

PS. See the HEAD commit on that branch for details.

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

PPS. On the extension side, here's the example configuration change I was playing with: a05fa2a

@tintoy
Copy link
Owner

tintoy commented Feb 21, 2018

Oh, and finally - all you need to do to build and use the language service is:

  1. Check out this repository (recursively, so the submodule containing the language server at lib\server is included).
  2. Run .\Build-LanguageServer.ps1 -VersionPrefix 0.2.28 -VersionSuffix dev
  3. Run vscode ..
  4. Press F5 :-D

@tintoy
Copy link
Owner

tintoy commented Mar 21, 2018

@MeikTranel - have you had a chance to play around with this yet? If you don't have the time, I'm happy to implement the option I mentioned above (override MSBuild global properties) which should get you 80% of the way towards what you need to do...

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

No branches or pull requests

2 participants