Skip to content

Conversation

RReichert
Copy link
Contributor

@RReichert RReichert commented May 6, 2025

Changes

Since we are upgrading the protobuf version, the location for the root CMakeLists.txt has changed, hence the update.

We also updated the swift targets (ex: swift_add_library or swift_add_executable) so that we can now set a global default C++ version with SWIFT_CXX_STANDARD.

Lastly tried to unify the interface between swift_add_test and the rest of the other swift targets since swift_add_test was developed separately to the rest, hence the difference in interface.

These changes will allow us to set the C++ for our Swift targets independently from the third party libraries.

Copy link

sonarqubecloud bot commented May 9, 2025

@RReichert RReichert marked this pull request as ready for review May 13, 2025 13:04
@RReichert RReichert requested a review from a team as a code owner May 13, 2025 13:04
@@ -10,13 +10,14 @@
# WARRANTIES OF MERCHANTABILITY AND/OR FITNESS FOR A PARTICULAR PURPOSE.
#

option(ABSL_PROPAGATE_CXX_STD "Use CMake C++ standard meta features (e.g. cxx_std_14) that propagate to targets that link to Abseil" true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

protobuf now has abseil, hence the need for this. I believe in the very latest version they've removed abseil, but we aren't there yet.

Comment on lines 254 to +257
if (x_CXX_STANDARD)
list(APPEND language_standards_args CXX ${x_CXX_STANDARD})
elseif (SWIFT_CXX_STANDARD)
list(APPEND language_standards_args CXX ${SWIFT_CXX_STANDARD})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here if the caller specified the CXX_STANDARD it prioritizes that over the global variable SWIFT_CXX_STANDARD

@@ -261,7 +261,7 @@ endfunction()

function(swift_add_test target)
set(argOption "INTEGRATION_TEST" "PARALLEL" "POST_BUILD" "UNIT_TEST" "VALGRIND_MEMCHECK")
set(argSingle "COMMENT" "WORKING_DIRECTORY")
set(argSingle "C_STANDARD" "CXX_STANDARD" "COMMENT" "WORKING_DIRECTORY")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can not set the C++ standard used by the test code via the function call

@RReichert RReichert merged commit 361035b into master May 19, 2025
3 checks passed
@RReichert RReichert deleted the rodrigor/upgrade-grpc branch May 19, 2025 00:05
RReichert pushed a commit to swift-nav/albatross that referenced this pull request May 19, 2025
Automated PR by Jenkins. If CI has passed successfully, merge away!



**cmake**  8920f2a3  ->  361035b6
-  361035b6  :  Update protobuf to v25.1 (swift-nav/cmake#183)
- 5bf22f7d : Update CMake minimum required version to 3.13
(swift-nav/cmake#182)


This pull request was created by
https://jenkins.ci.swift-nav.com/job/CI%20Infra/job/submodule-update/20021/.
RReichert added a commit to swift-nav/libsbp that referenced this pull request May 19, 2025
Automated PR by Jenkins. If CI has passed successfully, merge away!



**cmake**  8920f2a3  ->  361035b6
-  361035b6  :  Update protobuf to v25.1 (swift-nav/cmake#183)
- 5bf22f7d : Update CMake minimum required version to 3.13
(swift-nav/cmake#182)


This pull request was created by
https://jenkins.ci.swift-nav.com/job/CI%20Infra/job/submodule-update/20021/.

---------

Co-authored-by: Rodrigo Reichert <[email protected]>
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.

2 participants