-
Notifications
You must be signed in to change notification settings - Fork 419
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
Replace Travis CI with GitHub Actions #575
Conversation
gtest, libjpeg/libjpeg-turbo, libpng and libtiff provide pkg-config files which make compiling and linking against them easier. This is needed for running CI on macOS with GitHub Actions in a later commit.
OpenGL on macOS was deprecated in macOS 10.14 [1]. Silence the warnings because the OpenGL code won't be replaced anytime soon if at all. [1] https://developer.apple.com/library/archive/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_intro/opengl_intro.html
GoogleTest requires C++14 since 1.13.0 (see release notes [1]). This is required for running CI on macOS with GitHub Actions in a later commit because Homebrew will install GoogleTest 1.15.2 [2]. [1] https://github.com/google/googletest/releases/tag/v1.13.0 [2] https://formulae.brew.sh/formula/googletest
While setting up GitHub Actions, SurfTest.TestHaarWaveletsLargerKernel failed in the following CI environments: - clang version 10.0.0-4ubuntu, Ubuntu 20.04, x86_64 - Ubuntu clang version 14.0.0-1ubuntu1.1, Ubuntu 22.04, x86_64 - Ubuntu clang version 18.1.3 (1ubuntu1), Ubuntu 24.04, x86_64 - Apple clang 14.0.0 (clang-1400.0.29.202), macOS 12, x86_64 - Apple clang 15.0.0 (clang-1500.0.40.1), macOS 13, x86_64 Output of the Ubuntu 20.04 and 22.04 CI runners that failed: ------------------------------------------------------------ [ RUN ] SurfTest.TestHaarWaveletsLargerKernel sfm/gtest_surf.cc:151: Failure Expected equality of these values: 6.0f Which is: 6 dy Which is: 6 [ FAILED ] SurfTest.TestHaarWaveletsLargerKernel (0 ms) Output of the Ubuntu 24.04, macOS 12 and 13 CI runners that failed: ------------------------------------------------------------------- [ RUN ] SurfTest.TestHaarWaveletsLargerKernel sfm/gtest_surf.cc:151: Failure Expected equality of these values: 6.0f Which is: 6 dy Which is: 6.00000048 [ FAILED ] SurfTest.TestHaarWaveletsLargerKernel (0 ms) Fixed by replacing EXPECT_EQ with EXPECT_FLOAT_EQ which verifies that the values are approximately equal within 4 ULPs [1]. [1] https://google.github.io/googletest/reference/assertions.html#floating-point
Behavior of parsing doubles from streams is different in LLVM's libc++ compared to GCC's libstdc++, see [1]. This causes StringTest.StringConversionTest to fail when compiling against LLVM's libc++: [ RUN ] StringTest.StringConversionTest util/gtest_string.cc:48: Failure Expected equality of these values: 10.1234 util::string::convert<double>("10.1234asfd", false) Which is: 0 [ FAILED ] StringTest.StringConversionTest (0 ms) This mainly affects macOS because it defaults to libc++. The behavior of libstdc++ is what makes most sense here because std::stringstream::operator>>() will call std::strtod() internally which will extract 10.1234 from the string. This is fixed by implementing a workaround for libc++. [1] https://www.github.com/llvm/llvm-project/issues/18156
The Travis CI build in this repository no longer works but GitHub provides its own CI/CD infrastructure which is free for public repositories, quote from [1]: "GitHub Actions usage is free for standard GitHub-hosted runners in public repositories [...]" The list of standard GitHub-hosted runners for public repositories can be found at the following URL: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories [1] https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions
Travis CI has been replaced by GitHub Actions.
Apparently, vsprintf() is deprecated on macOS. Fixes the following compiler warning when compiling with Apple clang 14.0.0 (clang-1400.0.29.202) on macOS 12: clang++ -Wall -Wextra -Wundef -pedantic -march=native -funsafe-math-optimizations -fno-math-errno -std=c++14 -g -O3 -pthread -DGL_SILENCE_DEPRECATION=1 -fPIC -I../../libs `pkg-config --cflags libjpeg` `pkg-config --cflags libpng` `pkg-config --cflags libtiff-4` -c -o image_io.o image_io.cc image_io.cc:634:7: warning: 'vsprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Wdeprecated-declarations] ::vsprintf(msg, fmt, ap); ^ /Applications/Xcode_14.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/stdio.h:207:1: note: 'vsprintf' has been explicitly marked deprecated here
Found by Apple clang version 14.0.0 (clang-1400.0.29.202) on macOS 12: bundle_io.cc:743:9: warning: variable 'num_points_3d' set but not used [-Wunused-but-set-variable]
…dNormal() Found by Clang 17.0.6 on Gentoo Linux: patch_optimization.cc:316:17: warning: variable 'row' set but not used [-Wunused-but-set-variable] 316 | std::size_t row = 0; | ^
When compiling on Ubuntu 20.04 with Clang 10.0.0, the following warning shows up multiple times when compiling unit tests:
The warning no longer shows up with newer Clang versions, e.g. Ubuntu 22.04 with Clang 14.0.0, so I assume that this is an issue that has been fixed in Clang. I could modify the build system to detect the specific Clang version to silence the warning but I'm not sure if this is worth the effort because Clang 10.0.0 is rather old and Ubuntu 20.04 standard support ends on April 2025 1. If you want I can look into this or we could fix the issue in a follow-up PR. What do you think? Footnotes |
When compiling on macOS 12/13/14 with Clang, the following warning shows up multiple times during compilation of the tests:
I can also reproduce this on Gentoo Linux with Clang 18.1.8 and GoogleTest 1.15.2. It looks like that this is an issue with GoogleTest and how it determines whether a C++ attribute is supported by the compiler and the chosen C++ standard to compile with, see GoogleTest issue 4605. I see two ways to fix this issue:
What do you think? |
When compiling UMVE on Ubuntu 22.04/24.04 with GCC/Clang, the following warning shows up multiple times:
The problem is that Qt ships its own Judging from a post in the Qt forum, It looks like this issue has been present since at least December 2020. Looking at A possible workaround for this could be the following: diff --git a/libs/ogl/opengl.h b/libs/ogl/opengl.h
index 397388bd72a5..4fb5d95ea67d 100644
--- a/libs/ogl/opengl.h
+++ b/libs/ogl/opengl.h
@@ -27,7 +27,12 @@
#else
# define GL_GLEXT_PROTOTYPES
# include <GL/gl.h>
-# include <GL/glext.h>
+# if !defined(QT_OPENGL_LIB) || (defined(QT_OPENGL_LIB) && !defined(__glext_h_))
+# include <GL/glext.h>
+# if defined(QT_OPENGL_LIB)
+# define __glext_h_ 1
+# endif
+# endif
#endif
#endif /* OGL_OPEN_GL_HEADER */ Unfortunately, |
Agree, probably not worth the effort. |
Let's sit it out. It's just a warning, and only for testing, so not that critical. |
I've seen this warning, too. What concerns me a bit is that the versions are different ( |
First of all, thank you Andre, this PR is great! Very much appreciated. |
When clicking on the chip in README.md that points to |
It seems I had to enable Actions in the project repo settings. Still not sure if things are set up correctly. |
Maybe a new commit has to be pushed to master to trigger a CI run? I just noticed a minor issue with one of my commits in this PR. I'll create a PR for the fix and in the PR we'll see whether the CI stuff is setup correctly. |
Looking at the new PR I just created, everything is setup correctly! 🙂 |
Hi,
this PR replaces the currently broken Travis CI with GitHub Actions. GitHub Actions is GitHub's own CI infrastructure which is available for free for public repositories, quote from 1:
Commits 1 (db7f802) to 5 (3eb318d) are fixes/improvements that pave the way for setting up CI via GitHub Actions.
Commit 6 (048e0bb) is the main change which sets up CI via GitHub Actions and includes as many different build environments as possible:
Commit 7 (1b693c2) removes the obsolete Travis CI YAML file.
Commits 8 (247ea30) to 11 (1014e8f) fix some compiler warnings to reduce noise in the CI logs.
Footnotes
https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#about-billing-for-github-actions ↩