Skip to content
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

Add version macros for RapidJSON #311

Merged
merged 4 commits into from Apr 22, 2015
Merged

Add version macros for RapidJSON #311

merged 4 commits into from Apr 22, 2015

Conversation

miloyip
Copy link
Collaborator

@miloyip miloyip commented Apr 21, 2015

Since RapidJSON is header-only library, I think that macros should be sufficient.

RapidJSON shall become v1.0.0 release after merge.

Fixes #310

@miloyip miloyip added this to the v1.0 milestone Apr 21, 2015
@miloyip
Copy link
Collaborator Author

miloyip commented Apr 21, 2015

This may be the final commit for v1.0.0.
Can @thebusytypist and @pah review of it?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 5ab1e93 on issue310_versionmacro into 8d39282 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 95c6ec9 on issue310_versionmacro into 8d39282 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 95c6ec9 on issue310_versionmacro into 8d39282 on master.

@jollyroger
Copy link
Contributor

It is possible to generate version header automatically using CMake.

  • make additional file called "version.h.in"and put all your version-related stuff there, but using CMake variables in several places:
    #define RAPIDJSON_MAJOR_VERSION @LIB_MAJOR_VERSION@
    #define RAPIDJSON_MINOR_VERSION @LIB_MINOR_VERSION@
    #define RAPIDJSON_PATCH_VERSION @LIB_PATCH_VERSION@
  • add something like this to your CMakeLists.txt:
    configure_file(version.h.in version.h @ONLY)
    include_directories(${CMAKE_CURRENT_BINARY_DIR})
    install(FILES ${CMAKE_CURRENT_BINARY_DIR}/version.h DESTINATION ${INCLUDE_INSTALL_DIR})
  • include version.h in rapidjson.h

This approach has some cons though: there's no version.h file before you run cmake at least once.

@miloyip
Copy link
Collaborator Author

miloyip commented Apr 21, 2015

Yes. I have thought of this solution but I don't want cmake compulsory for using the library. Can it do another way round? Parse/match the header to cmake? Besides I do not know how those variables in cmake are used in the build process.

@spl
Copy link
Contributor

spl commented Apr 21, 2015

Just wanted to add that it is a very useful feature to be able to use RapidJSON as a header-only library, without needing to compile anything. So, I agree with @miloyip that a build tool should not be required.

@jollyroger
Copy link
Contributor

Those variables were just a stub for you to use. Although I tend to have version defined in the build tool, there are possibilities to get version from other sources. One useful option is to use git-describe to get version and parse its output. This version will change with every commit and would be a normal x.x.x only for a tagged commit though. Sure, we can get version from header files like this:

  • generate C file that writes version into output and exits
  • run it
  • read its outout

I thought about parsing header files directly and it seems it is doable with some more additional effort.

@miloyip
Copy link
Collaborator Author

miloyip commented Apr 22, 2015

@jollyroger git-describe seems not a good idea because the user may get a source package without git information. I don't know if we can use regex to extract the numbers in rapidjson.h in CMAKE. But anyway, this is not hurry we can improve and make it more automatically later. Now we can just simply manually set both files.

@pah Thanks for macro simplification advice. However I try to use the same variable names as those in CMAKE for consistency.

miloyip added a commit that referenced this pull request Apr 22, 2015
@miloyip miloyip merged commit 7f43373 into master Apr 22, 2015
@miloyip miloyip deleted the issue310_versionmacro branch April 22, 2015 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add one function to get rapidjson version
5 participants