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

Fix SourceLink failures #13

Merged
merged 5 commits into from
Apr 8, 2020
Merged

Fix SourceLink failures #13

merged 5 commits into from
Apr 8, 2020

Conversation

atifaziz
Copy link
Owner

@atifaziz atifaziz commented Apr 8, 2020

This PR is work-in-progress to should fix issue #12.

@atifaziz
Copy link
Owner Author

atifaziz commented Apr 8, 2020

As the sourcelink test shows for build of 829fdaf, the test fails:

1 Documents without URLs:
8edef9391a47680caec71f4ded6c435fd60d04067808138edf6d8fb16a81b307 sha256 csharp C:\Users\appveyor\AppData\Local\Temp\1\.NETStandard,Version=v1.3.AssemblyAttributes.cs
1 Documents with errors:
9b083e06cbf038077e3e53ac812c0712d87ef543286b503124af5c2ffc027fa3 sha256 csharp C:\projects\hazz\src\obj\Release\netstandard1.3\Fizzler.Systems.HtmlAgilityPack.AssemblyInfo.cs
https://raw.githubusercontent.com/atifaziz/Hazz/03015c704c9b4a3723531f721764e4befc18c0f3/src/obj/Release/netstandard1.3/Fizzler.Systems.HtmlAgilityPack.AssemblyInfo.cs
error: url failed NotFound: Not Found
sourcelink test failed
failed for lib/netstandard1.3/Fizzler.Systems.HtmlAgilityPack.pdb
1 Documents without URLs:
a6e03ae4df13fe05345e9022d1f1cd24ecae4bfd66db4843697c855d9f9335f4 sha256 csharp C:\Users\appveyor\AppData\Local\Temp\1\.NETStandard,Version=v2.0.AssemblyAttributes.cs
1 Documents with errors:
9b083e06cbf038077e3e53ac812c0712d87ef543286b503124af5c2ffc027fa3 sha256 csharp C:\projects\hazz\src\obj\Release\netstandard2.0\Fizzler.Systems.HtmlAgilityPack.AssemblyInfo.cs
https://raw.githubusercontent.com/atifaziz/Hazz/03015c704c9b4a3723531f721764e4befc18c0f3/src/obj/Release/netstandard2.0/Fizzler.Systems.HtmlAgilityPack.AssemblyInfo.cs
error: url failed NotFound: Not Found
sourcelink test failed
failed for lib/netstandard2.0/Fizzler.Systems.HtmlAgilityPack.pdb
2 files did not pass in C:\projects\hazz\dist\Fizzler.Systems.HtmlAgilityPack.1.2.1-ci-20200408t0858.symbols.nupkg

@atifaziz atifaziz marked this pull request as ready for review April 8, 2020 09:10
@atifaziz
Copy link
Owner Author

atifaziz commented Apr 8, 2020

Seems like we are now at the same stage as @daveaglick was with 1 document causing errors (instead of 2):

1 Documents with errors:
a6e03ae4df13fe05345e9022d1f1cd24ecae4bfd66db4843697c855d9f9335f4 sha256 csharp C:\projects\hazz\src\.AssemblyAttributes
https://raw.githubusercontent.com/atifaziz/Hazz/27bffca4ca4f580c63c86fd768cd74b926262c9e/src/.AssemblyAttributes
error: url failed NotFound: Not Found
sourcelink test failed

Looks as if $([System.IO.Path]::Combine('$(IntermediateOutputPath)', '$(TargetFrameworkMoniker).AssemblyAttributes$(DefaultLanguageSourceExtension)')) is evaluating to just .AssemblyAttributes since $(TargetFrameworkMoniker) and $(DefaultLanguageSourceExtension) are empty. 💭

…uild.targets"

Per following suggestion by @tmat

> Setting `TargetFrameworkMonikerAssemblyAttributesPath` needs to be
> done after importing SDK targets because `IntermediateOutputPath` is
> set in `Microsoft.Common.CurrentVersion.targets` and not before that.
>
> So I think the best would be to set it in `Directory.Build.targets`
> in the repo root.

Source: shiftkey/dotnetcore-sourcelink-test-bug#1 (comment)
@atifaziz
Copy link
Owner Author

atifaziz commented Apr 8, 2020

So applying the suggestion by @tmat to move TargetFrameworkMonikerAssemblyAttributesPath setting to Directory.Build.targets in 91f6566 seems to have worked but introduced a new failure:

1 Documents with errors:
8edef9391a47680caec71f4ded6c435fd60d04067808138edf6d8fb16a81b307 sha256 csharp C:\projects\hazz\src\obj\Release\netstandard1.3\.NETStandard,Version=v1.3.AssemblyAttributes.cs
https://raw.githubusercontent.com/atifaziz/Hazz/778cf36b6c5ab7f439e203ea109a56cead1cbc6d/src/obj/Release/netstandard1.3/.NETStandard,Version=v1.3.AssemblyAttributes.cs
error: url failed NotFound: Not Found
sourcelink test failed
failed for lib/netstandard1.3/Fizzler.Systems.HtmlAgilityPack.pdb

.NETStandard,Version=v1.3.AssemblyAttributes.cs looks right but because it's attempting to resolve https://raw.githubusercontent.com/atifaziz/Hazz/778cf36b6c5ab7f439e203ea109a56cead1cbc6d/src/obj/Release/netstandard1.3/.NETStandard,Version=v1.3.AssemblyAttributes.cs, it means it thinks it's a tracked source?

@atifaziz
Copy link
Owner Author

atifaziz commented Apr 8, 2020

No luck with applying suggestion (to EmbeddedFiles items to Directory.Build.targets) from @tmat in 574207c

MSBuild Structured Log Viewer (as suggested by @KirillOsenkov) shows that _SetEmbeddedFilesFromSourceControlManagerUntrackedFiles still appears before GenerateAssemblyInfo target:

image

Frustrating.

@KirillOsenkov
Copy link
Contributor

Basically two different generated files are involved here.

First one is Fizzler.Systems.HtmlAgilityPack.AssemblyInfo.cs, generated by the CoreGenerateAssemblyInfo target.

The second one is .NETStandard,Version=v2.0.AssemblyAttributes.cs, generated by the GenerateTargetFrameworkMonikerAttribute target.

Both files need to be mentioned in the EmbeddedFiles item.

KirillOsenkov added a commit to KirillOsenkov/DeterministicBuilds that referenced this pull request Apr 8, 2020
We were debugging source link with @atifaziz here: atifaziz/Hazz#13

We established that the `TargetFrameworkMonikerAssemblyAttributesPath` generated file also needs to be added to `@(EmbeddedFiles)`
@atifaziz
Copy link
Owner Author

atifaziz commented Apr 8, 2020

@KirillOsenkov Thanks for your explanation and help!

@clairernovotny
Copy link

All the fixes should be covered by the workaround here. dotnet/sourcelink#572

@atifaziz atifaziz merged commit 2351c3c into master Apr 8, 2020
@atifaziz atifaziz deleted the fix-sourcelink branch April 8, 2020 19:29
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.

3 participants