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

[libtorrent] update to 1.2.12 #15620

Merged
merged 5 commits into from Mar 11, 2021

Conversation

JonLiu1993
Copy link
Member

Fix #15553

Update libtorrent to the latest version 1.2.12

All features are tested successfully in the following triplets:

  • x86-windows
  • x64-windows
  • x64-windows-static

@JonLiu1993 JonLiu1993 added info:internal This PR or Issue was filed by the vcpkg team. category:port-update The issue is with a library, which is requesting update new revision labels Jan 13, 2021
@glassez
Copy link

glassez commented Jan 22, 2021

@JonLiu1993, why this PR is still Draft? Are there any other changes required?

Copy link
Member Author

@JonLiu1993 JonLiu1993 left a comment

Choose a reason for hiding this comment

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

@JonLiu1993, why this PR is still Draft? Are there any other changes required?

CI shows that there is a problem with X64-osx, and I haven't taken care of it yet. I will request a review as soon as possible after processing

ports/libtorrent/portfile.cmake Show resolved Hide resolved
@FranciscoPombal
Copy link
Contributor

@JonLiu1993

Hey, I've handled the last few upgrades of libtorrent in the past: https://github.com/microsoft/vcpkg/pulls?q=is%3Apr+libtorrent+is%3Aclosed+author%3AFranciscoPombal

There is nothing special about 1.2.11 -> 1.2.12 as far as I know that would cause a build error on macOS. Whatever failure that was in the CI, it was likely a spurious error, or maybe caused by something else that maybe has since been fixed. I can't really do better than to speculate now, since the build logs no longer seem to be available. Otherwise, LGTM.

IMO you should take this out of Draft status, rebase on top of the latest master commit to have the CI run again and tag a maintainer to come take a look and merge this sooner rather than later. For such a trivial change, it has sat here for too long already.

@JonLiu1993
Copy link
Member Author

@JonLiu1993

Hey, I've handled the last few upgrades of libtorrent in the past: https://github.com/microsoft/vcpkg/pulls?q=is%3Apr+libtorrent+is%3Aclosed+author%3AFranciscoPombal

There is nothing special about 1.2.11 -> 1.2.12 as far as I know that would cause a build error on macOS. Whatever failure that was in the CI, it was likely a spurious error, or maybe caused by something else that maybe has since been fixed. I can't really do better than to speculate now, since the build logs no longer seem to be available. Otherwise, LGTM.

IMO you should take this out of Draft status, rebase on top of the latest master commit to have the CI run again and tag a maintainer to come take a look and merge this sooner rather than later. For such a trivial change, it has sat here for too long already.

Sorry, I will modify it immediately and let it run again

@FranciscoPombal
Copy link
Contributor

Looks like the build failure persists, unfortunately. I don't have access to macOS hardware, by the way.

Relevant logs:

