[civetweb] download from GIT instead of bundle, bump from 1.16+ to latest master, and define proper CMake target#22731
Conversation
ferdymercury
force-pushed
the
bcivetclean
branch
from
3794b33 to
ddc397d
Compare
ferdymercury
force-pushed
the
bcivetclean
branch
from
ddc397d to
fc5bc89
Compare
ferdymercury
added
the
skip code analysis
ferdymercury
force-pushed
the
bcivetclean
branch
from
fc5bc89 to
e625669
Compare
|
thanks for this sizable effort! |
|
If everything works, and we go with this solution, we should perhaps move the source tarball to the LCG Packages, as all the others. |
Sounds good to me. In that case, it should be downloaded from commit civetweb/civetweb@588860e (no tagged version) and I guess upload it to LCG with name:
|
| if (dozip) | ||
| arg->CompressWithGzip(); | ||
| #endif | ||
| arg->CompressWithGzip(); // TODO: check in request header that gzip encoding is supported |
There was a problem hiding this comment.
As comment says, I had problem with external civetweb on my desktop (OpenSUSE) when trying to request gzip-ed content.
And this feature is not tested by introduced THttpServer tests.
Probably one need to add such test before we merge this PR.
There was a problem hiding this comment.
I will reopen PR to restart CI and see if everything works.
There was a problem hiding this comment.
Thanks for adding the test. It's failing, so at least we have now a reproducer with builtin_civetweb=ON :)
Should I put the ifdef back, or instead investigate why it's crashing with the newer builtin-version of civetweb?
There was a problem hiding this comment.
Tests fails on many platforms - with segfault crash.
This is exactly that I was observing before.
Return #ifdef back will not really help - test enforce check that output is gzipped.
Workaround was working only with web browser.
It correctly handle both situations - when content gzipped or not.
So we need find a reason for the crash - which happens when content gzipped by ROOT methods..
Seems to be civetweb during compilation add some symbols which conflict with ROOT linked libraries.
One need to resolve this symbols conflict - otherwise we will not be able to build civetweb with its own cmake.
There was a problem hiding this comment.
How can it then happen that using Rmemcompress with fixed buffers inside same test does not crash? (Tested locally with snippet above)
There was a problem hiding this comment.
Ok, sorry, I retire what I said. It does not crash if I put the snippet in test_suite.cxx
but it crashes if I put the snippet in THttpCallArg.cxx
So even with static buffers it crashes if it's inside that class.
There was a problem hiding this comment.
I have no idea now - and will not have time to test it before my vacations which starts tomorrow.
Most probably it is combination of several factors.
It is not only linking but probably some civetweb initialization.
It also can be that preallocated buffer for compression is several bytes too short.
It is compilation issue - so one should try to analyze compiler flags which are used by civetweb cmake.
Maybe shared library is problem?
There was a problem hiding this comment.
I tried both with static or shared library and did not see a difference, both crash.
Tried increasing buffer length does not help. It has already a minimum of 512 and the obj is just 105.
Wrt flags for building civetweb, here they are those of my PC on Ubu22:
//Flags used by the C compiler during all build types.
CMAKE_C_FLAGS:STRING= -fdiagnostics-color=always -Wno-implicit-fallthrough -pipe -Wall -W -pthread
//Flags used by the CXX compiler during all build types.
CMAKE_CXX_FLAGS:STRING= -fdiagnostics-color=always -Wno-implicit-fallthrough -Wno-noexcept-type -pipe -Wshadow -Wall -W -Woverloaded-virtual -fsigned-char -fsized-deallocation -pthread
Happy holidays!
There was a problem hiding this comment.
Funny that alma10 arm64 CMAKE_BUILD_TYPE=RelWithDebInfo, builtin_zlib=ON, builtin_zstd passes fine the THttpServer test.
Test Results 21 files 21 suites 3d 3h 23m 2s ⏱️ For more details on these failures, see this check. Results for commit 3cacd57. ♻️ This comment has been updated with latest results. |
ferdymercury
force-pushed
the
bcivetclean
branch
2 times, most recently
from
2c4bfc4 to
a53e137
Compare
ferdymercury
added
skip ci
…e target, and bump from 1.16+ to latest master and define proper CMake target [ci] explicit enable in alma10clang-ninja even if it was getting on implicitly fixes for SSL disable SSL dynamic loading fix openssl include fix builtin ssl name rename builtin name and copy-paste additional variables from xrootd to civetweb avoid cache vars [net] document civetweb builtin mechanism and version link [civetweb] add patches after 1.16 that solve several CVE detect if websockets component is part of systemwide civetweb [cmake] do not error out if build dir is git dir from willcern Create civetweb-marker.diff xdom socket missing in ubu packages see root-project/root-ci-images#116 (comment) enable builtin civetweb on mac and windows since no system version
3 participants
Cleaned-up version of #21947