search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.12k stars 4.04k forks source link

Some SourceLink links point to generated files #52837

Open mmitche opened 3 years ago

mmitche commented 3 years ago

The package Microsoft.CodeAnalysis.EditorFeatures.Wpf version 3.10.0-3.21216.10 has generated files in its source links. These files don't exist on GitHub and so the source link info is broken.

Validating  Microsoft.CodeAnalysis.EditorFeatures.Wpf.3.10.0-3.21216.10.symbols.nupkg ...The remote server returned an error: (404) Not Found.

    File lib/net472/Microsoft.CodeAnalysis.EditorFeatures.Wpf.pdb has broken links:
        Failed to retrieve https://raw.githubusercontent.com/dotnet/roslyn/9bfe989ad46e3f457d33ed800c3be012cec5afc6/artifacts/obj/Microsoft.CodeAnalysis.EditorFeatures.Wpf/Release/net472/InlineRename/Dashboard/Dashboard.g.cs
The remote server returned an error: (404) Not Found.
        Failed to retrieve https://raw.githubusercontent.com/dotnet/roslyn/9bfe989ad46e3f457d33ed800c3be012cec5afc6/artifacts/obj/Microsoft.CodeAnalysis.EditorFeatures.Wpf/Release/net472/.NETFramework
The remote server returned an error: (404) Not Found.
        Failed to retrieve https://raw.githubusercontent.com/dotnet/roslyn/9bfe989ad46e3f457d33ed800c3be012cec5afc6/artifacts/obj/Microsoft.CodeAnalysis.EditorFeatures.Wpf/Release/net472/Microsoft.CodeAnalysis.EditorFeatures.Wpf.InternalsVisibleTo.cs
The remote server returned an error: (404) Not Found.
        Failed to retrieve https://raw.githubusercontent.com/dotnet/roslyn/9bfe989ad46e3f457d33ed800c3be012cec5afc6/artifacts/obj/Microsoft.CodeAnalysis.EditorFeatures.Wpf/Release/net472/Microsoft.CodeAnalysis.Editor.EditorFeaturesWpfResources.cs
The remote server returned an error: (404) Not Found.
        Failed to retrieve https://raw.githubusercontent.com/dotnet/roslyn/9bfe989ad46e3f457d33ed800c3be012cec5afc6/artifacts/obj/Microsoft.CodeAnalysis.EditorFeatures.Wpf/Release/net472/Microsoft.CodeAnalysis.EditorFeatures.Wpf.AssemblyInfo.cs
D:\workspace\_work\1\a\signed\shipping\assets\symbols\Microsoft.CodeAnalysis.EditorFeatures.Wpf.3.10.0-3.21216.10.symbols.nupkg has broken SourceLink links.
##[error](NETCORE_ENGINEERING_TELEMETRY=SourceLink) D:\workspace\_work\1\a\signed\shipping\assets\symbols\Microsoft.CodeAnalysis.EditorFeatures.Wpf.3.10.0-3.21216.10.symbols.nupkg has broken SourceLink links
jasonmalinowski commented 3 years ago

@tmat @dotnet/roslyn-infrastructure How is this supposed to work? Is some magic supposed to embed sources for generated files?

jaredpar commented 3 years ago

@clairernovotny

To my understanding that shouldn't be happening because the generated files should be embedded in the PDB here and hence excluded from source link.

tmat commented 3 years ago

The sources are not embedded into the PDB for some reason. Isn't there a workaround for WPF projects somewhere in our targets that disables source embedding because of WPF bug?

jasonmalinowski commented 3 years ago

@tmat You would have been the one I'd ask that question to. What would it look like, what's the property name?

tmat commented 3 years ago

Trace EmbeddedFiles item group passed to the compiler.

jasonmalinowski commented 3 years ago

So I see no use of EmbeddedFiles in any of the MSBuild anywhere in Roslyn, other than of course the stuff in src/Compilers/Core/MSBuildTask which just the stuff we ship to customers. So this bug is elsewhere -- where to move to next?

jaredpar commented 3 years ago

@jasonmalinowski did you look at a binlog from an official or CI build? IIRC we only leverage source link in those situations and I don't think it would show up in a local build.

jasonmalinowski commented 3 years ago

@jaredpar I just did "git grep" on our repo and didn't see anything that way. I presume if there is a hack it's from arcade or the SDK or something external, at which point an actual expert in that will need to figure this out. I'm not even sure why this bug is assigned to me. :smile:

clairernovotny commented 3 years ago

Have you tried setting IncludePackageReferencesDuringMarkupCompilation to true? That was added in 5.0.200 to enable PackageReferences, including Source Link, to work with the WPF markup compiler pass. It's currently off by default in the SDK.

jasonmalinowski commented 2 years ago

Sending this back through triage -- I think this was assigned to me thinking it was related to source generators.

davidwengier commented 2 years ago

Confirmed this is still an issue with the latest packages

davidwengier commented 2 years ago

... but of course when building locally the sources are embedded as expected. Joy.

davidwengier commented 2 years ago

Confirmed this is still an issue with the latest packages

Actually I take that back, I downloaded a random recent build and it looks like the files are embedded in the PDB, they're just also part of the source link map. I know for Go To Definition i'm checking for embedded source, before source link. Is that the expectation @tmat? Or is it an issue that the files are part of the map (ie, they have the same prefix as the other source)?

@mmitche Does the validation that produced the log above look for embedded source, or does it just validate the source link map directly?

RikkiGibson commented 2 years ago

Shouldn't the binlog from the official build tell us if we have these settings right? Should we adjust that job to publish the binlog on a success?

davidwengier commented 2 years ago

Indeed, the binlog from the random build I linked above shows us embedding the source of these files correctly:

image