Skip to content

CDRIVER-4763 reapply BUILD_VERSION CMake option #1462

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

Merged

Conversation

kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Nov 1, 2023

Summary

Document removal of BUILD_VERSION CMake option.

Apply BUILD_VERSION CMake option

Background & Motivation

#1382 removes the ability to specify a custom BUILD_VERSION.

This may result in an error when if attempting to build a source archive with steps that previously worked on 1.24.4:

wget "https://github.com/mongodb/mongo-c-driver/archive/refs/tags/1.25.0.tar.gz"
tar xf 1.25.0.tar.gz
cd mongo-c-driver-1.25.0
cmake -DBUILD_VERSION=1.25.0 -S. -Bcmake-build

Results in this error due to BUILD_VERSION being ignored:

Traceback (most recent call last):
  File "/Users/kevin.albertson/Desktop/mongo-c-driver-1.25.0/build/cmake/../calc_release_version.py", line 400, in <module>
    RELEASE_VER = previous(main()) if PREVIOUS else main()
                                                    ^^^^^^
  File "/Users/kevin.albertson/Desktop/mongo-c-driver-1.25.0/build/cmake/../calc_release_version.py", line 320, in main
    head_commit_short = check_output(['git', 'rev-parse', '--revs-only',
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kevin.albertson/Desktop/mongo-c-driver-1.25.0/build/cmake/../calc_release_version.py", line 136, in check_output
    raise subprocess.CalledProcessError(ret, args[0])
subprocess.CalledProcessError: Command 'git' returned non-zero exit status 128.
CMake Error at build/cmake/BuildVersion.cmake:43 (message):
  Computing the build version failed! [1]:

Call Stack (most recent call first):
  build/cmake/BuildVersion.cmake:50 (compute_build_version)
  CMakeLists.txt:5 (include)


-- Configuring incomplete, errors occurred!

Creating a VERSION_CURRENT file resolves the error. This was done in Homebrew/homebrew-core#153065.


If git history is available, the BUILD_VERSION is quietly ignored. The git history is used to compute the version:

git checkout a09129be6a3e1c875d4f923111e47b9a6ed4702c
if [ -f VERSION_CURRENT ]; then rm VERSION_CURRENT; fi
cmake -Bcmake-build -S. -DBUILD_VERSION=1.25.0-custom > /dev/null 2>&1
cat VERSION_CURRENT

Results in computing the version from the git history:

1.11.1-20231101+gita09129be6a

With changes in this PR:

git checkout document-removal-of-BUILD_VERSION
if [ -f VERSION_CURRENT ]; then rm VERSION_CURRENT; fi
cmake -Bcmake-build -S. -DBUILD_VERSION=1.25.0-custom > /dev/null 2>&1
cat VERSION_CURRENT

Results in applying the BUILD_VERSION from the CMake options:

1.25.0-custom

This PR proposes documenting this change in NEWS to inform use of the alternative.

Specifying BUILD_VERSION is documented: https://mongoc.org/libmongoc/1.25.0/learn/get/from-source.html#configuring-for-libbson

This PR proposes adding back support for the BUILD_VERSION option. This is intended to avoid quietly ignoring the applied BUILD_VERSION on existing builds specifying a custom BUILD_VERSION CMake option.

Copy link
Contributor

@rcsanchez97 rcsanchez97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am OK with documenting it this way if we want to discourage users from using it. However, it isn't exactly true that it has been removed. At least, in debian/rules we still specify it

        -DBUILD_VERSION=$(DEB_VERSION_UPSTREAM) \

I am not aware of this breaking.

@kevinAlbs kevinAlbs changed the title document removal of BUILD_VERSION apply BUILD_VERSION CMake option Nov 1, 2023
@kevinAlbs kevinAlbs changed the title apply BUILD_VERSION CMake option CDRIVER-4763 apply BUILD_VERSION CMake option Nov 1, 2023
@kevinAlbs
Copy link
Collaborator Author

However, it isn't exactly true that it has been removed. At least, in debian/rules we still specify it

        -DBUILD_VERSION=$(DEB_VERSION_UPSTREAM) \

I am not aware of this breaking.

I expect the BUILD_VERSION option is quietly ignored, and instead is computed from the git history.

This PR has been updated to apply the BUILD_VERSION CMake option. This is intended to avoid quietly changing builds specifying a custom BUILD_VERSION which also have git history available.

@kevinAlbs kevinAlbs requested a review from rcsanchez97 November 1, 2023 19:13
Copy link
Contributor

@rcsanchez97 rcsanchez97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I agree that this is a better approach. The removal caused a minor issue for Kyle and me in the packaging of 1.25.0. It was easy to work around it, and while it wasn't a silent change in the packaging scenario (since the packaging build runs without Git history available) I can see how the silent change in behavior could catch someone by surprise.

@kevinAlbs kevinAlbs changed the title CDRIVER-4763 apply BUILD_VERSION CMake option CDRIVER-4763 reapply BUILD_VERSION CMake option Nov 2, 2023
@kevinAlbs kevinAlbs merged commit 0070a3e into mongodb:master Nov 7, 2023
kevinAlbs added a commit that referenced this pull request Nov 7, 2023
* document removal of BUILD_VERSION

* apply `BUILD_VERSION` from CMake option if specified

* update NEWS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants