Open sven-molkenstruck opened 1 year ago
Maybe it should be a version range https://learn.microsoft.com/en-us/nuget/concepts/dependency-resolution?
Thanks Prince. I think I need to clarify my post above. The problem is not the dependency on version 1.1.0 of that openssl package. The problem is that it depends on that package at all, instead of allowing me to use ANY openssl (or LibreSSL or whatever) package. The user should be allowed to link any openssl or libressl or whatever lib to their project, even from a different source than from NuGet. It IS possible when I include jwt-cpp in any other way, but not when I use NuGet. So I think inside the NuGet configuration there should not be any dependency listed.
It's been a fairly long time since I used NuGet in particular, but if I use a dependency manager I kind of expect it to pull in all dependencies of the package I install. It feels like that's its job. The issue here is in my opinion that only one possible package specified. In other systems its possible to specify a list of compatible packages and versions, one of which has to be installed, but I am not sure if NuGet can do this and if, how. jwt-cpp
definitely does support all the listed ssl libraries (and probably more as long as they are somewhat api compatible), we even run CI tests on every commit to make sure it does.
Looking at the docs it seems to be possible to specify a list of supported versions, but you can't specify a list of packages.
So basically we seem to be presented with three ways:
Personally I'd go with the last option, possible with a fourth package that does not depend on any library. Basically
jwt-cpp
jwt-cpp-openssl
jwt-cpp-wolfssl
jwt-cpp-libressl
EDIT: Thinking some more about it we kinda have the same issue with json libraries. Currently we depend on picojson, but we support a bunch of others.
EDIT2: Seems like its not that simple because nuget doesn't seem to be able to automatically pick the correct package for the current toolchain, so in addition to having a version for every ssl lib, every json library we'd also need a version for every breaking toolchain version. According to MS You can mix binaries built by different versions of the v140, v141, v142, and v143 toolsets. However, you must link by using a toolset at least as recent as the most recent binary in your app.
, but thats not really an option because if we always depend on the latest toolset version it prevents users using older VS from using jwt-cpp. So unless we require that (which sounds like a bad idea) we would have to build and publish at least num_ssl(4) * num_json(4) * num_toolset(4) = 64
as of right now, with the expectation for that number to increase in the future. It wouldn't be that much of an issue, because CI can do it, but it still feels wrong. TLDR: I think we might be best off removing all dependencies and putting a big fat warning in the package description with a list of usable/required dependencies.
Thanks @Thalhammer Very good reply!
I appreciate the concept "Dependency Manager". You are right, when I use NuGet, I should not (and actually I do not) want to choose my favorite sub-dependencies, I just want it to work. The challenge is that NuGet apparently has no openssl package that is as flexible (regarding platform/toolset) as jwt-cpp is.
As a workaround, I found that I can manually circumvent the dependency to openssl-vc141-static-x86_64.1.1.0 in NuGet using "force uninstall":
So while I do agree with your conclusion,
TLDR: I think we might be best off removing all dependencies and putting a big fat warning in the package description with a list of usable/required dependencies.
alternatively we could leave everything as it is, and add documentation how to make it work on platforms other than vc141.
Sven
Hunter requires nothing https://github.com/Thalhammer/hunter/blob/49b971ee85ee1aa481176110e1ec612748f8b71b/cmake/projects/jwt-cpp/hunter.cmake
vcpkg hardcodes picojson but no openssl... https://github.com/microsoft/vcpkg/blob/70992f64912b9ab0e60e915ab7421faa197524b7/ports/jwt-cpp/vcpkg.json#L8 there;s no override feature for a different one that I could find searching quickly
Conan lists openssl (but any recipe that's a provider can be overridden) https://github.com/conan-io/conan-center-index/blob/a08f48696f5e06bd6dfaa68a35458502bfd73b04/recipes/jwt-cpp/all/conanfile.py#L26 and there's no JSON library
and nuget hardcodes both...
So I guess the bigger question is what should we make the default?
Personally I think 2️⃣ is the best, but let me know if you think differently
Hunter requires nothing
Hunter requires what you select. It basically sets up a folder so that find_package
can find the stuff it needs, but it seems we only have hunter_add_package
for OpenSSL in there. Theoretically it could support all of them though (assuming we add support to the cmakelists.txt). You could then set the required one normally using JWT_SSL_LIBRARY
and it would "depend" on what you select.
@Sven-HP Even though that workaround works for you I don't think we should treat it as the default solution for this issue, because it doesn't really work for CI because the way I understand it you need to do the uninstall step manually every time you set up the project on a new machine.
but it seems we only have
hunter_add_package
for OpenSSL in there.
I did not see that when I look :eyes: I just noticed its in out repo, so we having to add it here which is kinda gross but easier to maintain.
That's 2 for option :two: IT's just nuget and vcpkg that we need to update to match?
@Sven-HP Even though that workaround works for you I don't think we should treat it as the default solution for this issue, because it doesn't really work for CI because the way I understand it you need to do the uninstall step manually every time you set up the project on a new machine.
I have tested that. It does work for CI. My workaround only has to be done once, when adding jwt-cpp to the project, and not on every machine. The result is the desired package selection (here: openssl-vc143 instead of openssl-vc141) stored correctly in my project files (*.vcxproj, packages.config), and that gets committed into my project's repo as usual, and works out of the box on other machines. :-)
Hi,
I am using NuGet to include jwt-cpp into my project. Unfortunately it force-installs Edit: ~an outdated~ a specific old NuGet package of OpenSSL, openssl-vc141-static-x86_64.1.1.0, which does not work under Visual Studio 2022 (vc143). I suggest to remove those dependencies and let the user decide and care about OpenSSL. Edit: I might want to link a vc143 version from NuGet, e.g. "zeroc.openssl.v143", or some other version from some other source.
That strict dependency is a contradiction to the README statement at https://github.com/Thalhammer/jwt-cpp#readme , "In the name of flexibility and extensibility, jwt-cpp supports... [various OpenSSL versions and alternatives like LibreSSL]".
From https://github.com/Thalhammer/jwt-cpp/blob/master/nuget/jwt-cpp.nuspec lines 17-20:
@diogo-strube It seems you are the expert about this :-)
Can you please simply remove those dependencies, or is there something I'm not seeing? Thank you!
Sven