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

Update cmake minimum version to 3.24 #12035

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hnakamur
Copy link
Contributor

CMake 3.22.1 on Ubuntu 22.04 causes the following link error for src/iocore/net/test_net.

FAILED: src/iocore/net/test_net                                                                                                                                                                                     : && /usr/bin/c++ -pthread -g -Wl,--allow-multiple-definition src/iocore/net/CMakeFiles/test_net.dir/libinknet_stub.cc.o src/iocore/net/CMakeFiles/test_net.dir/NetVCTest.cc.o src/iocore/net/CMakeFiles/test_net.dir/unit_tests/test_ProxyProtocol.cc.o src/iocore/net/CMakeFiles/test_net.dir/unit_tests/test_SSLSNIConfig.cc.o src/iocore/net/CMakeFiles/test_net.dir/unit_tests/test_YamlSNIConfig.cc.o src/iocore/net/CMakeFiles/test_net.dir/unit_tests/unit_test_main.cc.o -o src/iocore/net/test_net  -Wl,-rpath,/root/trafficserver/build-default/lib/swoc:/root/trafficserver/build-default/lib/yamlcpp  -ldl  src/tscore/libtscore.a  src/proxy/libproxy.a  src/iocore/net/libinknet.a  src/iocore/eventsystem/libinkevent.a  src/iocore/cache/libinkcache.a  src/mgmt/rpc/librpcpublichandlers.a  src/api/libtsapibackend.a  src/shared/liboverridable_txn_vars.a  src/proxy/http/libhttp.a  src/iocore/hostdb/libinkhostdb.a  src/proxy/http2/libhttp2.a  src/proxy/http/remap/libhttp_remap.a  src/proxy/logging/liblogging.a  src/iocore/dns/libinkdns.a  src/proxy/libproxy.a  src/iocore/net/libinknet.a  src/iocore/cache/libinkcache.a  src/mgmt/rpc/librpcpublichandlers.a  src/api/libtsapibackend.a  src/shared/liboverridable_txn_vars.a  src/proxy/http/libhttp.a  src/iocore/hostdb/libinkhostdb.a  src/proxy/http2/libhttp2.a  src/proxy/http/remap/libhttp_remap.a  src/proxy/logging/liblogging.a  src/iocore/dns/libinkdns.a  src/mgmt/rpc/libjsonrpc_protocol.a  /usr/lib/x86_64-linux-gnu/libssl.so  src/iocore/aio/libaio.a  lib/fastlz/libfastlz.a  /usr/lib/x86_64-linux-gnu/libz.so  /usr/lib/x86_64-linux-gnu/liblzma.so  src/iocore/utils/libinkutils.a  src/proxy/hdrs/libhdrs.a  src/iocore/eventsystem/libinkevent.a  src/records/librecords.a  src/tscore/libtscore.a  /usr/lib/x86_64-linux-gnu/libcrypto.so  /usr/lib/x86_64-linux-gnu/libpcre.so  /usr/lib/x86_64-linux-gnu/libresolv.so  /usr/lib/x86_64-linux-gnu/libcap.so  /usr/lib/x86_64-linux-gnu/libhwloc.so  src/tsutil/libtsutil.a  lib/swoc/libswoc-1.5.12.so  lib/yamlcpp/libyaml-cppd.so.0.8.0  -ldl  /usr/lib/x86_64-linux-gnu/libpcre2-8.so && :                                             /usr/bin/ld: src/proxy/http/remap/libhttp_remap.a(RemapConfig.cc.o): warning: relocation against `_ZN7IpAllow14accept_check_pE' in read-only section `.text._ZN7IpAllow17enableAcceptCheckEb[_ZN7IpAllow17enableAcceptCheckEb]'                                                                                                                                                                                                         /usr/bin/ld: src/proxy/http/remap/libhttp_remap.a(AclFiltering.cc.o): in function `src_ip_category_info_t::ask_ip_allow_about_category(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, swoc::_1_5_13::IPAddr const&) const':                                                                                                                                                                    /root/trafficserver/src/proxy/http/remap/AclFiltering.cc:36: undefined reference to `IpAllow::ip_category_contains_addr(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, swoc::_1_5_13::IPAddr const&)'
/usr/bin/ld: src/proxy/http/remap/libhttp_remap.a(RemapConfig.cc.o): in function `remap_load_plugin(char const* const*, int, url_mapping*, char*, int, int, int*, UrlRewrite*)':
/root/trafficserver/src/proxy/http/remap/RemapConfig.cc:949: undefined reference to `isPluginDynamicReloadEnabled()'
/usr/bin/ld: src/proxy/http/remap/libhttp_remap.a(RemapConfig.cc.o): in function `IpAllow::enableAcceptCheck(bool)':
/root/trafficserver/include/proxy/IPAllow.h:398: undefined reference to `IpAllow::accept_check_p'
/usr/bin/ld: /root/trafficserver/include/proxy/IPAllow.h:399: undefined reference to `IpAllow::accept_check_p'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE
collect2: error: ld returned 1 exit status

CMake 3.22.1 on Ubuntu 22.04 causes the following warning when running cmake --preset default:

CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_COMPILE_WARNING_AS_ERROR

CMake 3.24 does not cause the above problems.

@maskit
Copy link
Member

maskit commented Feb 13, 2025

We might want to stick with 3.20. ATS 10 meeting note says 3.20 is the minimum version required. The note doesn't mention where the requirement came from, but I don't think 3.20 is a random version.

@moonchen It seems like the link issue remains on some platforms.

@moonchen
Copy link
Contributor

I would like to upgrade to 3.24 if possible. The circular dependency issues with GNU ld are difficult to work around without link groups. It looks like we've been using CMAKE_COMPILE_WARNING_AS_ERROR in presets for the past two years.

@maskit
Copy link
Member

maskit commented Feb 13, 2025

I suppose CMAKE_COMPILE_WARNING_AS_ERROR is ok because it just doesn't do the job on an old cmake. Although we see the warning, it does not block building ATS. But LINK_GROUP does.

I personally don't mind upgrading to 3.24, but I think it should be proposed on the dev list.

@hnakamur
Copy link
Contributor Author

If we stick with CMake 3.20, we can use a workaround which uses -Wl,--start-group and -Wl,--end-group manually (hnakamur@83784aa).

target_link_libraries(
  test_net
  PRIVATE catch2::catch2
          ts::tscore
          -Wl,--start-group
          ${LINK_GROUP_LIBS}
          -Wl,--end-group
          ts::tsutil
          ts::inkevent
          libswoc::libswoc
)

If we upgrade CMake to 3.24, we might want add an instruction to install CMake >=3.24 from https://github.com/Kitware/CMake/releases to README.

@hnakamur
Copy link
Contributor Author

I found CMAKE_LINK_GROUP_USING_RESCAN_SUPPORTED is not defined on macOS even with CMake >= 3.24.
So special handling for macOS must be kept like below:
master...hnakamur:trafficserver:old_cmake_link_group_fallback

@moonchen
Copy link
Contributor

Thanks for finding the workaround with --start-group. That should help if we can't move to 3.24.

As far as macOS, the rescan behavior is included with the included linker. The else() case is there to avoid an error with duplicate symbols.

This current way is messy, but I hope we can remove the circular dependencies soon so none of this hacking is necessary.

@hnakamur
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants