Drop VersionFromGit.cmake module for hardcoded version#22
Conversation
In case where the library is downloaded as a tarball (eg. Buildroot), the VersionFromGit module would fail resulting in the library not building. Since there aren't any immidiate fixes, I've hardcoded the VERSION in the CMake project(). Signed-off-by: Andrea Ricchi <andrea.ricchi@amarulasolutions.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the project's versioning strategy to enhance build reliability, particularly for non-Git-based distributions like tarballs. By removing the dependency on Git for version extraction and hardcoding the project version, it resolves issues where the build process would fail due to an inability to determine the version. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
🧩 Build Artifacts ✅ The following build artifacts were produced: |
There was a problem hiding this comment.
Code Review
This pull request addresses a build failure when the library is distributed as a tarball by removing the VersionFromGit.cmake module and hardcoding the project version to 1.0.0 in CMakeLists.txt. While this resolves the immediate issue, it removes the dynamic versioning capabilities previously derived from Git tags. This change necessitates manual version updates for future releases and impacts the consistency of package naming.
I am having trouble creating individual review comments. Click here to see my feedback.
CMakeLists.txt (7-8)
The removal of the VersionFromGit.cmake module eliminates the dynamic versioning based on Git tags. While this resolves the build issue for tarball distributions, it means the project version will now need to be manually updated in the project() command for each release. This increases the risk of versioning inconsistencies and manual errors. Consider exploring alternative methods for dynamic versioning that are compatible with both Git repositories and tarball distributions, such as defining a default version and overriding it with Git information if available.
CMakeLists.txt (9)
The VERSION 1.0.0 is now hardcoded. This is a direct consequence of removing the VersionFromGit.cmake module. While it fixes the tarball build, it means the project version will not automatically reflect Git tags. This requires manual updates for each release, which can be error-prone and lead to outdated version information in packages or documentation. It's crucial to establish a clear process for manually updating this version or to re-introduce a more robust versioning strategy.
CMakeLists.txt (105)
The change from SEMVER to PROJECT_VERSION for the cpack_file_name is correct given the removal of VersionFromGit.cmake. However, since PROJECT_VERSION is now hardcoded, the generated package file name will always be PROJECT_NAME-v1.0.0-... unless the version in the project() command is manually updated. This further emphasizes the need for a clear versioning strategy to ensure package names accurately reflect the release version.
There was a problem hiding this comment.
I would prefer to improve the version_from_git so that it does not fail if there is no git-tag, and get a PROJECT_VERSION_OVERRIDE that is passed by buildroot script like:
MYAPP_VERSION = 1.2.3
MYAPP_SITE = https://github.com/user/myapp/archive/refs/tags/v$(MYAPP_VERSION).tar.gz
MYAPP_CONF_OPTS += -DPROJECT_VERSION_OVERRIDE=$(MYAPP_VERSION)
So all the version_from_git variables are populated even if no git repo/tag exists.
This would solve the problem, but having a CMake option to override the version could lead to inconsistent builds, so in my opinion is not very safe. |
In case where the library is downloaded as a tarball (eg. Buildroot), the VersionFromGit module would fail resulting in the library not building.
Since there aren't any immidiate fixes, I've hardcoded the VERSION in the CMake project().