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 support and nuget symbol packages #1930

Merged
merged 2 commits into from
Sep 18, 2022
Merged

Add SourceLink support and nuget symbol packages #1930

merged 2 commits into from
Sep 18, 2022

Conversation

mattico
Copy link
Contributor

@mattico mattico commented Sep 8, 2022

Fixes #1156.

Checklist

  • I have included examples or tests
  • I have updated the change log
  • I am listed in the CONTRIBUTORS file
  • I have cleaned up the commit history (use rebase and squash)

Changes proposed in this pull request:

  • The first commit adds SourceLink support and enables building nuget symbol packages for all published projects.
  • The second commit changes the AppVeyor build to push the nuget symbol packages to MyGet. I don't have a way to test this but I read the relevant documentation closely.

@oxyplot/admins

@HavenDV
Copy link
Contributor

HavenDV commented Sep 13, 2022

I think it's worth taking one step further and going directly to https://github.com/dotnet/reproducible-builds

@VisualMelon
Copy link
Contributor

I certainly wouldn't object; can we merge this now and do that later, or does it make sense to do it at the same time?

@HavenDV
Copy link
Contributor

HavenDV commented Sep 13, 2022

I certainly wouldn't object; can we merge this now and do that later, or does it make sense to do it at the same time?

This is a simplification and improvement of the ideas of the current PR. This will also add a deterministic build. It combines all the best solutions from these articles:
https://devblogs.microsoft.com/dotnet/producing-packages-with-source-link/
dotnet/sdk#2679

@HavenDV
Copy link
Contributor

HavenDV commented Sep 13, 2022

Also, VS is not good with symbols from snupkg files, so DebugType = embedded is much better for libraries and helps users debug problems:
dotnet/sdk#1458

This enables SourceLink and debug symbol embedding, among other things.
https://github.com/dotnet/reproducible-builds
This prevents the build from relying on outside libraries not described
in the repo.
@mattico
Copy link
Contributor Author

mattico commented Sep 17, 2022

I pushed new commits that use DotNet.ReproducibleBuilds and a Directory.Build.props. That's a very clean solution!

The last commit also adds DotNet.ReproducibleBuilds.Isolated, which ensures that the build doesn't depend on any outside libraries. So it adds a nuget package dependency for the .NET Framework reference assemblies since we no longer depend on the versions installed on the CI machines. We should be able to generate identical nuget packages on CI and developer machines with ContinuousIntegrationBuild enabled, in theory.

@VisualMelon
Copy link
Contributor

VisualMelon commented Sep 18, 2022

Sounds like it might help with CI configuration as well then. As an aside, with EnableWindowsTargeting=true I seem to be able to build everything from linux with .NET 6, which I can't recall being able to do the last time I tried a few years ago.

Directory.Build.props is mildly terrifying, but certainly keeps this tidy... might be worth in future adding a couple of well-named prop files and explicitly reference it from each project, moving common properties into it (e.g. licence info, icons, this stuff).

This seems to have taken effect within VS, but there is no indication of this in VS, which is annoying (that said, I don't recall it showing up other prop imports either).

NugetPackageExplorer reckons the CI build is OK:

image

@VisualMelon VisualMelon merged commit f682802 into oxyplot:develop Sep 18, 2022
@VisualMelon
Copy link
Contributor

VisualMelon commented Sep 18, 2022

@mattico @HavenDV thanks both for this.

Do we want a PR to add ContinuousIntegrationBuild to the CI configs?

What does identical mean exactly? Same filehash? (doesn't seem to be the case)


Of course, github actions is now complaining... https://github.com/oxyplot/oxyplot/actions/runs/3076201170

@HavenDV
Copy link
Contributor

HavenDV commented Sep 18, 2022

The last commit also adds DotNet.ReproducibleBuilds.Isolated, which ensures that the build doesn't depend on any outside libraries. So it adds a nuget package dependency for the .NET Framework reference assemblies since we no longer depend on the versions installed on the CI machines. We should be able to generate identical nuget packages on CI and developer machines with ContinuousIntegrationBuild enabled, in theory.

I haven't used DotNet.ReproducibleBuilds.Isolated so I can't say anything about it. But in general, the solution with Directory.Build looks good

Directory.Build.props is mildly terrifying, but certainly keeps this tidy... might be worth in future adding a couple of well-named prop files and explicitly reference it from each project, moving common properties into it (e.g. licence info, icons, this stuff).

Directory.Build.props is now a standard file for many repositories, these are just general properties for a specific directory and subdirectories. I usually add the entire NuGet configuration (unless each package requires a separate Description and Tags). To make it more explicit, I usually add it to the solution as a separate file at the same level where it is.
https://learn.microsoft.com/en-us/visualstudio/msbuild/customize-your-build?view=vs-2022#directorybuildprops-and-directorybuildtargets

Do we want a PR to add ContinuousIntegrationBuild to the CI configs?

No additional configuration is required, it's all done by the DotNet.ReproducibleBuilds package. It sets this property based on a CI-specific environment variable that is always there.

What does identical mean exactly? Same filehash? (doesn't seem to be the case)

Do you mean deterministic? it is described here
https://devblogs.microsoft.com/dotnet/producing-packages-with-source-link/#deterministic-builds

@VisualMelon
Copy link
Contributor

@HavenDV

Thanks for the additional information. I have to dash, will hopefully have time to look into the failing CI later today (mindlessly updating things hasn't done the job https://github.com/VisualMelon/oxyplot/actions/runs/3076290473/jobs/4970342386 )

@HavenDV
Copy link
Contributor

HavenDV commented Sep 18, 2022

I have to dash, will hopefully have time to look into the failing CI later today (mindlessly updating things hasn't done the job https://github.com/VisualMelon/oxyplot/actions/runs/3076290473/jobs/4970342386 )

You additionally need to change the TargetFramework from netcoreapp3.1 to net6.0 in those projects. Most likely, this will not require additional action.
image

@mattico mattico mentioned this pull request Sep 18, 2022
4 tasks
@HavenDV
Copy link
Contributor

HavenDV commented Nov 6, 2022

As I understand it, it is not necessary to specify Microsoft.NETFramework.ReferenceAssemblies, because this is done automatically by the DotNet.ReproducibleBuilds.Isolated package - https://github.com/dotnet/reproducible-builds/blob/main/src/DotNet.ReproducibleBuilds.Isolated/Sdk/Sdk.targets

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.

Source link for the portable projects
3 participants