2021-03-01T03:28:02.0726020Z Starting package 122/128: libtorrent:x64-osx
2021-03-01T03:28:02.0726590Z Building package libtorrent[core]:x64-osx...
2021-03-01T03:28:06.5976050Z CMake Deprecation Warning at scripts/cmake/vcpkg_check_features.cmake:182 (message):
2021-03-01T03:28:06.5979820Z   calling `vcpkg_check_features` without the `FEATURES` keyword has been
2021-03-01T03:28:06.5982570Z   deprecated.
2021-03-01T03:28:06.5983150Z 
2021-03-01T03:28:06.5988640Z       Please add the `FEATURES` keyword to the call.
2021-03-01T03:28:06.5993950Z Call Stack (most recent call first):
2021-03-01T03:28:06.5994980Z   ports/libtorrent/portfile.cmake:19 (vcpkg_check_features)
2021-03-01T03:28:06.5997300Z   scripts/ports.cmake:139 (include)
2021-03-01T03:28:06.5997610Z 
2021-03-01T03:28:06.5997810Z 
2021-03-01T03:28:06.5998760Z -- Downloading https://github.com/arvidn/libtorrent/archive/e3f2b016dcd37a9a6e8a94006c7befcf2cb7bfac.tar.gz -> arvidn-libtorrent-e3f2b016dcd37a9a6e8a94006c7befcf2cb7bfac.tar.gz...
2021-03-01T03:28:06.6000020Z -- Extracting source /Users/vagrant/Data/downloads/arvidn-libtorrent-e3f2b016dcd37a9a6e8a94006c7befcf2cb7bfac.tar.gz
2021-03-01T03:28:06.6000700Z -- Using source at /Users/vagrant/Data/buildtrees/libtorrent/src/cf2cb7bfac-f16ee15c49.clean
2021-03-01T03:28:06.6001100Z CMake Deprecation Warning at scripts/cmake/vcpkg_configure_cmake.cmake:72 (message):
2021-03-01T03:28:06.6001440Z   vcpkg_configure_cmake has been deprecated in favor of vcpkg_cmake_configure
2021-03-01T03:28:06.6001940Z   from the vcpkg-cmake port.
2021-03-01T03:28:06.6002220Z Call Stack (most recent call first):
2021-03-01T03:28:06.6002500Z   ports/libtorrent/portfile.cmake:47 (vcpkg_configure_cmake)
2021-03-01T03:28:06.6003090Z   scripts/ports.cmake:139 (include)
2021-03-01T03:28:06.6003600Z 
2021-03-01T03:28:06.6003770Z 
2021-03-01T03:28:06.6004590Z -- Configuring x64-osx-dbg
2021-03-01T03:28:06.6004920Z CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:105 (message):
2021-03-01T03:28:06.6007980Z     Command failed: /Volumes/data/downloads/tools/cmake-3.19.2-osx/cmake-3.19.2-macos-universal/CMake.app/Contents/bin/cmake /Users/vagrant/Data/buildtrees/libtorrent/src/cf2cb7bfac-f16ee15c49.clean -Ddeprecated-functions=OFF -Dbuild_examples=OFF -Dpython-bindings=OFF -Dbuild_tests=OFF -Dbuild_tools=OFF -Dboost-python-module-name= -Dstatic_runtime= -DPython3_USE_STATIC_LIBS=ON -DCMAKE_MAKE_PROGRAM=/Users/vagrant/Data/downloads/tools/ninja-1.10.1-osx/ninja -DCMAKE_SYSTEM_NAME=Darwin -DBUILD_SHARED_LIBS=OFF -DVCPKG_CHAINLOAD_TOOLCHAIN_FILE=/Volumes/data/work/1/s/scripts/toolchains/osx.cmake -DVCPKG_TARGET_TRIPLET=x64-osx -DVCPKG_SET_CHARSET_FLAG=ON -DVCPKG_PLATFORM_TOOLSET=external -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON -DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=ON -DCMAKE_FIND_PACKAGE_NO_SYSTEM_PACKAGE_REGISTRY=ON -DCMAKE_INSTALL_SYSTEM_RUNTIME_LIBS_SKIP=TRUE -DCMAKE_VERBOSE_MAKEFILE=ON -DVCPKG_APPLOCAL_DEPS=OFF -DCMAKE_TOOLCHAIN_FILE=/Volumes/data/work/1/s/scripts/buildsystems/vcpkg.cmake -DCMAKE_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION=ON -DVCPKG_CXX_FLAGS= -DVCPKG_CXX_FLAGS_RELEASE= -DVCPKG_CXX_FLAGS_DEBUG= -DVCPKG_C_FLAGS= -DVCPKG_C_FLAGS_RELEASE= -DVCPKG_C_FLAGS_DEBUG= -DVCPKG_CRT_LINKAGE=dynamic -DVCPKG_LINKER_FLAGS= -DVCPKG_LINKER_FLAGS_RELEASE= -DVCPKG_LINKER_FLAGS_DEBUG= -DVCPKG_TARGET_ARCHITECTURE=x64 -DCMAKE_INSTALL_LIBDIR:STRING=lib -DCMAKE_INSTALL_BINDIR:STRING=bin -D_VCPKG_ROOT_DIR=/Volumes/data/work/1/s -D_VCPKG_INSTALLED_DIR=/Users/vagrant/Data/installed -DVCPKG_MANIFEST_INSTALL=OFF -DCMAKE_OSX_ARCHITECTURES=x86_64 -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=/Users/vagrant/Data/packages/libtorrent_x64-osx/debug
2021-03-01T03:28:06.6010300Z     Working Directory: /Users/vagrant/Data/buildtrees/libtorrent/x64-osx-dbg
2021-03-01T03:28:06.6010620Z     Error code: 1
2021-03-01T03:28:06.6010860Z     See logs for more information:
2021-03-01T03:28:06.6011400Z       /Users/vagrant/Data/buildtrees/libtorrent/config-x64-osx-dbg-out.log
2021-03-01T03:28:06.6011990Z       /Users/vagrant/Data/buildtrees/libtorrent/config-x64-osx-dbg-err.log
2021-03-01T03:28:06.6012250Z 
2021-03-01T03:28:06.6012460Z Call Stack (most recent call first):
2021-03-01T03:28:06.6012780Z   scripts/cmake/vcpkg_configure_cmake.cmake:334 (vcpkg_execute_required_process)
2021-03-01T03:28:06.6013110Z   ports/libtorrent/portfile.cmake:47 (vcpkg_configure_cmake)
2021-03-01T03:28:06.6013380Z   scripts/ports.cmake:139 (include)
2021-03-01T03:28:06.6013570Z 
2021-03-01T03:28:06.6013740Z 
2021-03-01T03:28:06.6014250Z Error: Building package libtorrent:x64-osx failed with: BUILD_FAILED
2021-03-01T03:28:06.6014840Z Elapsed time for package libtorrent:x64-osx: 4.377 s

