-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
GHA: add NetBSD, OpenBSD, FreeBSD/arm64 and OmniOS jobs #13583
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
Conversation
Ref: https://github.com/curl/curl/actions/runs/9033005218/job/24822356703?pr=13583#step:3:421 edit: The complete list:
Ref: https://github.com/curl/curl/actions/runs/9301034519/job/25598321217?pr=13832 |
The build looked green anyway so I think this is a warning we can just ignore? |
It seems one of those OpenBSD-specific extra security warnings. Could explain why it doesn't trip on |
Right. strlcpy is not a portable function anyway so we cannot easily switch to it universally. Our existing uses of
Agreed. |
``` ../../lib/ldap.c: In function 'ldap_do': ../../lib/ldap.c:380:11: error: unused variable 'ldap_ca' [-Werror=unused-variable] 380 | char *ldap_ca = conn->ssl_config.CAfile; | ^~~~~~~ ../../lib/ldap.c:379:9: error: unused variable 'ldap_option' [-Werror=unused-variable] 379 | int ldap_option; | ^~~~~~~~~~~ ``` Ref: https://github.com/curl/curl/actions/runs/9033564377/job/24824192730#step:3:6059 Ref: curl#13583 Closes #xxxxx
Running tests in these env is risky, and it's done at this point for FreeBSDs and OmniOS. The latter had this fallout:
Ref: https://github.com/curl/curl/actions/runs/9034968328/job/24828726396#step:3:10013 Not much clue here: Both are static test scripts. |
Both depending on a C preprocessor |
We already have a FreeBSD 14.0 on Cirrus. Is there any reason to have this one, too? |
``` ../../lib/ldap.c: In function 'ldap_do': ../../lib/ldap.c:380:11: error: unused variable 'ldap_ca' [-Werror=unused-variable] 380 | char *ldap_ca = conn->ssl_config.CAfile; | ^~~~~~~ ../../lib/ldap.c:379:9: error: unused variable 'ldap_option' [-Werror=unused-variable] 379 | int ldap_option; | ^~~~~~~~~~~ ``` Ref: https://github.com/curl/curl/actions/runs/9033564377/job/24824192730#step:3:6059 Ref: #13583 Closes #13588
This method supports other BSDs and OmniOS, along with arm64 support. Also multiple versions of these OSes. The configuration is more familiar, being GHA. It demonstrates that these platforms can be built on GHA, which I think we haven't done yet in curl. This is based on these two projects, with more info about capabilities under their pages:
We can keep Cirrus CI as-is, and drop the x86_64 job from this workflow. Cirrus CI seems to complete faster. [DONE] |
`ENABLE_DEBUG=ON` enables curl-level debug features. Building and running the curl test suite requires setting this option. Before this patch, while building the `testdeps` target with debug features on with the default ('Release') CMake config, the build stopped with this linker error: ``` ld: CMakeFiles/unit1395.dir/unit1395.c.o: in function `test': unit1395.c:(.text+0x1a0): undefined reference to `dedotdotify' A failure has been detected in another branch of the parallel make ``` Ref: https://github.com/curl/curl/actions/runs/9037287098/job/24835990826#step:3:2483 It happened because `dedotdotify` compiled as a static function into `libcurlu` due to the undefined `DEBUGBUILD` macro. Then `unit1395` failed to link it. Even though the build requires the `DEBUGBUILD` macro, our CMake logic defined it when building with CMake's 'Debug' configuration only. That configuration is not required to build or run the test suite, nor to use curl-level debug features. This patch fixes this by always defining `DEBUGBUILD` when setting `ENABLE_DEBUG=ON`. Decoupling this custom option from the selected CMake Release/Debug configuration. This change may also allow dropping Debug mode in AppVeyor CMake builds, possibly making them build faster and run smoother. Also delete the internal variable `ENABLE_CURLDEBUG` which was a copy of `ENABLE_DEBUG`. Use the latter everywhere instead. Ref: curl#13583 Closes #xxxxx
All clear (with
Ref: https://github.com/curl/curl/actions/runs/9042288591/job/24855020568?pr=13583#step:3:9209 edit: failures are consistent for 2 runs. They seem to be TFTP related, except 1167, which was failing for OmniOS too. edit: Speaking of TFTP and OpenBSD, it seems the result contains the content twice:
It seems the transfer is executed twice. |
FTP: ``` TESTFAIL: These test cases failed: 148 2000 ``` Ref: https://github.com/curl/curl/actions/runs/9132619802/job/25114339271#step:3:19074
GSS API cannot be found with either heimdal and heimdal-libs packages
These new tests are ready to merge. They also might need tweaking if they turn out to be unstable or too much load. The autotools job for FreeBSD is slow, but left it there to match Cirrus CI (but using arm64 here). The Cirrus CI build configuration is also matched, in case we'd want to migrate it (its runner is a little flaky) to GHA. For NetBSD and OpenBSD it's not possible to uninstall system curl (it's required by cmake), so I used the There were various small issues that I stopped pursuing, e.g. I failed to enable GSSAPI/heimdal on OpenBSD and OmniOS. Also libidn2 on OmniOS. It's also kind of weird how autotools picks up dependencies automatically on some platforms, but requires the Tests requiring python (and packages), stunnel, nghttp2 server are not run. These components are installed for FreeBSD (based on Cirrus CI), but all tests are skipped on that platform due to emulated CPU performance (but they do work, or at least they did at one point while working on this). For the remaining platforms it's a fiddly task and did not end up finishing installing them. (It's also a maintenance burden due to versioned package names.) Then there is the TFTP failures on OpenBSD. Tests are run now, but results ignored as a workaround. |
I plan to merge this today. If the new jobs turn out to be more pain than gain, I'm perfectly fine with tuning and/or deleting any/all of them. If it turns out to be working well, we might drop Cirrus CI in favour of a new GHA job in this workflow. |
We do it in Cirrus CI, but for some platform it's not possible to delete it and tests work anyway. The test runner also runs `../src/curl` by default, which is always the one freshly built. The runner may also need a system curl to talk to APIs when needed. Also: - stop setting `CURL` env. This isn't picked up by the runners, and works out of the box anyway. - quote an option just in case. Follow-up to 90e644f curl#13583 Closes curl#13765
We do it in Cirrus CI, but for some platforms it's not possible to delete it and tests work anyway. The test runner also runs `../src/curl` by default, which is always the one freshly built. The runner may also need the system curl to talk to APIs when needed. Also: - stop setting `CURL` env. This isn't picked up by the runners, and works out of the box anyway. - quote an option just in case. Follow-up to 90e644f #13583 Closes #13765
``` TESTFAIL: These test cases failed: 3017 ``` Ref: https://github.com/curl/curl/actions/runs/9223543272/job/25376999226?pr=13759#step:3:16326 Ref: https://github.com/curl/curl/actions/runs/9230183764/job/25397883193?pr=13766#step:3:16345 Ref: #13583 (comment)
`ENABLE_DEBUG=ON` enables curl-level debug features. Building and running the curl test suite requires setting this option. Before this patch, while building the `testdeps` target with debug features on with the default ('Release') CMake config, the build stopped with this linker error: ``` ld: CMakeFiles/unit1395.dir/unit1395.c.o: in function `test': unit1395.c:(.text+0x1a0): undefined reference to `dedotdotify' A failure has been detected in another branch of the parallel make ``` Ref: https://github.com/curl/curl/actions/runs/9037287098/job/24835990826#step:3:2483 It happened because `dedotdotify` compiled as a static function into `libcurlu` due to the undefined `DEBUGBUILD` macro. Then `unit1395` failed to link it. Even though the build requires the `DEBUGBUILD` macro, our CMake logic defined it when building with CMake's 'Debug' configuration only. That configuration is not required to build or run the test suite, nor to use curl-level debug features. This patch fixes this by always defining `DEBUGBUILD` when setting `ENABLE_DEBUG=ON`. Decoupling this custom option from the selected CMake Release/Debug configuration. This change may also allow dropping Debug mode in AppVeyor CMake builds, possibly making them build faster and run smoother. Ref: curl#13583 Closes curl#13592
Before this patch `ENABLE_DEBUG=ON` always enabled the TrackMemory (aka `ENABLE_CURLDEBUG=ON`) feature, but required the `Debug` CMake configration to actually enable curl debug features (aka `-DDEBUGBUILD`). Curl debug features do not require compiling with C debug options. This also made enabling debug features unintuitive and complicated to use. Due to other issues (subject to PR #13694) it also caused an error in default (and `Release`/`MinSizeRel`/`RelWithDebInfo`) configs, when building the `testdeps` target: ``` ld: CMakeFiles/unit1395.dir/unit1395.c.o: in function `test': unit1395.c:(.text+0x1a0): undefined reference to `dedotdotify' ``` Ref: https://github.com/curl/curl/actions/runs/9037287098/job/24835990826#step:3:2483 Fix it by always defining `DEBUGBUILD` when setting `ENABLE_DEBUG=ON`. Decoupling this option from the selected CMake configuration. Note that after this patch `ENABLE_DEBUG=ON` unconditionally enables curl debug features. These features are insecure and unsuited for production. Make sure to omit this option when building for production in default, `Release` (and other not-`Debug`) modes. Also delete a workaround no longer necessary in GHA CI jobs. Ref: 1a62b6e (2015-03-03) Ref: #13583 Closes #13592
MQTT / OmniOS: ``` TESTFAIL: These test cases failed: 1190 1198 3017 ``` Ref: https://github.com/curl/curl/actions/runs/9258522297/job/25468730731?pr=13694#step:3:10251 MQTT / OmniOS: ``` TESTFAIL: These test cases failed: 1194 2200 2203 2205 ``` Ref: https://github.com/curl/curl/actions/runs/9150523540/job/25155409832#step:3:10233 FTP / OmniOS: ``` TESTFAIL: These test cases failed: 1096 ``` Ref: https://github.com/curl/curl/actions/runs/9150702711/job/25155793948#step:3:10247 FTP / OmniOS: ``` TESTFAIL: These test cases failed: 381 ``` Ref: https://github.com/curl/curl/actions/runs/9163863822/job/25193897640#step:3:10230 FTP / OmniOS: ``` TESTFAIL: These test cases failed: 340 ``` Ref: https://github.com/curl/curl/actions/runs/9233804752/job/25406671742?pr=13771#step:3:10245 Ref: #13583 (comment)
Add these jobs to GHA:
no parallelism (was flaky sometimes)
with python, -j8, TFTP results ignored due to TFTP tests fail on OpenBSD (ci/GHA) #13623.
(Tests disabled for arm64, because they are slow. It's available for
x86_64 with python, -j12.)
Configuration matches our existing Cirrus CI one.
All build with websockets and examples.
Closes #13583
ENABLE_DEBUG=ON
to always set-DDEBUGBUILD
#13592 (or a variant) landed.cpp
work on OmniOS. [FIXED by symlinkingcpp
togcpp
.]