Thanks @agocke!

To get the build green with these changes, the worker image in AppVeyor.yml will need to be updated to VS2022 (IIRC).

dev now includes a .NET 6 target so fingers crossed this will all come together 🤞

Thanks! Looks like there have been a few changes between when I originally wrote this and now, so it needs a little fixup. I'll move to draft until I can address the last remaining warnings.

@agocke

i pulled the ITuple parts of this into the main line.

would you mind rebasing

Could you also add a sample app that consumes serilog and uses trimming. So we can smoke test the changes

@SimonCropp All updated. Happy to add a sample. Where would you like it? FYI, there should be some correctness assurance via the PublishTrimmed property. That enables the analyzer in .NET 6+, which checks that there aren't any trim warnings. It's much more accurate in .NET 7, so I would recommend building with a .NET 7 SDK version once that releases.

Great! It looks like there are still some problems to resolve -

C:\projects\serilog\src\Serilog\Settings\KeyValuePairs\SettingValueConversions.cs(33,20): error IL2026: Using member 'System.Type.GetType(string, bool)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. The type might be removed. [C:\projects\serilog\src\Serilog\Serilog.csproj]
C:\projects\serilog\src\Serilog\Settings\KeyValuePairs\KeyValuePairSettings.cs(95,17): error IL2046: Interface member 'Serilog.Configuration.ILoggerSettings.Configure(Serilog.LoggerConfiguration)' with 'RequiresUnreferencedCodeAttribute' has an implementation member 'Serilog.Settings.KeyValuePairs.KeyValuePairSettings.Configure(Serilog.LoggerConfiguration)' without 'RequiresUnreferencedCodeAttribute'. 'RequiresUnreferencedCodeAttribute' annotations must match across all interface implementations or overrides. [C:\projects\serilog\src\Serilog\Serilog.csproj]
C:\projects\serilog\src\Serilog\Settings\KeyValuePairs\SettingValueConversions.cs(33,20): error IL2026: Using member 'System.Type.GetType(string, bool)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. The type might be removed. [C:\projects\serilog\src\Serilog\Serilog.csproj]
    0 Warning(s)
    3 Error(s)

(From the failed CI build)

@nblumhardt Thanks, sorry for the delay, took a little digging to track this down.

The remaining errors appear to be caused by a bug and an unimplemented feature, both of which were fixed in the .NET 7 SDK. Unfortunately, those fixes don't apply in your build because you're using the .NET 6 SDK (and even if you built using .NET 7, your global.json forces the 6.0 version).

I'm not sure what your preferred fix is. I could introduce some hacks, but using the 7.0 SDK seems more reliable.

given 7.0 SDK should be out this week, i dont think hacks to work with 6sdk will be necessary

Looks like appveyor Ubuntu is missing .net 7. Not sure if there’s a way to speed that up, or if we just need to wait for them to update.

It's been a couple months and I still don't see any updates for Ubuntu .NET. It would be good to know if they have plans to install 7.0, or if they stick on 6.0.

@nblumhardt @SimonCropp What do you think about switching from Appveyor to GitHub Actions? I got the impression that GitHub Actions has richer tuning possible and evolves faster.

Could you also add a sample app that consumes serilog and uses trimming. So we can smoke test the changes

I suggest to include "smoke test" directly into ci pipeline like it was done here https://github.com/graphql-dotnet/graphql-dotnet/blob/master/.github/workflows/test-code.yml#L74-L83, PR - graphql-dotnet/graphql-dotnet#3506

Another suggestion - use PolySharp as was done here https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/Directory.Build.props#L29 . PolySharp allows to get rid of annoying ifdefs scuttered across the codebase.

I think I managed to add an apt install command that works. Now things are failing because I think I made 8+ element tuples work on net5+ and there's a test that says they shouldn't 😄

we have net7 installing and running on linux on the main branch. what am i missing?

I think I just missed that -- I have to fix up the tests but that may be it.

This one's now been open a long time; I think there's still more to explore (could we just polyfill DynamicallyAccessedMembers for downlevel targets? Some unnecessary feature constants, etc.) but let's get the code through the pipeline as-is and iterate from dev. I'll have a hack at the failing test, and merge if I can sort it out :-)

Thanks @agocke!

could we just polyfill DynamicallyAccessedMembers for downlevel targets?

PolySharp does this