Update to lts 31 #165

tebeco · 2020-01-18T17:18:47Z

This PR intention is to update it to netcoreapp3.1 as it is LTS.
As of today, when serilog-aspnetcore is used in project targeting netcoreapp3.1, it "leaks" few transitive dependencies Microsoft.AspNetCore.xxx and Microsoft.Extensions.xxx version 2.x.y.

Update dependencies from 2.0.x to 2.1.* (Wildcard to get the latest patch available at build time)
As AspNetCore 3.1 is NOT netstandard2.0/2.1 but only netcoreapp3.1, i cross targetted to both
- netcoreapp3.1 for AspNetCore3.1 (LTS)
- netstandard2.0 for AspNetCore2.1 (Also LTS)
As AspNetCore 3.1 is LTS, i changed the Sample TargetFramework from netcoreapp3.0 to netcoreapp3.1
I've set MsBuild Condition on the TargetFramework because
- AspNetCore2.x is build with direct dependencies on Msft.Extensions 2.x
- AspNetCore2.x is build with direct dependencies on Msft.Extensions 3.x
  This means the dependencies version need to follow this
I change AppVeyor image to Vs 2019 (because netcoreapp is supposed to requires vs2019 but not sure it's needed here are running dotnet-install.ps1 -Channel 3.1 would just install the latest 3.1 SDK
I removed all powershell warning from the linter (general idea is to never use alias, as it could leads to missunderstand or error, eg : rm -rf the -f is for -Filter and not --force)
added serilog-extension-nuget.png to the repo and PackageIcon as the current PackageIconUrl is is deprecated for recent SDK (see https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#packageiconurl)

tebeco · 2020-01-18T17:24:02Z

I did not checked if some of the Serilog dependencies could be updated here to be fair :

<ItemGroup>
    <PackageReference Include="Serilog" Version="2.8.0" />
    <PackageReference Include="Serilog.Extensions.Hosting" Version="3.0.0" />
    <PackageReference Include="Serilog.Sinks.Console" Version="3.1.1" />
    <PackageReference Include="Serilog.Sinks.File" Version="4.0.0" />
    <PackageReference Include="Serilog.Sinks.Debug" Version="1.0.1" />
    <PackageReference Include="Serilog.Settings.Configuration" Version="3.1.0" />
    <PackageReference Include="Serilog.Formatting.Compact" Version="1.0.0" />
</ItemGroup>

but i can take a look

If you'd like to avoid manual check for the future, there's a very easy way using nuget to acheive "Get latest stable version", that could be used only for serilog dependencies here :

  <PropertyGroup>
    <LatestVersion>[*, 9999.0)</LatestVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Serilog" Version="$(LatestVersion)" />
    <PackageReference Include="Serilog.Extensions.Hosting" Version="$(LatestVersion)" />
    <PackageReference Include="Serilog.Sinks.Console" Version="$(LatestVersion)" />
    <PackageReference Include="Serilog.Sinks.File" Version="$(LatestVersion)" />
    <PackageReference Include="Serilog.Sinks.Debug" Version="$(LatestVersion)" />
    <PackageReference Include="Serilog.Settings.Configuration" Version="$(LatestVersion)" />
    <PackageReference Include="Serilog.Formatting.Compact" Version="$(LatestVersion)" />
  </ItemGroup>

tebeco · 2020-01-18T17:42:12Z

as you can see here :

Serilog.Extension.Hosting need to same update because of Microsoft.Extensions.Hosting.Abstractions (2.1.0)
Serilog.Settings.Configuration for Microsoft.Extensions.Options.ConfigurationExtensions (2.0.0) and Microsoft.Extensions.DependencyModel (2.0.4)

sungam3r · 2020-01-18T18:23:44Z

Build.ps1

 	Remove-Item .\artifacts -Force -Recurse
 }

 & dotnet restore --no-cache

-$branch = @{ $true = $env:APPVEYOR_REPO_BRANCH; $false = $(git symbolic-ref --short -q HEAD) }[$env:APPVEYOR_REPO_BRANCH -ne $NULL];
-$revision = @{ $true = "{0:00000}" -f [convert]::ToInt32("0" + $env:APPVEYOR_BUILD_NUMBER, 10); $false = "local" }[$env:APPVEYOR_BUILD_NUMBER -ne $NULL];
+$branch = @{ $true = $env:APPVEYOR_REPO_BRANCH; $false = $(git symbolic-ref --short -q HEAD) }[$null -ne $env:APPVEYOR_REPO_BRANCH];


What is the reason for changing the order of the arguments?

it's a warning from the linter too, shipped within the Powershell extensions (msft powershell team)

i don't have the underlying reason for this one.
my personal guess would be that it applies the same principle as some unit test lib : expected/constants on the left operand.

but it's just a guess

This reminds me https://en.wikipedia.org/wiki/Yoda_conditions

i can totally revert it ;)
less warning, less "eye catchy squiggle" ^^
like "always try to enable WarnAsError"

It doesn’t matter to me.

tebeco · 2020-01-18T18:29:00Z

FYI : currently updating the 2 other serilog repository that leak transitive from 2.x and i will really need a decision from serilog maintainer
more detail to come in these PR

andrewlock · 2020-01-19T18:23:28Z

I previously suggested multi-targeting to 3.x with @nblumhardt in serilog/serilog-extensions-logging#155 but we concluded there's actually no reason to do so, and it makes tooling etc more complicated.

I discussed the issues recently in a blog post: https://andrewlock.net/converting-a-netstandard-2-library-to-netcore-3/#libraries-that-depend-on-microsoft-extensions-nuget-packages

The conclusion was that there's no need to upgrade this library, serilog-extensions-hosting, or serilog-extensions-logging! The other commits that remove deprecated properties etc would definitely be handy though 🙂

tebeco · 2020-01-20T08:53:48Z

EDIT : previous answers was weird ;)

As you said, serilog-extensions-logging only consume Microsoft.Extensions.Logging
https://github.com/serilog/serilog-extensions-logging/blob/dev/src/Serilog.Extensions.Logging/Serilog.Extensions.Logging.csproj#L32
So you are right serilog-extensions-logging, can stay on netstandard2.0

This repo consumes a package the was previously shipped as a Nuget, and now shipped from the FrameworkReference (which is only netcoreapp3.1)
https://github.com/serilog/serilog-aspnetcore/blob/dev/src/Serilog.AspNetCore/Serilog.AspNetCore.csproj#L31

I made 2 other PR to 2 others repo that only consumed Microsoft.Extension.XXX,
I first used netcoreapp3.1 to discriminates for consumer that uses AspNetCore 2.x or AspNetCore 3.x

Few minutes before your answer here, i rolled that back to netstandard2.0 with this idea :
There's only 2 LTS version that has not reached End Of Life yet : 2.1 and 3.1
So anybody staying on 2.1 kinda is "pinning his runtime" to a specific (old/older) version already.
My point is that, pinning the runtime also means pinning dependencies compliant with that runtime.

This is done already in Serilog.Settings.Configuration, for example :
https://github.com/serilog/serilog-settings-configuration/blob/dev/src/Serilog.Settings.Configuration/Serilog.Settings.Configuration.csproj#L34

nblumhardt · 2020-02-10T21:34:14Z

Hi @tebeco - I think we should go ahead with this change; thanks for sending the PR, and sorry about the slow turnaround at this end.

Would it be possible to remove the other build script and CSPROJ changes (latest version references etc.) from the PR, and just commit the netcoreapp3.1 targeting change? The other parts are somewhat standardized across all of the Serilog repos and any changes there will need more consideration/discussion that's separate to the issue covered by this PR.

Thanks!

tebeco · 2020-02-12T17:04:14Z

So this means that when the nuget is released, serilog transitive will potentially points to deprecated version ?

tebeco · 2020-02-12T17:11:25Z

As you said, you standardize few things, like the icon URL. As nuget team is actively depracting it, that's why i changed it here as it's probably going to be needed.

No idea why the change, but i guess that relying on an external URL for an icon can lead to issues like DNS issues / timeout / unpaid domain / overflow / crafted response / ...

tebeco · 2020-02-12T17:12:32Z

@nblumhardt
Can you check the new history now ? Does it look like what you had in mind ?

nblumhardt · 2020-02-12T23:51:27Z

Thanks for the follow-up @tebeco 👍

We should make the <PackageIcon> change - sorry I wasn't clear (we've updated this in many other Serilog repos already). It was changes like $(LatestVersion) that I'd prefer to hold off.

It looks like you may not have pushed your updated changes to your branch? Everything still looks the same. Cheers!

tebeco · 2020-02-17T16:00:47Z

@nblumhardt
Sorry I pushed the wrong branch -_-
classic PEBKAC

nblumhardt

Thanks! This looks great; just one minor thing regarding referenced package versions, otherwise I think we're set! 😎

src/Serilog.AspNetCore/Serilog.AspNetCore.csproj

tebeco · 2020-02-19T08:30:35Z

@nblumhardt
any thing else I could have missed ?

tebeco · 2020-02-19T08:34:42Z

regarding all the ps1 stuff, it almost feels like you want to create a Serilog.BuildTool package just like dotnet/arcade so that all your technical scripts / images for the Icon / Copyright etc ... are all in an eng folder (engineering I guess)

so everytime you update the build process,

you update that lib (or dotnet tool ?)
pull it down per repo
ran it so that it update eng folder
you're never supposed to manually edit eng folder (like a lock file)
git add eng && git commit -m "update build tooling"

that way you could may a bot attempts to automatically creates PR + run the script of all organization repositories here

=> One fix to fix them all

… 3.1

…2019

…s://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#packageiconurl)

…uget being used at build time

tebeco · 2020-02-19T09:10:02Z

(rebased / force-with-lease on dev branch)

nblumhardt · 2020-02-19T21:24:58Z

👍 thanks! I'll bump the package version to 3.3.0 because of the dependency version updates. We'd normally consider these a breaking change, but since the package will still run on the earlier versions of those dependencies, I think we can squeeze this into a minor version bump.

tebeco · 2020-02-19T21:29:43Z

Thx a lot for your time ;)

Do you think the others repo that were not bump could cross target netstandard2.1, i understand that they do no explicitly uses ns2.1 API, but that would makes netcoreapp3.1 resolves it, so that Transtive like Msft.Ext.Logging could be resolving 3.1.2 for example.

So far nuget resolves the oldest possible match => it does not respect anymore HigestDependency Settings so sometimes it resolves Msft.Ext.Logging 2.1 at work for stacks using netcoreapp3.1

That would be a way to helps nuget resolves proper dependencies based on the TFM wihtout breaking backward compat.
Should i re-open an issue about that on these repo ?
(pinging @andrewlock as he was also involved with you in that process / discussion)

CleverNeologism · 2020-02-19T22:19:04Z

Should the SDK in global.json also be updated to latest 3.1? Stock VS2019 install will not open this solution. It is also not the latest 3.0 version.

tebeco · 2020-02-19T22:20:37Z

for this PR ? 3.1.102 was released last night so that may be why

also as you can see here i've setup proper version for packages (my brain half functions):

tebeco · 2020-02-19T22:26:22Z

#172

sungam3r · 2020-02-20T06:18:38Z

@CleverNeologism @tebeco Hi, can you tell me what is the point in the global.json file? When I do not have the exact sdk mentioned in global.json, then solution does not open. It’s easier for me just to delete this file so that everything works.

tebeco · 2020-02-20T06:20:05Z

https://docs.microsoft.com/en-us/dotnet/core/tools/global-json?tabs=netcore3x

that's probably faster to link than attempting a long explanation that could potentially be wrong
in short ...
if you have trouble right now since this file points to the latest version
=> please everything you are doing and update both your sdk and visual studio
vs2019 16.4 is required for netcoreapp3.1

tebeco requested a review from nblumhardt January 18, 2020 17:18

sungam3r reviewed Jan 18, 2020

View reviewed changes

This was referenced Jan 18, 2020

Update to lts 31 serilog/serilog-extensions-hosting#17

Closed

Update to lts 31 serilog/serilog-settings-configuration#205

Closed

nblumhardt reviewed Feb 18, 2020

View reviewed changes

src/Serilog.AspNetCore/Serilog.AspNetCore.csproj Outdated Show resolved Hide resolved

tebeco added 7 commits February 19, 2020 10:09

Cross target to netcoreapp3.1 (LTS)

de27db1

cross target the samples to be sure it ran for both netcoreapp2.1 and…

f76b6db

… 3.1

re-target samples from netcoreapp3.0 to 3.1 (LTS)

17ecc78

Change base image of app veyor to vs2019 as netcoreapp3.0 requires Vs…

