Improve Project Files by mattico · Pull Request #1937 · oxyplot/oxyplot

mattico · 2022-09-18T19:25:52Z

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:

As a follow-up to Add SourceLink support and nuget symbol packages #1930, adds Directory.Build.props to Solution Items folders
Add EnableWindowsTargeting to enable building .NET Framework libs from Linux and Mac
Factor out common project properties to Directory.Build.props now that we have one of those
A drive-by fix to some SkiaSharp deprecated functions
Update supported TargetFrameworks to remove EoL versions and default to .NET 6.0 where possible.

@oxyplot/admins

VisualMelon · 2022-09-18T19:38:52Z

This looks great, thanks for putting this together. I'll try to find time to look it over properly tomorrow.

mattico · 2022-09-18T21:12:03Z

Source/Directory.Build.targets

+    <PackageProjectUrl>https://oxyplot.github.io/</PackageProjectUrl>
+    <PackageIcon>OxyPlot_128.png</PackageIcon>
+  </PropertyGroup>
+</Project>


The Build.targets file is executed towards the end of the build process, so we can depend on variables set in the individual projects. In here we have common properties for our published packages.

Typically properties and Items are added to the Directory.Build.props file. Directory.Build.targets is used for custom targets. But in general, it doesn't matter.

You can also add a README for all libraries:

<PropertyGroup Label="Nuget"> <PackageReadmeFile>README.md</PackageReadmeFile> </PropertyGroup> <ItemGroup Label="Nuget"> <None Include="$(MSBuildThisFileDirectory)../../README.md" Pack="true" PackagePath="\" Visible="false" /> </ItemGroup>

I also recommend using $(MSBuildThisFileDirectory) wherever possible instead of $(SolutionDir). $(SolutionDir) is not set when running dotnet build on a specific .csproj file

mattico · 2022-09-18T21:13:16Z

@VisualMelon no rush! Thanks for taking a look.

VisualMelon · 2022-09-19T07:08:30Z

I think we should include a comment somewhere prominent that we now require .NET 5 or .NET 6 to build the libraries, though they still target .NET Core 3.1 and need it for testing: that could create some real confusion for people trying to get it up and running the first time. To keep thing simple, may make sense to add net6.0 targets to everything now as @HavenDV suggested (not necessarily in this PR) so that basic dev tasks can all be performed with .NET 6.

Jonarw

We should consider upgrading things like example browser to .NET 6 anyway, as .NET 3.1 runs out of support at the end of this year anyway (if I remember correctly). But this does not need to be part of this PR.

mattico · 2022-09-20T17:31:55Z

Source/Directory.Build.props

@@ -4,4 +4,10 @@
    <PackageReference Include="DotNet.ReproducibleBuilds" Version="1.1.1" PrivateAssets="All"/>
    <PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.3" PrivateAssets="All" />


@Jonarw from what I understand the ReferenceAssemblies included in this NuGet package mean that we don't need to have any targeting packs installed.

Certainly seems we don't need them to build the assemblies, but I think we do need to keep the example and tests projects current as they determine the runtime actually used by devs and the CI (and for the framework runtimes you need a targeting pack, I think)

Hmm, that didn't seem to be enough in my case. I was unable to build before installing the targeting pack manually. Of course now that I have installed it I can't reproduce the issue anymore.
In any case, I think upgrading to .NET 4.6.2 makes sense anyway, as the older versions have been out of support since April 2022.

mattico · 2022-11-06T19:19:51Z

Okay, I updated all the targetframeworks so we're only shipping for supported targets: .NET Framework 4.6.2, .NET Standard 2.0, and .NET 6.0. I had to fix a few nullable warnings to work on .NET 6.0.

While running the tests I changed some of the tests to use TestCaseSource so the cases can run in parallel and so you can see which test cases are failing and debug them individually.

mattico · 2022-11-06T19:23:20Z

Source/OxyPlot.Wpf.Tests/ExampleLibraryTests.cs

