Build cleanup

[mgravell]

• use Directory.Build.props for common config
• use Directory.Packages.props for dependencies
• add package icon
• enable source link, deterministic builds, etc
• configure licence, package, etc in package file
• add net481 as an explicit TFM (avoids some ns* package-hell scenarios)
• use automatic versioning with pinned assembly version
• configure build SDK

[mgravell]

Before:

After:

[mgravell]

Build fail is because of git height / automatic version; investigating:

Nerdbank.GitVersioning.GitException: Shallow clone lacks the objects required to calculate version height. Use full clones or clones with a history at least as deep as the last version height resetting change.

We might also have to tweak global.json; IMO we should change the CI server to build with (whatever it says there), and run with (chosen platform) - otherwise we need to limit the C# to the lowest version supported by SDKs in the CI suite.

[atakavci]

consider changing the way versions handled for release pipeline here in nuget-release

[mgravell]

Yep, that would be good to resolve along with NBGV, which is the default/go-to automated versioning tool; however, that was complaining about git checkout - looks like somewhere we're using shallow clone (NBGV needs a bit more to calculate the version via commits), so: that's something we'd need to fix in parallel. I suggest we defer that for now, but yes: that would be a good thing to fix.

[mgravell]

basically: this version.json was "get that process started", but: a cog fell out, so that's slightly incomplete

[atakavci]

Build fail is because of git height / automatic version; investigating:

@mgravell , might require the git history with actions/checkout@4 for NerdBank, i think i hit similar issue with SE.Redis pipeline before here .
try with fetch-depth: 0

[mgravell]

ah, yes, that's the voodoo I was looking for; what's your preference:

• I fix that in this one?
• I do that in a separate PR?

[mgravell]

As a side note: before we pat ourselves on the back about ticking all the boxes: IMO the big missing box at the moment is strong naming (#77), which is causing problems (#99, #416). Normally I'd be a bit "eek" about adding a strong-name, but IMO since it only went 1.0 in March, my vote goes for "YOLO, add the strong name, and rev the major to 2.0" - I'd love your input on that, @uglide @atakavci

[atakavci]

@mgravell
i think those are two different PR's with possibly some confusion there. One attempts to sign the code while the original request (at least what i understand) is strong naming.
i explored there are cases that strong naming the lib without a clear context and in-advance notice created a lot of friction with some other client libraries in the past. That was the reason i hit the breaks.
And still don't have a strong opinion but a bit on the side of not doing it, since it doesn't seem to bring a huge value.

[atakavci]

ah, yes, that's the voodoo I was looking for; what's your preference:

* I fix that in this one?

* I do that in a separate PR?

whatever feels comfortable with you =)

[mgravell]

On strong name vs signing - you're right; I think one of the later comments in the signing one was actually talking about strong naming (see the public-key-token reference), and that misled me. Actual signing is ... yeah, a massive nuisance. If Redis have the key infrastructure to do so with suitable keys: fine. But I would suggest that it doesn't actually solve a problem re supply-chain substitution attacks unless SE.Redis and any other dependencies are *also* signed.

[AMeyerInkSoft]

Sorry for the spam, but heads-up in case this PR might be a better place to add a couple properties I suggested via #434 . The screenshots listed in comments toward the start of this PR suggest the packaging might be updated to cite the licensing another way, so pardon me if my suggestions are redundant. EDIT: I missed it at first glance, originally, but it looks like the this PR does indeed include a PackageLicenseExpression tag.

[mgravell]

I was worried for a moment that I'd missed an obvious pre-existing PR, but it looks like yours is a newer one, and that I've already got it covered. If that understanding is correct, I think it is best we close the new one as a dup of this, but: thanks: your input is appreciated and correct - simply: I'm already cleaning up.

…

Message ID: ***@***.***>