Skip to content

Conversation

@yaruno
Copy link
Contributor

@yaruno yaruno commented May 14, 2025

Downgrade googletest to 1.16.x as latest one requires C++ 17

samumbach and others added 2 commits May 12, 2025 16:46
Downgrade googletest to 1.16.x as latest one requires C++ 17
@maxsharabayko maxsharabayko added this to the v1.5.5 milestone May 15, 2025
@maxsharabayko maxsharabayko added Type: Maintenance Work required to maintain or clean up the code [build] Area: Changes in build files labels May 15, 2025
@ethouris
Copy link
Collaborator

That didn't seem to work. Might be that USER is not defined.

@yaruno
Copy link
Contributor Author

yaruno commented May 15, 2025

$USER should be defined but this seems to be a git config issue now in the virtual environment, might be better to download the exact brew formula that has the wanted version from homebrew git history as cloning the whole repo as I originally suggested is excessively heavy. I can check this out, but @ethouris do you recall which Gtest version is known to work with this project?

Edit: latest commit installs v1.16

try to download the exact version of google test formula from homebrew history
Copy link
Contributor Author

@yaruno yaruno left a comment

Choose a reason for hiding this comment

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

V1.16.0 is the last Gtest that supports C++ 14

"The 1.16.x branch will be the last to support C++14. Future development will require at least C++17."
https://github.com/google/googletest/releases/tag/v1.16.0

@ethouris
Copy link
Collaborator

The exact versions are defined in the cmake file. Find the definition of GTest installation.

	# Version ranges are only supported with CMake 3.19 or later.
	# Need GTest v1.10 or higher to support GTEST_SKIP.
	if (${CMAKE_VERSION} VERSION_LESS "3.19.0")
		find_package(GTest 1.10)
	else()
		find_package(GTest 1.10...1.12)
	endif()

downgrade googletest to 1.12.1
Copy link
Contributor Author

@yaruno yaruno left a comment

Choose a reason for hiding this comment

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

downgrade googletest to 1.12.1

@ethouris
Copy link
Collaborator

ethouris commented May 15, 2025

LOL I'm afraid that you'll have to merge both changes together - fixed Google test installation in CI and lifting the CMake version.

At least we know from CI that Google Test installation has passed and is accepted. The problem is only with version compat on Mac.

You might do a premature merge of this PR to #3167 in order to test if it fixed the problem. Then we will merge them in this order.

@yaruno
Copy link
Contributor Author

yaruno commented May 15, 2025

LOL I'm afraid that you'll have to merge both changes together - fixed Google test installation in CI and lifting the CMake version.

At least we know from CI that Google Test installation has passed and is accepted. The problem is only with version compat on Mac.

You might do a premature merge of this PR to #3167 in order to test if it fixed the problem. Then we will merge them in this order.

Heh, that seems to be the case. I've merged 3167 to this one. Lets see if it compiles happily now.

@yaruno
Copy link
Contributor Author

yaruno commented May 18, 2025

Unfortunately I'm not able to trigger the workflows, but if @ethouris can trigger it on when you have a moment then we'll see if it's working as intended.

@ethouris
Copy link
Collaborator

Sure!

@yaruno
Copy link
Contributor Author

yaruno commented May 19, 2025

Sure!

Looks like OSX builds now successfully.

@yaruno
Copy link
Contributor Author

yaruno commented May 19, 2025

Any timeline when we'll get this merged to main? Would love to have it as soon as possible to fix my build processes (and to continue fixing node-js wrapper for it https://github.com/Eyevinn/node-srt).

@cl-ment cl-ment merged commit 19113e6 into Haivision:master May 19, 2025
8 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[build] Area: Changes in build files Type: Maintenance Work required to maintain or clean up the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants