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.

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

de-vendor GTest #11841

Merged
merged 4 commits into from
Aug 25, 2023
Merged

de-vendor GTest #11841

merged 4 commits into from
Aug 25, 2023

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Aug 19, 2023

This requires a vcpkg update form mixxxdj/vcpkg#82

Edit: This update is now included, It does not work standalone.
It includes openssl 3.1.2 and gtest 1.13

@github-actions github-actions bot added the build label Aug 19, 2023
@daschuer daschuer marked this pull request as ready for review August 23, 2023 18:58
@daschuer
Copy link
Member Author

All green finally :-)

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

LGTM. any1 else wanting to sign off on this?

Copy link
Member

@JoergAtGithub JoergAtGithub left a comment

Choose a reason for hiding this comment

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

I tested this locally on Windows 7 and Windows 11, unfortunately it fails on both:

On Windows 7 I get this linker failure:

Linking CXX executable mixxx-test.exe
  FAILED: mixxx-test.exe
  cmd.exe /C "cd . && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=CMakeFiles\mixxx-test.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100190~1.0\x64\mt.exe --manifests  -- C:\PROGRA~2\MICROS~3\2019\COMMUN~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\mixxx-test.rsp  /out:mixxx-test.exe /implib:mixxx-test.lib /pdb:mixxx-test.pdb /version:0.0 /machine:x64 /debug /INCREMENTAL:NO /subsystem:console  /OPT:REF /entry:mainCRTStartup /manifest  && cmd.exe /C "cd /D C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\build\x64__portable && C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe -noprofile -executionpolicy Bypass -file C:/Users/Joerg/source/repos/JoergAtGithub/mixxx/buildenv/mixxx-deps-2.4-x64-windows-d4dab88/scripts/buildsystems/msbuild/applocal.ps1 -targetBinary C:/Users/Joerg/source/repos/JoergAtGithub/mixxx/build/x64__portable/mixxx-test.exe -installedDir C:/Users/Joerg/source/repos/JoergAtGithub/mixxx/buildenv/mixxx-deps-2.4-x64-windows-d4dab88/installed/x64-windows/bin -OutVariable out""
  LINK: command "C:\PROGRA~2\MICROS~3\2019\COMMUN~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\mixxx-test.rsp /out:mixxx-test.exe /implib:mixxx-test.lib /pdb:mixxx-test.pdb /version:0.0 /machine:x64 /debug /INCREMENTAL:NO /subsystem:console /OPT:REF /entry:mainCRTStartup /manifest /MANIFEST /MANIFESTFILE:mixxx-test.exe.manifest" failed (exit code 1120) with the following output:
C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\build\x64__portable\enginebufferscalelineartest.cpp.obj : error LNK2019: unresolved external symbol "private: static void __cdecl testing::Mock::FailUninterestingCalls(unsigned __int64)" (?FailUninterestingCalls@Mock@testing@@CAX_K@Z) referenced in function "protected: virtual void __cdecl `anonymous namespace'::EngineBufferScaleLinearTest::SetUp(void)" (?SetUp@EngineBufferScaleLinearTest@?A0xe7ed0782@@MEAAXXZ)
C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\build\x64__portable\enginebufferscalelineartest.cpp.obj : error LNK2019: unresolved external symbol "private: static void __cdecl testing::Mock::UnregisterCallReaction(unsigned __int64)" (?UnregisterCallReaction@Mock@testing@@CAX_K@Z) referenced in function "public: virtual void * __cdecl testing::StrictMock<class `anonymous namespace'::ReadAheadManagerMock>::`scalar deleting destructor'(unsigned int)" (??_G?$StrictMock@VReadAheadManagerMock@?A0xe7ed0782@@@testing@@UEAAPEAXI@Z)
C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\build\x64__portable\mixxx-test.exe : fatal error LNK1120: 2 unresolved externals
  [11/11] Linking CXX executable mixxx.exe
  ninja: build stopped: subcommand failed.

Build All failed.

I could fix the linking issue, by commenting out enginebufferscalelineartest.cpp in the CMakeLists.txt file. But than many test fails:

