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

VSWhere usage currently causes dependency on Visual Studio #74

Closed
Tiberriver256 opened this issue Mar 7, 2024 · 16 comments
Closed

VSWhere usage currently causes dependency on Visual Studio #74

Tiberriver256 opened this issue Mar 7, 2024 · 16 comments
Labels
bug Something isn't working

Comments

@Tiberriver256
Copy link
Contributor

Tiberriver256 commented Mar 7, 2024

Goal

Make contributing to Reqnroll codebase easier for developers who are not using Visual Studio

Additional Information

See discussion here.

Note on background from @SabotageAndi

This is necessary to find the msbuild executable for building the projects. We build it with msbuild, which is a .NET Framework binary, to test the compilation as in Visual Studio. Building within Visual Studio is still done in .NET Framework. This is also true for .NET (Core) projects.

Proposed direction from @gasparnagy

OK. So maybe we should just:

  • make clear what tests need this (so that they can be excluded)
  • maybe provide a backup mechanism to override the MsBuild path with an environment variable an if that is present we don't use vswhere

I also dug into this a bit, and it seems we technically don't need Visual Studio to get VSWhere. There are some alternative ways to install it, one of which is a nuget package.

Before that, though, I'm going to try and document in this bug discussion exactly what tests use it and why.

@Tiberriver256 Tiberriver256 added the bug Something isn't working label Mar 7, 2024
@SabotageAndi
Copy link

vswhere is already added via nuget package in the TestProject folder: https://github.com/reqnroll/Reqnroll/blob/main/Reqnroll.TestProjectGenerator/Reqnroll.TestProjectGenerator/Reqnroll.TestProjectGenerator.csproj#L24

@ajeckmans
Copy link
Contributor

yeah, the issue is vswhere is specifically created to locate Visual Studio or other Microsoft-related build tooling. I think this leaves out .net tooling installed through Jetbrains products, the C# Dev Kit for Visual Studio Code or even a direct install of .NET SDK

@Tiberriver256
Copy link
Contributor Author

Just adding some more notes:

  • VSWhere usage is isolated to VisualStudioFinder.cs
  • Location of the VSWhere executable is a two-stage approach
    • Stage 1: Looks for VSWhere to be installed as part of Visual Studio
    • Stage 2: Looks for VSWhere in nuget packages folder using the Nuget package @SabotageAndi referenced
  • The VisualStudioFinder.cs class has two public methods but only one of the methods has any reference. The one public method that is actually getting used is:
    • FindMSBuild: This method is used here in Compiler.cs
    • The other method Find seems like it was at one point used to locate Microsoft.VisualStudio.PackageGroup.TestTools.Core but as there are not current references to that method my guess is we no longer need it for that.

@Tiberriver256
Copy link
Contributor Author

@gasparnagy
Copy link
Contributor

@Tiberriver256 well, yes, but that would require to restructure the whole generator concept, I fear. The need for testing through MsBuild is getting less and less important, so I would not invest that much time on it. So I think we should stick to executing MsBuild.exe, but get the path in a more flexible way.

@Tiberriver256
Copy link
Contributor Author

Sounds good 👍

A new method for locating MSBuild is provided from a nuget package called Microsoft.Build.Locator. Documentation is provided here.

It supports finding MSBuild installed as part of the dotnet SDK in addition to Visual Studio. The only caveat is provided in this warning:

The Microsoft.Build.Locator package contains assemblies for .NET Framework and .NET Core (also applicable to .NET 5 and later). In a .NET Framework application, the .NET Framework version of Microsoft.Build.Locator is used, and in a .NET Core application, the .NET Core version is used. However, the .NET Core version can only find instances of MSBuild built with .NET Core, the MSBuild used by dotnet.exe in .NET SDK installations, not Visual Studio installations or Visual Studio Build Tools installations. The .NET Framework version of Microsoft.Build.Locator can only see Visual Studio installations, Visual Studio Build Tools installations, not .NET SDK installations. Therefore, it might be necessary to build a tool in two different target framework configurations to target both.

@SabotageAndi
Copy link

The need for testing through MsBuild is getting less and less important.

As long as Visual Studio runs on .NET Framework and uses the integrated MSBuild, this is important.

Specially the assembly loading is different so every third party dependency can make problems.

@gasparnagy
Copy link
Contributor

As long as Visual Studio runs on .NET Framework and uses the integrated MSBuild, this is important.

Sure. That's what I wanted to say with "less and less" - hopefully VS will also change its behavior at some point... :)

Specially the assembly loading is different so every third party dependency can make problems.

Do you mean especially generator plugins? Or have you seen problems with normal lib dependencies as well?

@gasparnagy
Copy link
Contributor

Fixed by #75

@gasparnagy
Copy link
Contributor

Unfortunately this solution does not work. Now as I work on "portability" tests of system testing and want to test the build with MsBuild, it turned out:

  • The Microsoft.Build.Locator, in VisualStudioInstance.MSBuildPath only returns the folder of MsBuild, not the executable itself.
  • The folder it returns is a .NET SDK folder, e.g. C:\Program Files\dotnet\sdk\6.0.321, but that does not contain an msbuild.exe executable, but only an msbuild.dll, that is not usable for our tests
  • In fact the .NET Core frameworks does not contain an msbuild.exe at all. Reading back what @ajeckmans answered, if you install Rider, it actually installs an own version of msbuild.exe, just like Visual Studio does.

Conclusion: I will undo this change and revert to a solution that finds msbuild.exe from VS, if you have one, otherwise it requires you to set an environment variable with the msbuild path or just make sure that msbuild is in the PATH.

@gasparnagy gasparnagy reopened this Mar 26, 2024
@Tiberriver256
Copy link
Contributor Author

Sounds good. Bummer on the sdk only including a dll. I'll take another stab at this but sounds like we should prioritize getting tests running in the CI first before we go again so I don't drop more broken stuff in.

@gasparnagy
Copy link
Contributor

@Tiberriver256 yes, this is my plan with #82. I made the fix there and as a result now it works locally (on a machine with VS). I will try to setup the CI tomorrow.

@ajeckmans
Copy link
Contributor

Next to vswhere, when it fails to locate an installation, we should probably use https://www.nuget.org/packages/JetBrains.Rider.PathLocator (which is https://github.com/JetBrains/RiderSourceCodeAccess/tree/main/Source/RiderSourceCodeAccess/Private/RiderPathLocator) to locate the installation of Rider. If one is found we can use the packaged msbuild.exe. It is in a fixed path within that folder at /tools/MSBuild/Current/Bin/msbuild.exe

@gasparnagy
Copy link
Contributor

@ajeckmans I will do that. Does Rider adds this folder to the PATH maybe? (or at least for the scope of the test execution process)?

@ajeckmans
Copy link
Contributor

I have no idea.

@gasparnagy
Copy link
Contributor

Fixed as part of #82. The Rider heuristics are not added yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants