Add net5.0 targetframework

[stefannikolei]

#1432

[antonfirsov]

We had several discussions about this with @JimBobSquarePants, and if I'm not mistaken the conclusion was that net5.0 should be a test-only target.
The reason is that we don't expect to add 5.0-specific code to the library anytime soon, therefore it's not worth to pay the costs of an additional target for now (larger package, longer local builds, complexity etc.)

Nevertheless, it's very important to make our CI run tests against net5.0 so we can ensure that there are no regressions in the library on that platform. @stefannikolei any chance you can turn this into a test/CI-only change?

[stefannikolei]

@antonfirsov Sure i can make that change. The pr in shared infrastructure needs to be merged too. Because I got only errors when I added net50 as target

[stefannikolei]

Build is not working without the .net50 target framework

/usr/share/dotnet/sdk/5.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(241,5): error NETSDK1005: Assets file '/home/runner/work/ImageSharp/ImageSharp/artifacts/obj/src/ImageSharp/project.assets.json' doesn't have a target for 'net5.0'. Ensure that restore has run and that you have included 'net5.0' in the TargetFrameworks for your project. [/home/runner/work/ImageSharp/ImageSharp/src/ImageSharp/ImageSharp.csproj]

[brianpopow]

Build is not working without the .net50 target framework

I am able to build it as it is right now (without the .net50 target framework in the ImageSharp.csproj). Im using .Net SDK version 5.0.101. It also works on linux (with docker image mcr.microsoft.com/dotnet/sdk:5.0.101).

Not sure why it does not work for @stefannikolei and the CI. @stefannikolei: which .Net SDK version are you using?

[stefannikolei]

On my Mac and rider I do not see that problem. Gonna look into the build script what they call there.

[stefannikolei]

Ok i now know where the problem is.
In The build script we explicitly call the build with .net5 this is the call which fails.

[brianpopow]

Ok i now know where the problem is.
In The build script we explicitly call the build with .net5 this is the call which fails.

The build works now, but the test execution fails on mac-os in the CI. I do not have a mac to test this. @stefannikolei can you run ./ci-test.ps1 "macos-latest" "net5.0" "-x64" "false" and see if it works for you?

[stefannikolei]

@brianpopow it is a vicious circle. It can not run because the .net5.0 dll was not build ...

If i remove the explicit Targetframework in ci-build.ps1 it builds all frameworks and the tests execute

[JimBobSquarePants]

Wrap separate TargetFrameworks in the csproj files in a condition based on a custom environmental variable, one with NET5.0 one without. Then we can target the framework only when we need to during CI testing.

[stefannikolei]

@JimBobSquarePants I added the env variable. But now i am hitting the problem why i opened SixLabors/SharedInfrastructure#15 on SharedInfrastructure...

[JimBobSquarePants]

See the comment regarding the targetframework options.

[codecov]

Codecov Report

Merging #1519 (78d23c5) into master (954d233) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1519   +/-   ##
=======================================
  Coverage   83.50%   83.50%           
=======================================
  Files         742      742           
  Lines       32801    32801           
  Branches     3671     3671           
=======================================
  Hits        27392    27392           
  Misses       4691     4691           
  Partials      718      718           

Flag	Coverage Δ
unittests	83.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 954d233...78d23c5. Read the comment docs.

[JimBobSquarePants]

Oooh @stefannikolei I approved this but just realized there's an issue.

We only want NET5 on the build/test part, not on publish so we should be using a custom environmental property as the filter instead. Use SIXLABORS_TESTING both the Build and Test steps and as the MsBuild filter.

We're explicitly setting the CI variable in the Test step but we shouldn't be since it's a default for all steps.
https://docs.github.com/en/actions/reference/environment-variables#default-environment-variables

[stefannikolei]

SIXLABORS_TESTING

So just change CI with SIXLABORS_TESTING?

[JimBobSquarePants]

So just change CI with SIXLABORS_TESTING?

Yep but you need to set it in both steps not just Test

ImageSharp/.github/workflows/build-and-test.yml

Lines 67 to 76 in 954d233

	- name: Build
	shell: pwsh
	run: ./ci-build.ps1 "${{matrix.options.framework}}"
	- name: Test
	shell: pwsh
	run: ./ci-test.ps1 "${{matrix.options.os}}" "${{matrix.options.framework}}" "${{matrix.options.runtime}}" "${{matrix.options.codecov}}"
	env:
	CI: True
	XUNIT_PATH: .\tests\ImageSharp.Tests # Required for xunit

[stefannikolei]

@JimBobSquarePants Want me to change TestEnvironment.cs to check for SIXLABORS_TESTING rather than CI, or should i add the CI back to the build script...

Hmm test failure seems unrelated, or am i mssing something?

[JimBobSquarePants]

The failure is unrelated. It happens every so often when there’s heavy traffic in GitHub Actions. The CI environmental variable is always true here. GitHub set it. Check the link I shared for a list of defaults.