Any way to access these files from the Azure CI?

2021-03-01T03:28:06.6011400Z       /Users/vagrant/Data/buildtrees/libtorrent/config-x64-osx-dbg-out.log
2021-03-01T03:28:06.6011990Z       /Users/vagrant/Data/buildtrees/libtorrent/config-x64-osx-dbg-err.log

It would be most convenient.

@JonLiu1993
Copy link
Member Author

@FranciscoPombal ,Please take a look at this:
config-x64-osx-dbg-err.log
config-x64-osx-dbg-out.log

@FranciscoPombal
Copy link
Contributor

@JonLiu1993

Relevant snippet from the logs:

-- Performing Test HAVE_CXX_ATOMICS_WITHOUT_LIB
-- Performing Test HAVE_CXX_ATOMICS_WITHOUT_LIB - Failed
-- Performing Test HAVE_CXX_ATOMICS64_WITHOUT_LIB
-- Performing Test HAVE_CXX_ATOMICS64_WITHOUT_LIB - Failed
-- Performing Test HAVE_CXX_ATOMICS_WITH_LIB
-- Performing Test HAVE_CXX_ATOMICS_WITH_LIB - Failed
-- Performing Test HAVE_CXX_ATOMICS64_WITH_LIB
-- Performing Test HAVE_CXX_ATOMICS64_WITH_LIB - Failed
-- Configuring incomplete, errors occurred!

Here are the relevant lines from libtorrent's CMakeLists.txt:

https://github.com/arvidn/libtorrent/blob/e3f2b016dcd37a9a6e8a94006c7befcf2cb7bfac/CMakeLists.txt#L632

# check if we need to link with libatomic (not needed on MSVC)
if (NOT Windows)
	# TODO: migrate to CheckSourceCompiles in CMake >= 3.19
	include(CheckCXXSourceCompiles)

	set(ATOMICS_TEST_SOURCE [=[
		#include <atomic>
		#include <cstdint>
		std::atomic<int> x{0};
		int main() {
			x.fetch_add(1, std::memory_order_relaxed);
			return 0;
		}
	]=])
	string(REPLACE "std::atomic<int>" "std::atomic<std::int64_t>" ATOMICS64_TEST_SOURCE "${ATOMICS_TEST_SOURCE}")

	check_cxx_source_compiles("${ATOMICS_TEST_SOURCE}" HAVE_CXX_ATOMICS_WITHOUT_LIB)
	check_cxx_source_compiles("${ATOMICS64_TEST_SOURCE}" HAVE_CXX_ATOMICS64_WITHOUT_LIB)
	if((NOT HAVE_CXX_ATOMICS_WITHOUT_LIB) OR (NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB))
		set(CMAKE_REQUIRED_LIBRARIES "atomic")
		check_cxx_source_compiles("${ATOMICS_TEST_SOURCE}" HAVE_CXX_ATOMICS_WITH_LIB)
		check_cxx_source_compiles("${ATOMICS64_TEST_SOURCE}" HAVE_CXX_ATOMICS64_WITH_LIB)
		if ((NOT HAVE_CXX_ATOMICS_WITH_LIB) OR (NOT HAVE_CXX_ATOMICS64_WITH_LIB))
			message(FATAL_ERROR "No native support for std::atomic, or libatomic not found!")
		else()
			message(STATUS "Linking with libatomic for atomics support")
			unset(CMAKE_REQUIRED_LIBRARIES)
			target_link_libraries(torrent-rasterbar PUBLIC atomic)
		endif()
	endif()
