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

Remove condition on the import of VisualStudio.targets #15493

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jasonmalinowski
Copy link
Member

@jasonmalinowski jasonmalinowski commented Feb 7, 2025

This condition has the side effect of changing the packages that we depend on, depending on the type of MSBuild used to restore packages. This is because VisualStudio.targets itself contains PackageReferences. This can cause all sorts of slowdowns when restore needs to rerun a lot.

DRAFT NOTE: it's quite possible this will break all sorts of things; this condition is six years old, so there's no way to know what might be "fixed" by it.

This condition has the side effect of changing the packages that we
depend on, depending on the type of MSBuild used to restore packages.
This is because VisualStudio.targets iself contains PackageReferences.
This can cause all sorts of slowdowns when restore needs to rerun a lot.
@jasonmalinowski jasonmalinowski force-pushed the remove-condition-on-visualstudio-import branch from 927ca27 to a86a08b Compare February 7, 2025 01:37
@mmitche
Copy link
Member

mmitche commented Feb 7, 2025

@jasonmalinowski What's the motivation for removing the condition? Does the VSSDK now support netcore?

@jasonmalinowski
Copy link
Member Author

@mmitche: I ran into this condition trying to do write some other targets in a Roslyn repo that depended on the VS SDK targets. Until I found this condition, it meant the VS SDK targets I thought were being imported weren't in a lot of cases, resulting in various obscure error messages in some scenarios.

This is also going to cause restore instability. If you have VS open, VS will restore with these packages, and dotnet build will restore without the packages, and since they can't agree on the actual set of packages, they keep fighting. Run dotnet build with VS open, then it'll restore it's way, causing VS to retrigger it's restore which switches it back over. It breaks incrementality so every dotnet build is a full restore + full build (all while VS is reloading projects and consuming lots of CPU there too.)

I'll be honest I'm not sure if the VS SDK really does support netcore or not -- but years ago we did work to exclude the one specific tool we knew would be problematic, so it's not clear to me if this is still needed. The condition is 6 years old so I'm trying this in draft just to see what breaks if I change it.

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.

2 participants