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.

Sign up for GitHub

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

fix(cpp): Actually use the VERSION file #376

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+22 −10
Open

fix(cpp): Actually use the VERSION file #376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
865ffb1
fix(cpp): Actually use the VERSION file
musicinmybrain Feb 25, 2025
757833a
Changelog entry for PR #376
musicinmybrain Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ This document is formatted according to the principles of [Keep A CHANGELOG](htt

## [Unreleased]

### Fixed
- [cpp] Actually use the VERSION file ([#376](https://github.com/cucumber/gherkin/pull/376))

## [32.0.0] - 2025-02-17
### Changed
- [.NET] Reduce NuGet size by only targeting .NET Standard 2.0
Expand Down
25 changes: 17 additions & 8 deletions cpp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
###
#
# Project
# name and version
#
###
cmake_minimum_required(VERSION 3.12 FATAL_ERROR)
set(VERSION 30.0.4)
Copy link
Contributor

Choose a reason for hiding this comment

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

Question) Is this line correct, shouldn't this be feeding from the actual version stored in cpp/VERSION

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am no CMake expert, so I simply imitated what #328 did for c/.

This value in this initial set is basically a bogus placeholder and – given the existence of a cpp/VERSION file – it is replaced in the following lines:

if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/VERSION")
    file(STRINGS "VERSION" LINES)
    list(GET LINES 0 VERSION)
endif()

I did verify that this happens in practice, but I did not investigate alternatives, like using something more obviously bogus as an initial value for the set, or perhaps somehow initializing VERSION directly from the VERSION file in the first place.

If we do change the implementation here, we should adjust cpp/CMakeLists.txt to match.

@chybz, do you have any thoughts on this?


project(cucumber_gherkin VERSION 30.0.4 LANGUAGES C CXX)
if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/VERSION")
file(STRINGS "VERSION" LINES)
list(GET LINES 0 VERSION)
endif()

# Crude Semver parsing
if("${VERSION}" MATCHES "^([^\\.]+)\\.([^\\.]+)\\.([^\\.]+)$")
set(VER_MAJOR ${CMAKE_MATCH_1})
set(VER_MINOR ${CMAKE_MATCH_2})
set(VER_PATCH ${CMAKE_MATCH_3})
else()
message(FATAL_ERROR "unable to parse version: ${VERSION}")
endif()

cmake_minimum_required(VERSION 3.12 FATAL_ERROR)
project(cucumber_gherkin VERSION ${VERSION} LANGUAGES C CXX)

###
#
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/lib/gherkin/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ set_target_properties(
cucumber_gherkin_lib
PROPERTIES
CXX_STANDARD 20
VERSION 30.0.4
SOVERSION 30
VERSION ${VERSION}
SOVERSION ${VER_MAJOR}
EXPORT_NAME gherkin
OUTPUT_NAME cucumber_gherkin
)