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

SourceLink doesn't work well with WPF projects #91

Closed
tmat opened this issue Jun 13, 2018 · 15 comments
Closed

SourceLink doesn't work well with WPF projects #91

tmat opened this issue Jun 13, 2018 · 15 comments
Labels
external wontfix This will not be worked on

Comments

@tmat
Copy link
Member

tmat commented Jun 13, 2018

Error

Microsoft.Managed.Core.targets(90,5): SourceRoot items must include at least one top-level (not nested) item when DeterministicSourcePaths is true [C:\Temp\WpfApp1\WpfApp1\WpfApp1_3qoc4wf0_wpftmp.csproj]

is reported when building with /p:ContinuousIntegrationBuild=true.

Due to NuGet/Home#5894 - WPF projects do not import .props and .targets files from NuGet packages.

@tmat
Copy link
Member Author

tmat commented Jun 13, 2018

The workaround is to add

<DeterministicSourcePaths Condition="'$(EnableSourceLink)' == ''">false</DeterministicSourcePaths>

to the .csproj file.

@JamesNK
Copy link
Member

JamesNK commented Jun 23, 2018

I get this in Newtonsoft.Json.Tests and Newtonsoft.Json.TestConsole when I build. I'm guessing because IsPackable is false?

@JamesNK
Copy link
Member

JamesNK commented Jun 24, 2018

Or is the issue that I am building an entire solution and source link doesn't like multiple csprojs?

I've fixed the error by adding the code above to the two test projects - https://github.com/JamesNK/Newtonsoft.Json/pull/1746/files#diff-9d91529ee3797584a27061bbad1d49ceR16

@tmat
Copy link
Member Author

tmat commented Jun 27, 2018

The issue in Newtonsoft.Json is related but different than the WPF issue.

When DeterministicSourcePaths is enabled the compiler needs to know the set of directories that all source code resides under. This set is specified by SourceRoot items.
In projects that reference SourceLink package SourceRoot items are populated by SourceLink since it knows where the repository root is.

In projects that do not reference SourceLink package this SourceRoot is undefined and hence the compiler doesn’t know how to normalize the paths and reports the error [1].

The recommendation is to include the SourceLink package reference to all projects in the repo (even to test projects). This can be easily accomplished via adding Directory.Build.props file in the root of the repository that includes the SoruceLink package:

<Project>
  <PackageReference Include="Microsoft.SourceLink.GitHub" Version="..." PrivateAssets="all"/>
</Project>

[1] The compiler does not infer the source root(s) based on source file paths it is given as this might produce confusing results. Consider a repo with two projects:

.../src/ProjectA/ProjectA.csproj
.../src/ProjectA/Program.cs
.../src/ProjectB/ProjectB.csproj
.../src/ProjectB/Program.cs

If the compiler inferred the source roots for these projects they would be .../src/ProjectA/ and .../src/ProjectB/ and the paths embedded in DLLs and PDBs to Program.cs files would be /_/Program.cs for both .../src/ProjectA/Program.cs and .../src/ProjectB/Program.cs. So it would be impossible to distinguish which Program.cs is which.

@tmat tmat added wontfix This will not be worked on and removed wontfix This will not be worked on labels Jun 27, 2018
@xied75
Copy link

xied75 commented Aug 13, 2018

In my case I have a src project with .NET Framework and a test project, building and packaging in a Jenkins. So that I need to turn on DeterministicSourcePaths for the msbuild.exe. (Turn on ContinuousIntegrationBuild has no effects at all.)

Then the work around suggested above won't work anymore, as the Property value supplied at command line to msbuild.exe overrides csproj settings. In order to "remove" the test project from sourcelink process, I need to add this to the test csproj file:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" DefaultTargets="Build" TreatAsLocalProperty="DeterministicSourcePaths" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
...
  <DeterministicSourcePaths>false</DeterministicSourcePaths>
  </PropertyGroup>

@tmat
Copy link
Member Author

tmat commented Aug 13, 2018

Turn on ContinuousIntegrationBuild has no effects at all.

Does your project set Deterministic property to true?

in order to "remove" the test project from sourcelink process

Why do you need to remove it?

@xied75
Copy link

xied75 commented Aug 15, 2018

@tmat

if you mean /Deterministic to the Roslyn, I didn't set it.

Why do you need to remove it?

Because I don't want to install SourceLink nuget package into my testing project, and then the compile would error out.

@tmat
Copy link
Member Author

tmat commented Aug 15, 2018

Since you do not import Microsoft.Net.Sdk and do not set Deterministic then ContinuousIntegrationBuild has no effect since only affects deterministic builds.

the compile would error out.

What error does it report?

@xied75
Copy link

xied75 commented Aug 15, 2018

(InitializeSourceRootMappedPaths target) -> 
  C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\MSBuild\15.0\bin\Roslyn\Microsoft.Managed.Core.targets(90,5): error : SourceRoot items must include at least one top-level (
not nested) item when DeterministicSourcePaths is true [C:\github\xxx\src\Shared.Utilities\tests\Shared.Utilities.Test.csproj]

    0 Warning(s)
    1 Error(s)

msbuild on the src csproj alone would work, if msbuild the solution file, which including the test project, would give above error.

@tmat
Copy link
Member Author

tmat commented Aug 15, 2018

Right, so that's because you don't have SourceLink and you switch DeterministicSourcePaths to true. I guess I do not understand why you don't want SourceLink in your testing project.

@xied75
Copy link

xied75 commented Aug 16, 2018

@tmat The reason to use SourceLink is because I'm nuget packaging my library code into nupkg, but I'm not packaging my tests, at least in my case, test project was run to verify the library project then the lib nupkg is produced on Jenkins.

@tmat tmat added the external label Oct 18, 2018
@tmat tmat closed this as completed Oct 18, 2018
Kuinox pushed a commit to CK-Build/CKli that referenced this issue Oct 30, 2019
0xced added a commit to 0xced/Six.BankMaster that referenced this issue Nov 8, 2020
This should solve this issue popping on AppVeyor:
> SourceRoot items must include at least one top-level (not nested) item when DeterministicSourcePaths is true

See explanation in dotnet/sourcelink#91 (comment)
poke added a commit to poke/Westerhoff.Configuration.Encryption that referenced this issue Nov 18, 2020
Source link is currently broken for WPF as per dotnet/sourcelink#91.
@shuebner
Copy link

shuebner commented Oct 29, 2021

For lack of a better place than this long-closed issue:
What helped me was not to just disable DeterministicSourcePaths for the WPF project, but to set a dummy SourceRoot for the MapSourceRoot task to find, like this, which can be put in a Directory.Build.targets to apply to all WPF projects of the repo:

<Project>
  <Target Name="WpfSourceLinkWorkaround" BeforeTargets="InitializeSourceRootMappedPaths" Condition="'$(UseWPF)' == 'true' And $(DeterministicSourcePaths)">
    <!-- WPF causes an error with SourceLink because its build targets create a temporary project without a PackageReference to SourceLink, see https://github.com/dotnet/sourcelink/issues/91,
         causing the @SourceRoot property to be unexpectedly empty for the MapSourceRoot task
        
         For context, see https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets
         and https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/MSBuildTask/MapSourceRoots.cs
         
         This workaround sets the SourceRoot manually to some deterministic value to keep the promise given by having DeterministicSourcePaths set to true -->
    <Message Text="using deterministic source path workaround for WPF project instead of SourceLink" />
    <ItemGroup>
      <!-- There needs to be at least one SourceRoot defined, its value does not seem to matter as long as it ends with a directory separator -->
      <SourceRoot Include="\" />
    </ItemGroup>
  </Target>
</Project>

@Dunge
Copy link

Dunge commented Oct 23, 2023

The workaround above by @shuebner stopped working in VS 17.8.0 Preview 4.0:

Directory.Build.props(28,92): error MSB4100: Expected "$(DeterministicSourcePaths)" to evaluate to a boolean instead of "", in condition "'$(UseWPF)' == 'true' And $(DeterministicSourcePaths)".

I belive it can be fixed by changing the condition to
Condition="'$(UseWPF)' == 'true' And $(DeterministicSourcePaths) != ''"
anyone can confirm this is okay?

@tmat
Copy link
Member Author

tmat commented Oct 23, 2023

@Dunge looks good to me. Does the issue (without workaround) reproduce when using .NET 8 SDK?

@Dunge
Copy link

Dunge commented Oct 23, 2023

@tmat you are right, I do not remember the error that forced me to add this workaround a few years ago, but it doesn't seem necessary anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

5 participants