endif()

So the question is, why can't AppleClang find the <atomic> header or link with libatomic?
We can easily get this building with vcpkg by just patching this whole check out, but I'd be more interested in figuring out how to fix the underlying issue, if that's possible.

@glassez
Copy link

glassez commented Mar 2, 2021

So the question is, why can't AppleClang find the <atomic> header or link with libatomic?

And why there are no such errors when building qBittorrent CI? Is it really also because of c++17? Maybe try to force c++17 here?

@FranciscoPombal
Copy link
Contributor

So the question is, why can't AppleClang find the <atomic> header or link with libatomic?

And why there are no such errors when building qBittorrent CI? Is it really also because of c++17? Maybe try to force c++17 here?

What makes you think that? I doubt the problem has to do with the C++ version <atomic> is available since C++11, and libtorrent's CMake ensures it is built with at least that version if the compiler defaults to a lower version (this is the case for AppleClang, which defaults to C++98). In the case of qBittorrent, we need to additionally force libtorrent to be built with C++17 mode, not just C++11 because of an ABI incompatibility when linking against libtorrent 1.2.x built with C++11, but that isn't relevant when just trying to build libtorrent itself.

@FranciscoPombal
Copy link
Contributor

FranciscoPombal commented Mar 2, 2021

@glassez

OK, actually I think I know what the problem is. Since AppleClang defaults to C++98, we need to force the atomics test source to be built with C++11.

We need to set CMAKE_REQUIRED_FLAGS to -std=c++11 prior to checking if the atomics test source compiles. That should be enough.

@JonLiu1993 I would suggest trying out something like this (new lines marked with +, it's a pseudo-diff):

# .... other code ....

+      if(APPLE)
+          set(CMAKE_REQUIRED_FLAGS "-std=c++11")
+      endif()
	check_cxx_source_compiles("${ATOMICS_TEST_SOURCE}" HAVE_CXX_ATOMICS_WITHOUT_LIB)
	check_cxx_source_compiles("${ATOMICS64_TEST_SOURCE}" HAVE_CXX_ATOMICS64_WITHOUT_LIB)
	if((NOT HAVE_CXX_ATOMICS_WITHOUT_LIB) OR (NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB))
		set(CMAKE_REQUIRED_LIBRARIES "atomic")
		check_cxx_source_compiles("${ATOMICS_TEST_SOURCE}" HAVE_CXX_ATOMICS_WITH_LIB)
		check_cxx_source_compiles("${ATOMICS64_TEST_SOURCE}" HAVE_CXX_ATOMICS64_WITH_LIB)
		if ((NOT HAVE_CXX_ATOMICS_WITH_LIB) OR (NOT HAVE_CXX_ATOMICS64_WITH_LIB))
			message(FATAL_ERROR "No native support for std::atomic, or libatomic not found!")
		else()
			message(STATUS "Linking with libatomic for atomics support")
			unset(CMAKE_REQUIRED_LIBRARIES)
			target_link_libraries(torrent-rasterbar PUBLIC atomic)
		endif()
	endif()
+      if(APPLE)
+          unset(CMAKE_REQUIRED_FLAGS)
+      endif()

# .... other code ....

If this works, this patch can then be submitted upstream.

EDIT: capitalize Apple -> APPLE

@JonLiu1993 JonLiu1993 marked this pull request as ready for review March 10, 2021 08:39
@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Mar 10, 2021
@ras0219-msft ras0219-msft merged commit eb41906 into microsoft:master Mar 11, 2021
@ras0219-msft
Copy link
Contributor

LGTM, thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libtorrent] update to 1.2.12
5 participants