-
-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding JWT_VERSION_NUMBER #273
Comments
Hmm this is a very interesting idea. I am curious how you plan on using it? Do you need to simultaneously support two version in your code base? With CMake's I'd love to understand a little more how this will be helpful 😄 |
I think a huge point where this would be usefull is when jwt-cpp is not imported by another piece of the software and you want to adopt to whatever version is available. I had a similar issue with io_uring recently which makes it near impossible to adopt/detect available features at compile time without support from the build system, because theres no version define. libcurl on the other hand is a really good example where this is possible, it provides a version define, a function to check if it is above a specific version and the version something was introduced is part of the documentation of every function/symbol. This makes adopting to various different versions really simple. I think its a great idea to do something similar for jwt-cpp, but I definitely think we should tie it into the build system somehow so we don't have to update/change the number manually every time. |
Currently my code did not use cmake but just import jwt source in the folder. If possible, I prepare my code using another library be compatible with all versions. And a last point : I suggest you break api compatibility (by removing old function like get_payload_claims / get_header_claims only if this is really needed) sqlite, curl did not broke compatibility since 10 years (at least)! |
Another point for compatiblity: If both are incompatible, this create problem |
Don't forget people which don't use cmake |
I didn't necessarily mean that in the way of using the build system to insert it into the header. An alternative would be to store it in the header and parse the header inside the buildsystem to use when doing e.g. packaging. I just wanna avoid having too many places where we need to update stuff. CMake is fairly flexible when it comes to building and executing stuff at configure time, so a fairly straightforward way (apart from a regex) would be to build a small c++ snippet that includes the header and just prints the version number to
Unless they are contained in separate pieces (i.e. two shared objects with internal linking) and never visible at the same time (both link and compile unit) this should work, however if they are (and unless you take extra care, they probably are) this causes problems regardless if we break api or not. jwt-cpp is a header only library, meaning every change is abi breaking (except for formating, but even that is debatable) because you'll get ODR violations. You can't guarantee correctness if you use multiple versions (as in the file hash differs, not in release version number) in the same program and we can't and won't give any guarantees if you do. Note that because jwt-cpp is templated on the json lib, this means that a breaking change in the json lib you use is also a breaking change in jwt-cpp for the same reasons.
This is perfectly fine and in fact the way it was intended in the beginning. CMake is merely there to make it simpler to package, invoke tests, etc.
SQLite and Curl are fundamentally different libraries. They are not typesafe, have a C api and haven't really changed in the last 10 years (at least the user api). Don't get me wrong, I don't like breaking API (it prevents people from upgrading and thats kinda bad in a security related library like jwt-cpp) and I think we managed to do this fairly good in previous versions. But at some point you need to get rid of bad design decisions and move forward. The claim API imho is one of them. It was intended to hide the underlying json library when it was an implementation detail and not a user choice, but that changed over the time and now its simply inconsistent and doesn't fit the rest of the library. Usually we mark stuff as deprecated for a few versions (which is already a breaking change in many cases cause if you compile with Werror, as you should, your code won't compile anymore) and eventually remove them. |
What would you like to see added?
I suggest adding a JWT_VERSION_NUMBER constant in jwt.h
Additional Context
JWT 0.7 introduce creaking change API.
I suggest adding JWT_VERSION_NUMBER, like OPENSSL_VERSION_NUMBER
The text was updated successfully, but these errors were encountered: