-
Notifications
You must be signed in to change notification settings - Fork 454
Cleanup and relocate BUILD_VERSION calculation #1382
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
Cleanup and relocate BUILD_VERSION calculation #1382
Conversation
- The computation of BUILD_VERSION is now lifted above project(), which allows us to use that value in the project() call itself, giving us correct PROJECT_VERSION values throughout. - Define BUILD_VERSION using mongo_setting(), and use the calc_release_version.py to compute its default value.
OK, so before I review the changes, I will try to address some of your overall questions/concerns. A little bit of historical context is probably useful here. In the beginning (of the libbson and mongo-c-driver projects), releases were made as tarballs (which you well know from your recent work on that part of the build). Knowledge of the current release version (and previous release version) was necessary for the build to produce correct results (i.e., so that it could populate the release version in one of the headers, and so that it could generate correct documentation for the current version and links to prior version documentation). In a release tarball that lacked Git history, the way to accomplish this was by creating the files When the packaging for RPM and DEB was done, it was necessary to determine the version number from the associated package-specific metadata (RPM spec file and My recollection is that the solution was to implement the
This rough logical outline reflects what I believe is the final/current state. It bears mentioning that we arrived here as a result of evolving the process through various steps. Some of the conditions which motivated some previous conditions are no longer relevant. I have no problem with completely reworking how we handle informing the build of what version is being built, since given what has changed, it seems likely we could use a much simpler approach to achieve the necessary result. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely like the simplification you've implemented here. In light of my overall comment on the PR, I would absolutely be in favor of a more comprehensive rethinking/reworking of how we handle the version (i.e., especially given that the release tarball is no longer a thing).
Also, when you merge this (or if you decide on a more comprehensive reworking, after you merge that) could you implement corresponding changes in the C++ driver and libmongocrypt so that we don't end up with divergent ways of handling the build version?
@vector-of-bool One thing I forgot to mention is that you have to be careful about Python 2 -> 3. That is, your change doesn't seem to make provision for running on a platform that has only Python 2. In the libmongocrypt project (with which these changes will have to be synced at some point), we still build on RHEL 6.2, which has Python 2.6. The approach that we take still needs to work there. |
Both this and #1383 can be tweaked to work with Python 2.6, so that requirement is no problem. |
Possible concerns:
LoadVersion
andParseVersion
are extremely similar, but both are required to get the project to work. Some locations refer to the_VERSION_PATCH
suffix, while others use_MICRO_VERSION
suffix. These could/should probably be consolidated to a single variable set.BUILD_VERSION
(as a cache variable) serves whenVERSION_CURRENT
(the text file) will usually take precedence.