PKISharp / ACMESharpCore

An ACME v2 client library for .NET Standard (Let's Encrypt)
MIT License
325 stars 72 forks source link

Update dependencies #30

Closed georg-jung closed 4 years ago

georg-jung commented 4 years ago

While developing a plugin for win-acme I noticed that the dependencies of this library are not up-to-date. Due to the way the plugins in win-acme are loaded this is a problem if you consume a library, which uses up-to-date dependencies (i.e. the plugin consumes library X which uses Json.Net 12 -> plugin can not be loaded).

I updated all dependencies in the projects, fixed some incompatibilities due to the upgrades and ran all tests I was able to (so not the IntegrationTests), which succeeded.

Feel free to ask if you need more input regarding this.

I also changed the azure-pipelines configuration so that it publishes artifacts and runs some of the tests, see https://dev.azure.com/georg-jung/ACMESharpCore/_build?definitionId=5.

ebekker commented 4 years ago

You said "Due to the way the plugins in win-acme are loaded" -- how does it load its libraries that makes this a requirement?

In general, if you start down this path, it means you need to keep updating your libraries every time an external library dependency gets updated. That's not really a very tenable approach.

Additionally, by bumping up the target frameworks, because this is primarily a library and not an app, you break this library for any other apps that may be using it because you've increased the minimum target framework version dependency.

georg-jung commented 4 years ago

I believe I didn't change the target framework of any library component, but just the one of test projects. If you point me to any incompatibility I introduced, I'll of corse look into it! I can't think of any drawback in targeting the newest frameworks for the tests, but I guess it makes it simpler to stay up to date in future. I really tried to keep an eye on not introducing any breaking changes in this regard.

I'd be happy about the updated dependencies specifically because I want to add support for Cloudflare to win-acme by writing a plugin (I already did and tested it, I'd send a PR as soon as there are no conflicting dependencies anymore). That's one of the reasons I started working on https://github.com/georg-jung/FluentCloudflare. The library targets Json.Net 12.x, as this is currently the newest available version. ACMESharpCore targets 11.x, but as far as I can see updating to 12.x wouldn't be breaking in any way.

"The way plugins are loaded" now means, as they are not really a dependency but are loaded at runtime, it's problematic if a plugin uses a newer version of a dependency then the core software, including its "real" dependencies. One can't ship a newer version of i.e. Json.Net with the plugin as an assembly with the same name is already loaded. The plugin can not be loaded with the older, already loaded version either, because it's a major version behind. As far as I can see there are only two ways out:

  1. Update the dependencies
  2. Every library needs to target the lowest common denominator. I.e. I would build my library targeting outdated dependencies.

In my opinion its better to keep as many dependencies up-to-date as possible than to introduce kind of a hard top end of possible library versions that can be used all over the dependency tree.

So, I don't just want to update for the sake of updating but I think there is a direct advantage in doing so this time. I'm not saying library authors should push (in sense of to NuGet) new versions of their library for every single updated dependency. Much more I would like to solicit for updating them from time to time, possibly as part of a next release which would have happened anyway. Of course, when there are relevant breaking changes, this needs to be reconsidered. But when there are not, do you see anything against updating?

Personally I like https://dependabot.com/ which make it easier to keep up with the new versions of dependencies, especially if they don't introduce relevant breaking changes. If I can help with anything further feel free to ask.

georg-jung commented 4 years ago

Note: I rebased this on the current master