ff288cd

…2019

Update serilog transitive

072ac22

Add PackageIcon as PackageIconUrl in going to be deprecated (see http…

e712b40

…s://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#packageiconurl)

Remove wildcard, in favor of using explicitly the latest version of n…

d5bf961

…uget being used at build time

nblumhardt merged commit 3433261 into serilog:dev Feb 19, 2020

tebeco deleted the update-to-lts-31 branch February 19, 2020 23:16

nblumhardt mentioned this pull request Jul 24, 2020

3.4.0 Release #202

Merged

This was referenced Mar 17, 2021

Serilog dropped cross-build for netcoreapp #239

Closed

Drop Microsoft.AspNetCore.Http.Abstractions for netcoreapp3.1 and higher open-telemetry/opentelemetry-dotnet#1917

Merged

julian94 mentioned this pull request Dec 14, 2022

Serilog.AspNetCore depends on a version of Serilog.Settings.Configuration that depends on a deprecated dependency with expired signature #311

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to lts 31 #165

Update to lts 31 #165

tebeco commented Jan 18, 2020

tebeco commented Jan 18, 2020 •

edited

Loading

tebeco commented Jan 18, 2020

sungam3r Jan 18, 2020

tebeco Jan 18, 2020 •

edited

Loading

sungam3r Jan 18, 2020

tebeco Jan 18, 2020 •

edited

Loading

sungam3r Jan 18, 2020

tebeco commented Jan 18, 2020 •

edited

Loading

andrewlock commented Jan 19, 2020

tebeco commented Jan 20, 2020

nblumhardt commented Feb 10, 2020

tebeco commented Feb 12, 2020

tebeco commented Feb 12, 2020

tebeco commented Feb 12, 2020

nblumhardt commented Feb 12, 2020

tebeco commented Feb 17, 2020

nblumhardt left a comment

tebeco commented Feb 19, 2020

tebeco commented Feb 19, 2020

tebeco commented Feb 19, 2020

nblumhardt commented Feb 19, 2020

tebeco commented Feb 19, 2020

CleverNeologism commented Feb 19, 2020 •

edited

Loading

tebeco commented Feb 19, 2020 •

edited

Loading

tebeco commented Feb 19, 2020

sungam3r commented Feb 20, 2020

tebeco commented Feb 20, 2020 •

edited

Loading

Update to lts 31 #165

Update to lts 31 #165

Conversation

tebeco commented Jan 18, 2020

tebeco commented Jan 18, 2020 • edited Loading

tebeco commented Jan 18, 2020

sungam3r Jan 18, 2020

Choose a reason for hiding this comment

tebeco Jan 18, 2020 • edited Loading

Choose a reason for hiding this comment

sungam3r Jan 18, 2020

Choose a reason for hiding this comment

tebeco Jan 18, 2020 • edited Loading

Choose a reason for hiding this comment

sungam3r Jan 18, 2020

Choose a reason for hiding this comment

tebeco commented Jan 18, 2020 • edited Loading

andrewlock commented Jan 19, 2020

tebeco commented Jan 20, 2020

nblumhardt commented Feb 10, 2020

tebeco commented Feb 12, 2020

tebeco commented Feb 12, 2020

tebeco commented Feb 12, 2020

nblumhardt commented Feb 12, 2020

tebeco commented Feb 17, 2020

nblumhardt left a comment

Choose a reason for hiding this comment

tebeco commented Feb 19, 2020

tebeco commented Feb 19, 2020

tebeco commented Feb 19, 2020

nblumhardt commented Feb 19, 2020

tebeco commented Feb 19, 2020

CleverNeologism commented Feb 19, 2020 • edited Loading

tebeco commented Feb 19, 2020 • edited Loading

tebeco commented Feb 19, 2020

sungam3r commented Feb 20, 2020

tebeco commented Feb 20, 2020 • edited Loading

tebeco commented Jan 18, 2020 •

edited

Loading

tebeco Jan 18, 2020 •

edited

Loading

tebeco Jan 18, 2020 •

edited

Loading

tebeco commented Jan 18, 2020 •

edited

Loading

CleverNeologism commented Feb 19, 2020 •

edited

Loading

tebeco commented Feb 19, 2020 •

edited

Loading

tebeco commented Feb 20, 2020 •

edited

Loading