The following tests FAILED:
      9 - AutoDJProcessorTest.FullIntroOutro_LongerIntro (Failed)
     10 - AutoDJProcessorTest.FullIntroOutro_LongerOutro (Failed)
     11 - AutoDJProcessorTest.FadeAtOutroStart_LongerIntro (Failed)
     12 - AutoDJProcessorTest.FadeAtOutroStart_LongerOutro (Failed)
     13 - AutoDJProcessorTest.TransitionTimeLoadedFromConfig (Failed)
     14 - AutoDJProcessorTest.DecksPlayingWarning (Failed)
     15 - AutoDJProcessorTest.Decks34PlayingWarning (Failed)
     16 - AutoDJProcessorTest.QueueEmpty (Failed)
     17 - AutoDJProcessorTest.EnabledSuccess_DecksStopped (Failed)
     18 - AutoDJProcessorTest.EnabledSuccess_DecksStopped_TrackLoadFails (Failed)
     19 - AutoDJProcessorTest.EnabledSuccess_DecksStopped_TrackLoadFailsRightDeck (Failed)
     20 - AutoDJProcessorTest.EnabledSuccess_PlayingDeck1 (Failed)
     21 - AutoDJProcessorTest.EnabledSuccess_PlayingDeck1_TrackLoadFailed (Failed)
     22 - AutoDJProcessorTest.EnabledSuccess_PlayingDeck2 (Failed)
     23 - AutoDJProcessorTest.EnabledSuccess_PlayingDeck2_TrackLoadFailed (Failed)
     24 - AutoDJProcessorTest.EnabledDisabledSuccess (Failed)
     25 - AutoDJProcessorTest.FadeToDeck1_LoadOnDeck2_TrackLoadSuccess (Failed)
     26 - AutoDJProcessorTest.FadeToDeck1_LoadOnDeck2_TrackLoadFailed (Failed)
     27 - AutoDJProcessorTest.FadeToDeck2_LoadOnDeck1_TrackLoadSuccess (Failed)
     28 - AutoDJProcessorTest.FadeToDeck2_LoadOnDeck1_TrackLoadFailed (Failed)
     29 - AutoDJProcessorTest.FadeToDeck2_Long_Transition (Failed)
     30 - AutoDJProcessorTest.FadeToDeck2_Pause_Transition (Failed)
     31 - AutoDJProcessorTest.FadeToDeck2_SeekEnd (Failed)
     32 - AutoDJProcessorTest.FadeToDeck2_SeekBeforeTransition (Failed)
     33 - AutoDJProcessorTest.TrackZeroLength (Failed)
    158 - ControlObjectScriptTest.CompressingProxyCompareCount1 (Failed)
    159 - ControlObjectScriptTest.CompressingProxyCompareValue1 (Failed)
    160 - ControlObjectScriptTest.CompressingProxyCompareCount2 (Failed)
    161 - ControlObjectScriptTest.CompressingProxyCompareValue2 (Failed)
    162 - ControlObjectScriptTest.QueuedCompareCount2 (Failed)
    163 - ControlObjectScriptTest.QueuedCompareValue2 (Failed)
    164 - ControlObjectScriptTest.CompressingProxyCompareCountMulti (Failed)
    165 - ControlObjectScriptTest.CompressingProxyCompareValueMulti (Failed)
    166 - ControlObjectScriptTest.CompressingProxyMultiConnection (Failed)
    167 - ControlObjectScriptTest.QueuedFallbackMultiConnection (Failed)
    168 - ControlObjectScriptTest.CompressingProxyManyEvents (Failed)
    259 - EngineMasterTest.SingleChannelOutputWorks (Failed)
    260 - EngineMasterTest.SingleChannelPFLOutputWorks (Failed)
    261 - EngineMasterTest.TwoChannelOutputWorks (Failed)
    262 - EngineMasterTest.TwoChannelPFLOutputWorks (Failed)
    263 - EngineMasterTest.ThreeChannelOutputWorks (Failed)
    264 - EngineMasterTest.ThreeChannelPFLOutputWorks (Failed)
    498 - MusicBrainzRecordingsTaskTest.ClinetSideTimeout (Failed)
    499 - MusicBrainzRecordingsTaskTest.RespodsEmpty (Failed)
    512 - PortMidiControllerTest.OpenClose (Failed)
    513 - PortMidiControllerTest.WriteShort (Failed)
    514 - PortMidiControllerTest.WriteSysex (Failed)
    515 - PortMidiControllerTest.WriteSysex_Malformed (Failed)
    516 - PortMidiControllerTest.Poll_Read_Basic (Failed)
    517 - PortMidiControllerTest.Poll_Read_SysExWithRealtime (Failed)
    518 - PortMidiControllerTest.Poll_Read_SysEx (Failed)
    519 - PortMidiControllerTest.Poll_Read_SysExInterrupted_FollowedByNormalMessage (Failed)
    520 - PortMidiControllerTest.Poll_Read_SysExInterrupted_FollowedBySysExMessage (Failed)
    521 - PortMidiControllerTest.Poll_Read_SysEx_BufferOverflow (Failed)

On Windows 11 the following test runs into a stack overflow now (with plain 2.4 only 4 tests on Windows 11 fail (due to #11094) and on Windows 7 every test passes):

Exception thrown at 0x00007FF91DDAB325 (Qt5Cored.dll) in mixxx-test.exe: 0xC00000FD: Stack overflow (parameters: 0x0000000000000001, 0x000000BDE5203FA8).
unknown file: error: SEH exception with code 0xc00000fd thrown in the test body.
C:\Users\Joerg.WORLDWARTWEB\source\repos\JoergAtGithub\mixxx\src\test\controlobjectscripttest.cpp(259): error: Actual function call count doesn't match EXPECT_CALL(*coScript2, slotValueChanged(42.0, _))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
C:\Users\Joerg.WORLDWARTWEB\source\repos\JoergAtGithub\mixxx\src\test\controlobjectscripttest.cpp(256): error: Actual function call count doesn't match EXPECT_CALL(*coScript1, slotValueChanged(kMaxNumOfRecursions, _))...
         Expected: to be called once
           Actual: never called - unsatisfied and active

Since this is a test of the maximum recursions limit, it seems plaussible, that a new gtest version with more instrumentation code brings it over the edge.

Since we are a few days for the release, please retarget the GTest update/devendoring to Main. There only the last fail is an issue, because we plan to drop Windows 7 support for 2.5 anyway, due to the Qt6 update. But for 2.4 point release we need to maintain the testabilty.

@daschuer
Copy link
Member Author

Did you update your vcpkg environment?

@JoergAtGithub
Copy link
Member

Sure, I tested full buildenv and release buildenv on Windows11 and only full buildenv on Windows 7.

@daschuer
Copy link
Member Author

The windows 7 failure should be a cmake issue. It looks like it finds the wrong installation.
Can you please provide the CMakeCache.txt from the build directory?

@daschuer
Copy link
Member Author

Does it find the old GTest in build/lib/googletest? Please remove that directory along with all libgtest and libgmock.

@JoergAtGithub
Copy link
Member

You're right. There was the old lib/googletest existing in my Mixxx working copy on Windows 7.
I deleted it, rebuilt everything and all tests were Pass. Sorry for the false alarm.

@JoergAtGithub
Copy link
Member

The Windows 11 test fail is not that critical for me, since I can explain it, and it doesn't happen on CI.

@JoergAtGithub JoergAtGithub merged commit aa4b9d5 into mixxxdj:2.4 Aug 25, 2023
13 checks passed
@JoergAtGithub
Copy link
Member

Thank you!

@Swiftb0y
Copy link
Member

Thank you @JoergAtGithub for doing the manual test and digging into the issue. 👍

@fwcd
Copy link
Member

fwcd commented Aug 28, 2023

Would be great to have a 2.4 -> main merge again.

Along with #11880 that would be the final piece of the puzzle in getting the main branch to build with Qt 6. Currently, I get a bunch of linker errors when using a fresh vcpkg environment since the main branch still has the vendored gtest which apparently clashes with vcpkg's gtest.

@ronso0
Copy link
Member

ronso0 commented Aug 29, 2023

I'm running into trouble building 2.4 after this was merged.
What piece am I missing?

  • build failed with
CMake Error at CMakeLists.txt:1923 (find_package):
  Could not find a package configuration file provided by "GTest" with any of
  the following names:

    GTestConfig.cmake
    gtest-config.cmake

  Add the installation prefix of "GTest" to CMAKE_PREFIX_PATH or set
  "GTest_DIR" to a directory containing one of the above files.  If "GTest"
  provides a separate development package or SDK, be sure it has been
  installed.

so I ran sudo /tools/debian_buildenv.sh setup which installed libgtest-dev and libgmock-dev, so now I have these and googletest, all version 1.10.0

  • configured cmake, got the same error message as above
  • cleared the build dir, made sure all packages are up to date, configured cmake, got the same error message as above
  • don't find GTestConfig.cmake or gtest-config.cmake on my system

@uklotzde
Copy link
Contributor

Finally a patchless RPM Fusion build without any issues*: https://koji.rpmfusion.org/koji/buildinfo?buildID=26656

*Except some disabled, unreliable tests.

@Holzhaus
Copy link
Member

@ronso0 which OS are you running? Maybe Debian ships a broken version of gtest without the CMake config file?

@ronso0
Copy link
Member

ronso0 commented Aug 29, 2023

I'm on Ubuntu 20.04, got googletest from the Universe repo.
I do have /usr/src/googletest/googletest/cmake/Config.cmake.in though 🤷‍♂️

dpkg-query -L googletest | grep cmake
/usr/src/googletest/googlemock/cmake
/usr/src/googletest/googlemock/cmake/gmock.pc.in
/usr/src/googletest/googlemock/cmake/gmock_main.pc.in
/usr/src/googletest/googletest/cmake
/usr/src/googletest/googletest/cmake/Config.cmake.in
/usr/src/googletest/googletest/cmake/gtest.pc.in
/usr/src/googletest/googletest/cmake/gtest_main.pc.in
/usr/src/googletest/googletest/cmake/internal_utils.cmake
/usr/src/googletest/googletest/cmake/libgtest.la.in

@Holzhaus
Copy link
Member

Yup, that looks broken. It should contain a GTestConfig.cmake file, like it does in 22.04.

@ronso0
Copy link
Member

ronso0 commented Aug 29, 2023

I posted the wrong output, but libgtest-dev doesn't contain the cmake dir at all.
https://packages.ubuntu.com/jammy/amd64/libgtest-dev/filelist
vs
https://packages.ubuntu.com/focal/amd64/libgtest-dev/filelist

meeeh..

@Holzhaus
Copy link
Member

If you don't want to upgrade: libgtest-dev from jammy only contains header files, cmake files and static libs. You could try to install the jammy package on focal...

@ronso0
Copy link
Member

ronso0 commented Aug 29, 2023

Yeah, will try that. The ppa build for 20.04 is also broken, no?

Anyway, I filed https://bugs.launchpad.net/ubuntu/+source/googletest/+bug/2033438

@Holzhaus
Copy link
Member

Didn't we drop support anyway? If I read our minimum requirements policy correctly, 22.04 should be minimum, or am I missing something?

@ronso0
Copy link
Member

ronso0 commented Aug 29, 2023

The ppa still builds for 20.04
I don't mind since I could work around it.

Anyhow, 2.4-beta builds fail for all versions https://launchpad.net/~mixxx/+archive/ubuntu/mixxxbetas/+packages
@daschuer can you take a look?

@ronso0
Copy link
Member

ronso0 commented Aug 30, 2023

For the record:
on Ubuntu 20.04 configure didn't work out after just adding the missing cmake files from the 22.04 package.

I removed the packages libgmock-dev, libgtest-dev and googletest and installed the 22.04 versions from https://launchpad.net/ubuntu/+source/googletest (order: googletest, libgmock, libgtest)

Building now fails with

Scanning dependencies of target mixxx-lib
make[3]: *** No rule to make target '/mixxx_source/lib/googletest/googletest/include/gtest/gtest_prod.h',
needed by 'CMakeFiles/mixxx-lib.dir/src/analyzer/analyzerbeats.cpp.o'.  Stop.

There is no /lib/googletest/ dir in the source tree, but I have
/usr/include/gtest/gtest_prod.h and
/usr/src/googletest/googletest/include/gtest/gtest_prod.h

Any help is appreciated.

Upgrading to 22.04 is no option currently (requires a fresh install since Ubuntu Studio uses KDE desktop) so I will try to revert this PR in order to be able to build 2.4 again.

@ronso0
Copy link
Member

ronso0 commented Aug 30, 2023

Okay, after clearing the build and reconfiguring multiple times I can build 2.4 again (with the gtest-related packages from 22.04).
Maybe a caching issue? 🤷‍♂️

@daschuer daschuer deleted the gtest branch September 6, 2023 05:38
@daschuer daschuer added this to the 2.4.0 milestone Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants