dotnet / project-system

The .NET Project System for Visual Studio
MIT License
967 stars 386 forks source link

Legacy project system failing to properly evaluate PropertyGroup items #4868

Open bording opened 5 years ago

bording commented 5 years ago

Visual Studio Version: VS 2019 16.1.1

Summary: Since starting to use VS 2019, I've been seeing a problem when building solutions that use the legacy project system. The solutions fail to build with various different errors, but they all have a common problem. It appears that there are <PropertyGroup> items that aren't being properly evaluated, so various targets aren't being run.

Using the specific project I've listed in this issue as an example, the main project fails to build an assembly with a strong name.

The other thing I've noticed is that when the build fails, the GetVersion target (which comes from the GitVersionTask package) does not run because of:

Target "GetVersion" skipped, due to false condition; ($(GetVersion) == 'true') was evaluated as ( == 'true').

But if you look at the GitVersionTask.targets file, $(GetVersion) is defined as

<GetVersion Condition=" '$(GetVersion)' == '' ">true</GetVersion>

So the condition shouldn't be false.

Steps to Reproduce:

  1. Clone https://github.com/Particular/NServiceBus.AzureServiceBus
  2. Check out the support-7.2 branch
  3. Build the solution and observe build failure
  4. Rebuild the solution multiple times and see build failure (sometimes it succeeds, sometimes it fails)
  5. Close the solution and re-open it.
  6. Build the solution and observe that it never fails

Expected Behavior: The solution should always successfully build.

Actual Behavior: The solution fails with the following errors:

Error   CS8002  Referenced assembly 'NServiceBus.Azure.Transports.WindowsAzureServiceBus, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' does not have a strong name.   NServiceBus.AzureServiceBus.AcceptanceTests
Error   CS8002  Referenced assembly 'NServiceBus.Azure.Transports.WindowsAzureServiceBus, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' does not have a strong name.   NServiceBus.AzureServiceBus.Tests

I was able to repro this with the referenced solution very consistently, but when I started writing this issue and went to go back and get the error details, it started being somewhat inconsistent, with Step 4 sometimes succeeding instead of always failing. Because of this, it seems likely a race condition is involved!

User Impact: The impact varies, but it seems like it can be large. In this case, the problem causes incorrect build output (unsigned assembly despite the project being configured to sign it) that doesn't cause a build failure in the main project. The solution failures are from the test projects that reference the main project, so if those weren't there, this problem might go unnoticed.

NOTE: I know this repo is for the new project system and not the legacy system, but I wasn't sure if there was another public repo that would actually be relevant or not.

davkean commented 5 years ago

Does that property group that sets the property come from a NuGet package? //cc @tmeschter

davkean commented 5 years ago

I'm guessing yes: https://github.com/Particular/NServiceBus.AzureServiceBus/blob/support-7.2/src/Transport/NServiceBus.AzureServiceBus.csproj#L282.

bording commented 5 years ago

Does that property group that sets the property come from a NuGet package?

Yes, it does come from the GitVersionTask package.

tmeschter commented 5 years ago

@bording And these projects use packages.config style of NuGet, as opposed to project.json/PackageReference?

bording commented 5 years ago

@tmeschter Yes, all of the projects where I've seen this sort of behavior are using packages.config.

tmeschter commented 5 years ago

@bording And just to confirm, you never had this issue with VS2017?

bording commented 5 years ago

While I can't say for 100% sure, I don't believe it's anything I ever experienced with VS 2017, only since I switched to VS 2019.

I don't have VS 2017 installed anymore, but I'll verify with a coworker who has both installed and has also seen the behavior on VS 2019.

tmeschter commented 5 years ago

@bording Thanks; much appreciated.

@panopticoncentral We may have introduced another evaluation issue into 16.0 or 16.1.

tmeschter commented 5 years ago

Regarding the involvement of a NuGet package: since this uses packages.config-style NuGet and the project system is largely blind to that style, I don't think it is relevant to the bug.

bording commented 5 years ago

@tmeschter I was able to check with some coworkers, and it looks like this is a problem in VS 2017 as well, which is not what I originally understood when I opened the issue.

How does the fact this it repros in VS 2017 as well alter your assessment of the problem?