+        {
+            Directory.CreateDirectory(BaselineDirectory);
+            Directory.CreateDirectory(DestinationDirectory);
+            Directory.CreateDirectory(DiffDirectory);


Aside: should this delete the existing dirs? Some of the examples use random data so they error if you run the tests multiple times without clearing out the baseline dir. Could also seed the rng in tests but that's not exposed from the examples currently.

Baseline should be left alone; the others could be deleted, but it's not a problem I don't think.

Jonarw · 2023-07-11T08:52:07Z

Hi @mattico!
First of all, sorry that this PR has been open for so long. We would like to get this merged finally! Would you be interested in rebasing / sorting out the merge conflicts that have accumulated? Alternatively, would it be alright with you if I took care of that?
Thanks for this contribution in any case!

so it's more visible in Visual Studio.

…Linux and Mac

as it's included by DotNet.ReproducibleBuilds

.NET Framework versions updated to 4.6.2 as 4.5.2 is EOL. .NET Standard versions before 2.0 are no longer supported as .NET Framework 4.6.2 supports .NET Standard 2.0. Examples moved to .NET 6.0. .NET Core 3.1 support removed as it is EOL on December 13, 2022. Tests changed to run on .NET Framework and .NET 6.0. Project SDKs updated to new Microsoft.NET.Sdk. Fix nullable errors from updating to .NET 6.0.

The underlying library is .NET Framework only, and it's deprecated. It has a bug on .NET 6 related to code pages.

Hopefully this has .NET 6.0

May as well, since we need to build ExampleLibrary for net462 for the tests anyway.

mattico · 2023-07-11T17:14:37Z

@Jonarw no worries!

Rebased. The merge conflicts were only from the changelog. Feel free to update the PR in the future if necessary.

VisualMelon

Looks like the target framework lines may have gone wrong in the rebase

(Thanks for coming back to this!)

Source/Examples/ExampleLibrary/ExampleLibrary.csproj

Source/Examples/PerformanceTest/PerformanceTest.csproj

Source/OxyPlot.ImageSharp/OxyPlot.ImageSharp.csproj

Source/OxyPlot.Pdf.Tests/OxyPlot.Pdf.Tests.csproj

Source/OxyPlot.Pdf/OxyPlot.Pdf.csproj

Source/OxyPlot.SkiaSharp/OxyPlot.SkiaSharp.csproj

Source/OxyPlot.WindowsForms/OxyPlot.WindowsForms.csproj

Source/OxyPlot.Wpf/OxyPlot.Wpf.csproj

Source/OxyPlot/Imaging/Png/PngDecoder.cs

Source/OxyPlot/OxyPlot.csproj

Source/OxyPlot.ImageSharp/OxyPlot.ImageSharp.csproj

- Update github actions versions - Add .NET 7.0 targets - Don't overwrite NoWarn

HavenDV · 2023-07-13T02:15:34Z

Thank you very much for the quick changes.

VisualMelon · 2023-07-13T05:35:08Z

Source/OxyPlot/OxyPlot.csproj

        <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
        <LangVersion>8</LangVersion>
-        <NoWarn>618</NoWarn>
+        <NoWarn>$(NoWarn);CS0618;CS8767</NoWarn>


Would prefer we disable CS8767 only in the one place it lights up (CompareHelper) with #pragma warning disable/restore CS8767. Without changing to C#9 (which I don't think is supported on net framework) I can't find a way to make the code happy.

Without changing to C#9 (which I don't think is supported on net framework) I can't find a way to make the code happy.

.Net Framework supports C# 9. You can also always use PolySharp - https://github.com/Sergio0694/PolySharp

@HavenDV do you have a reference for C#9 support in net framework? I searched but could only find comments about net 5.0 and above, which I took to mean not net framework, but keen to hear otherwise.

I have many projects that include net451 or net462 TargetFramework and for which I install the latest version of the language. Everything works correctly. But, for example, to support records, you need to add special classes to the project (or use PolySharp, which will do the same)

This states it is not supported (in 4.8 at least), but I still can't find a clear definitive reference dotnet/docs#26587 (comment)

I'm tempted to suggest we remove the ComparisonHelper API and rewrite the only usage thereof (which is confusing and depends on implementation details in List<T>.BinarySearch). For the time being, it's probably simplest just to disable the warning locally, which won't create a burden anywhere else.

Quoting the MSDN

Choosing a language version newer than the default can cause hard to diagnose compile-time and runtime errors.

Certainly OxyPlot isn't doing anything exciting run-time wise, and I'm not aware of any situations where it actually goes wrong, but I'd still be uneasy using something that isn't officially supported.

With our LangVersion at 8 we are actually already in unsupported territory. I suspect MS writes this as they want a) people to move away from .NET Framework and b) don't want to be held responsible for any misuses.

But I agree, let's keep it simple for now, the LangVersion might be a discussion for another day. Let's disable the warning locally and keep everything else as is.

The question for me regarding bumping the language version is really whether it can create silent bugs. Continuing the search, I can't find any evidence that this has occurred, and most discussion is just concerning using newer features.

All that said, it looks like C# 8 isn't strictly supported either, so I should probably stop worrying.

(Sorry for the confusing messages: I've managed to miss both of @Jonarw 's message before sending mine somehow)

VisualMelon · 2023-07-13T16:05:39Z

All looks good to me!

VisualMelon · 2023-07-13T16:16:48Z

@HavenDV anything else from your side? (And thank you for chiming in)

HavenDV · 2023-07-13T18:58:06Z

    <PropertyGroup Label="Analyzers">
        <EnableNETAnalyzers>true</EnableNETAnalyzers>
        <AnalysisLevel>latest</AnalysisLevel>
        <AnalysisMode>All</AnalysisMode>
    </PropertyGroup>

