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

Add SourceLink to NuGet packages #8464

Merged
merged 2 commits into from
Sep 9, 2022
Merged

Conversation

MiYanni
Copy link
Member

@MiYanni MiYanni commented Sep 8, 2022

Fixes: #5619

Adds the SourceLink functionality to our NuGet packages. It also makes our build marked with the deterministic flag.

Verification

I used the NuGet Package Explorer to verify the packages. When they started, they looked like this:
image

I enabled SourceLink but it was still showing issues. Seems I needed the EmbedUntrackedSources setting.
image

After adding EmbedUntrackedSources, the build was broken. After some investigation, I found an issue that was reported for the problem, but no workaround: dotnet/sourcelink#492
I looked into how the generated .g.cs files wanted to reference the .xaml. Then, I determined how to output them to that path. I added those targets to the affected projects. After that, we had SourceLink working.
image

I noticed the Deterministic flag was still red. Found that I needed to add ContinuousIntegrationBuild for it to add the /deterministic compiler flag. After that, everything was green.
image

Microsoft Reviewers: Open in CodeFlow

@MiYanni MiYanni requested a review from a team as a code owner September 8, 2022 01:31
Directory.Build.props Outdated Show resolved Hide resolved
Copy link
Member

@drewnoakes drewnoakes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks similar to what I've had to do to get this working in my own libraries (minus the XAML file bit). Thanks for the clear PR description too.

In one of my libs I also added these. Do you think they'd be useful here?

<Deterministic>true</Deterministic>
<IncludeSource>true</IncludeSource>

Also, do we specify PackageProjectUrl, PackageLicenseExpression, RepositoryType and RepositoryUrl anywhere?

Directory.Build.props Outdated Show resolved Hide resolved
@KirillOsenkov
Copy link
Member

I think Deterministic property is already true by default in SDK-style projects.

@MiYanni
Copy link
Member Author

MiYanni commented Sep 8, 2022

This looks similar to what I've had to do to get this working in my own libraries (minus the XAML file bit). Thanks for the clear PR description too.

In one of my libs I also added these. Do you think they'd be useful here?

<Deterministic>true</Deterministic>
<IncludeSource>true</IncludeSource>

Also, do we specify PackageProjectUrl, PackageLicenseExpression, RepositoryType and RepositoryUrl anywhere?

I'll look into what Deterministic and IncludeSource actually do. My assumption is ContinuousIntegrationBuild sets Deterministic to true.

Package properties are here:

<PackageLicenseUrl>http://go.microsoft.com/fwlink/?LinkId=529443</PackageLicenseUrl>
<RepositoryUrl>https://github.com/dotnet/project-system</RepositoryUrl>
<PackageProjectUrl>$(RepositoryUrl)</PackageProjectUrl>

We use PackageLicenseUrl instead of PackageLicenseExpression, and we don't seem to have RepositoryType set. I can add that.

@MiYanni
Copy link
Member Author

MiYanni commented Sep 8, 2022

I think Deterministic property is already true by default in SDK-style projects.

According to what NuGet Package Explorer was showing, the assemblies were not deterministic until I set ContinuousIntegrationBuild to true. See the description for this PR.

@KirillOsenkov
Copy link
Member

Yes, NuGet Package Explorer's definition of deterministic includes the deterministic compilation as well as normalized paths to source files in the Pdb. The Deterministic property only controls the compiler determinism, whereas ContinuousIntegrationBuild also turns on normalized Pdb paths (which may break local debugging scenarios as I've outlined above). I think ideally set ContinuousIntegrationBuild to true on CI only.

@MiYanni
Copy link
Member Author

MiYanni commented Sep 8, 2022

@drewnoakes I'm adding the Deterministic property but not the IncludeSource property. It seems to not function with the SymbolPackageFormat I'm adding. See: NuGet/Home#8589

@MiYanni MiYanni merged commit 18edf87 into dotnet:main Sep 9, 2022
@ghost ghost added this to the 17.4 milestone Sep 9, 2022
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.

Add SourceLink
4 participants