Home: DeterministicSourcePaths doesn't take NuGet source packages into account

[clairernovotny]

Transferred from https://github.com/dotnet/sdk/issues/11105

Version Used: 5.0p1

Steps to Reproduce:

1. Create a class library
2. Add a package reference to xunit.assert.source
3. ContinuousIntegrationBuild

Expected Behavior:
A source root should be created for the NuGet source package and the path to the source normalized/deterministic.

Actual Behavior:
The source paths for the source package point to the NuGet directory and are not deterministic.

Possible solution:
NuGet has a variable for its package root. We can add a SourceRoot for that and that'd cover all source packages under that root:

  <ItemGroup>
    <SourceRoot Include="$(NuGetPackageRoot)" Condition="'$(NuGetPackageRoot)' != ''" />
  </ItemGroup>

[clairernovotny]

See https://github.com/dotnet/sdk/issues/11105#issuecomment-613130894 for additional solution comment: https://github.com/dotnet/sdk/issues/11105#issuecomment-613130894

[nkolev92]

@clairernovotny @tmat
We'll consider this task at the end of our sprint.

This change for this should be trivial.

cc @aortiz-msft

[nkolev92]

Self-assigning for Sprint 170 per the MSBuild sync-up discussion.

[clairernovotny]

Just checking in on this?

[nkolev92]

I'll get to this by preview 3, so eow or next week.
Sorry for the delay.

[bording]

FYI I started rolling out the workaround mentioned in the original post, and on some of my builds, it's failing with a SourceRoot paths are required to end with a slash or backslash error.

The odd thing for me is only some of my builds seem to be failing in this way, when in theory they should all be having the same problem. I'm still trying to track down why that's happening.

It seems like the real implementation for this should ensure the added SourceRoot has a trailing slash:

<ItemGroup>
    <SourceRoot Include="$([MSBuild]::EnsureTrailingSlash($(NuGetPackageRoot)))" Condition="'$(NuGetPackageRoot)' != ''" />
</ItemGroup>

@clairernovotny Might be a good idea to update the root post with that.

[nkolev92]

@bording

Guess that's covered by: https://github.com/NuGet/Home/issues/7968

So we're saying that we need https://github.com/NuGet/Home/issues/7968 to be in the same release as this isuse in order to cover the scenario, correct?

[bording]

If you're wanting to be more generally thorough, it does seem like that is needed. To solve the specific problem I ran into, using EnsureTrailingSlash seems to be enough.

[bording]

I was able to track down the reason I was seeing some failures on some of my builds. The failing builds had the NUGET_PACKAGES env var set to override the default location, and the value did not have a trailing slash.

[nkolev92]

The CI says SourceRoot needs to have a trailing slash.
So looks like it's a hard requirement.
We need to prioritize #7968 too.

Unless someone adds a SourceRoot Update EnsureTrailingSlash :)

[davkean]

As a heads up, this ended up breaking our build:

  "F:\workspace\_work\1\s\build\proj\Build.proj" (default target) (1) ->
       "F:\workspace\_work\1\s\build\proj\CoreBuild.proj" (default target) (2) ->
       "F:\workspace\_work\1\s\ProjectSystem.sln" (Build;Test;Pack target) (3:2) ->
       "F:\workspace\_work\1\s\src\Microsoft.VisualStudio.ProjectSystem.Managed.VS.Client\Microsoft.VisualStudio.ProjectSystem.Managed.VS.Client.csproj" (default target) (13:10) ->
         C:\Users\vsagent\.nuget\packages\roslyntools.repotoolset\1.0.0-beta-62705-01\tools\SourceLink.targets(31,5): error : SourceRoot.SourceLinkUrl is empty: 'C:\Users\vsagent\.nuget\packages\' [F:\workspace\_work\1\s\src\Microsoft.VisualStudio.ProjectSystem.Managed.VS.Client\Microsoft.VisualStudio.ProjectSystem.Managed.VS.Client.csproj]

This is a hybrid version of sourcelink so not sure if other folks will run into this, but left it here in case other folks run into this. I plan on working around it.

[tmat]

@davkean What version of SourceLink package is used in the project-system repo?

[tmat]

I can't reproduce the above error in a vanilla repository with dotnet library created by dotnet new classlib.

I found another issue with this change though - it does not have the desired effect.
Filed https://github.com/NuGet/Home/issues/9810