I usually add this to Directory.Build.props.
But in fact, this is likely to generate a lot of warnings and is suitable for a single PR or a series of PRs.

VisualMelon · 2023-07-13T19:23:25Z

@Jonarw I'm happy for this to be merged if you are

Jonarw · 2023-07-14T05:55:59Z

Thanks @mattico for this PR (and for your patience)!

mattico commented Sep 18, 2022

View reviewed changes

mattico changed the title ~~Improve ci~~ Improve Project Files Sep 18, 2022

VisualMelon approved these changes Sep 19, 2022

View reviewed changes

Jonarw approved these changes Sep 19, 2022

View reviewed changes

VisualMelon mentioned this pull request Sep 19, 2022

Oxyplot doesn't build on fresh VS2022 install (net452 targeting pack can no longer be installed by VS installer) #1938

Closed

mattico commented Sep 20, 2022

View reviewed changes

mattico commented Nov 6, 2022

View reviewed changes

mattico added 14 commits July 11, 2023 12:04

Add Directory.Build.props to Solution Items folders

b753ab4

so it's more visible in Visual Studio.

Add EnableWindowsTargeting to enable building .NET Framework libs on …

f309a12

…Linux and Mac

Pull out common project properties to Directory.Build.props

3972509

OxyPlot.SkiaSharp: fix use of deprecated functions

6376725

Update Changelog

b3ac8b8

Remove ReferenceAssemblies nuget

f3d8150

as it's included by DotNet.ReproducibleBuilds

Update GitHub Actions .NET versions

b895776

Remove .NET 6 builds for OxyPlot.Pdf

64657b4

The underlying library is .NET Framework only, and it's deprecated. It has a bug on .NET 6 related to code pages.

Fix test discovery on .NET 6.0

79db4d6

Change some tests to use TestCaseSource

508b311

Update changelog

29cbce1

Update AppVeyor image version

2bc2d7a

Hopefully this has .NET 6.0

Enable net462 for example projects

12b1b64

May as well, since we need to build ExampleLibrary for net462 for the tests anyway.

mattico force-pushed the improve-ci branch from 2e1279e to 12b1b64 Compare July 11, 2023 17:13

VisualMelon requested changes Jul 11, 2023

View reviewed changes

mattico force-pushed the improve-ci branch from a366577 to 30028c6 Compare July 11, 2023 19:09

HavenDV reviewed Jul 13, 2023

View reviewed changes

Source/OxyPlot.ImageSharp/OxyPlot.ImageSharp.csproj Outdated Show resolved Hide resolved

Address review feedback

7b3d559

- Update github actions versions - Add .NET 7.0 targets - Don't overwrite NoWarn

mattico force-pushed the improve-ci branch from 84d11fd to 7b3d559 Compare July 13, 2023 01:59

VisualMelon reviewed Jul 13, 2023

View reviewed changes

Improve Directory.Build.targets

f5107d4

mattico force-pushed the improve-ci branch from f317ae2 to f5107d4 Compare July 13, 2023 15:37

Move nullability error suppression to use site

720a009

VisualMelon approved these changes Jul 13, 2023

View reviewed changes

Jonarw merged commit f1b3d44 into oxyplot:develop Jul 14, 2023

Jonarw mentioned this pull request Jul 14, 2023

Update SkiaSharp to v2.88.3 #2013

Merged

4 tasks

mattico deleted the improve-ci branch July 14, 2023 15:34

This was referenced Jan 14, 2026

Bump the tests group with 4 updates VahidN/PdfReport.Core#116

Closed

Bump the tests group with 5 updates VahidN/PdfReport.Core#117

Closed

dependabot bot mentioned this pull request Feb 4, 2026

Bump the tests group with 7 updates VahidN/PdfReport.Core#118

Open

		@@ -4,4 +4,10 @@
		<PackageReference Include="DotNet.ReproducibleBuilds" Version="1.1.1" PrivateAssets="All"/>
		<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.3" PrivateAssets="All" />

Conversation

mattico commented Sep 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Changes proposed in this pull request:

Uh oh!

VisualMelon commented Sep 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HavenDV Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattico commented Sep 18, 2022

Uh oh!

VisualMelon commented Sep 19, 2022

Uh oh!

Jonarw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VisualMelon Sep 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattico commented Nov 6, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jonarw commented Jul 11, 2023

Uh oh!

mattico commented Jul 11, 2023

Uh oh!

VisualMelon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HavenDV commented Jul 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VisualMelon Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VisualMelon Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattico commented Sep 18, 2022 •

edited

Loading

HavenDV Jul 13, 2023 •

edited

Loading

VisualMelon Sep 21, 2022 •

edited

Loading

VisualMelon left a comment •

edited

Loading

VisualMelon Jul 13, 2023 •

edited

Loading

VisualMelon Jul 13, 2023 •

edited

Loading