[Support] [Debuginfod] Move HTTPClient to Debuginfod library.

[noajshu]

Summary

Following the discussion in D112753, this moves the HTTPClient from Support to Debuginfod library so that tools depending on Support do not automatically depend on Curl as well. This also removes HTTPClient::initialize() and HTTPClient::cleanup() from InitLLVM so these steps should be implemented by user tools instead.

[phosek]

LGTM

llvm/unittests/Debuginfod/HTTPClientTests.cpp

1

This should be updated as well.

[kpn]

BTW, for a future ticket: having curl enabled without first checking to see if it is installed creates builds failures on non-Linux hosts. OK, yes, it can be disabled with a cmake argument, but it would be better if the extra cmake argument wasn't needed. A warning at cmake configure time that says what features will be missing without curl is probably a good idea as well.

[noajshu]

In D115131#3173872, @kpn wrote:

BTW, for a future ticket: having curl enabled without first checking to see if it is installed creates builds failures on non-Linux hosts. OK, yes, it can be disabled with a cmake argument, but it would be better if the extra cmake argument wasn't needed. A warning at cmake configure time that says what features will be missing without curl is probably a good idea as well.

Thanks, the configuration step does check if curl is installed first before trying to build with it. Does this not work for your build? And does D115131 solve the problem?

[noajshu]

Fix header of HTTP Unit tests.

[kpn]

I don't see how this will resolve my issue. I'll start a new thread on llvm-dev. Don't let me block this review.

[noajshu]

In D115131#3174420, @kpn wrote:

I don't see how this will resolve my issue. I'll start a new thread on llvm-dev. Don't let me block this review.

Sounds good, let's debug it there.

[noajshu]

Thanks for the comments. I'm watching out for any buildbot failures on 0e0f1b28fce841b077228fa23356b1ae38844ae2. If there are none then I will commit D115131 later today.

[tstellar]

Moving the dependency to another library doesn't help in the case of DLLVM_BUILD_LLVM_DYLIB=ON, because libLLVMDebuginfod is included in libLLVM.so. Are there still plans to make the libcurl dependency off by default?

[phosek]

In D115131#3183381, @tstellar wrote:

Moving the dependency to another library doesn't help in the case of DLLVM_BUILD_LLVM_DYLIB=ON, because libLLVMDebuginfod is included in libLLVM.so. Are there still plans to make the libcurl dependency off by default?

I think we should consider it, toolchain vendors who want to include debuginfod in their distribution can still enable it manually, but we'll avoid the accidental dependency for everyone else.

[noajshu]

D115500 should set Curl off by default :)

llvm/lib/Debuginfod/CMakeLists.txt

+# Link LibCURL if the user wants it
+if (LLVM_ENABLE_CURL)
+  set(imported_libs ${CURL_LIBRARIES})
+endif()
+
 add_llvm_component_library(LLVMDebuginfod
   Debuginfod.cpp
+  HTTPClient.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Debuginfod
 
+  LINK_LIBS
+  ${imported_libs}
+
   LINK_COMPONENTS
   Support
   )
+
+set(llvm_system_libs ${system_libs})

llvm/lib/Debuginfod/Debuginfod.cpp

 /// file) associated with a build-id from debuginfod servers. If a source file
 /// is to be fetched, its absolute path must be specified in the Description
 /// argument to fetchInfo.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Debuginfod/Debuginfod.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Debuginfod/HTTPClient.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/Caching.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileUtilities.h"
-#include "llvm/Support/HTTPClient.h"
 #include "llvm/Support/xxhash.h"
 
 namespace llvm {
 static std::string uniqueKey(llvm::StringRef S) { return utostr(xxHash64(S)); }
 
 // Returns a binary BuildID as a normalized hex string.
 // Uses lowercase for compatibility with common debuginfod servers.
 static std::string buildIDToString(BuildIDRef ID) {

llvm/lib/Debuginfod/HTTPClient.cpp

-//===-- llvm/Support/HTTPClient.cpp - HTTP client library -------*- C++ -*-===//
+//===-- llvm/Debuginfod/HTTPClient.cpp - HTTP client library ----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 ///
 /// This file defines the methods of the HTTPRequest, HTTPClient, and
 /// BufferedHTTPResponseHandler classes.
 ///
 //===----------------------------------------------------------------------===//
 
-#include "llvm/Support/HTTPClient.h"
+#include "llvm/Debuginfod/HTTPClient.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #ifdef LLVM_ENABLE_CURL
 #include <curl/curl.h>
 #endif

llvm/lib/Support/CMakeLists.txt

if (MSVC)
   set (delayload_flags delayimp -delayload:shell32.dll -delayload:ole32.dll)
 endif()
 
 # Link Z3 if the user wants to build it.
 if(LLVM_WITH_Z3)
   set(system_libs ${system_libs} ${Z3_LIBRARIES})
 endif()
 
-# Link LibCURL if the user wants it
-if (LLVM_ENABLE_CURL)
-  set(system_libs ${system_libs} ${CURL_LIBRARIES})
-endif()
-
 # Override the C runtime allocator on Windows and embed it into LLVM tools & libraries
 if(LLVM_INTEGRATED_CRT_ALLOC)
   if (CMAKE_BUILD_TYPE AND NOT ${LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE}} MATCHES "^(MT|MTd)$")
     message(FATAL_ERROR "LLVM_INTEGRATED_CRT_ALLOC only works with /MT or /MTd. Use LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE} to set the appropriate option.")
   endif()
 
   string(REGEX REPLACE "(/|\\\\)$" "" LLVM_INTEGRATED_CRT_ALLOC "${LLVM_INTEGRATED_CRT_ALLOC}")
 
add_llvm_component_library(LLVMSupport
   FileUtilities.cpp
   FileOutputBuffer.cpp
   FoldingSet.cpp
   FormattedStream.cpp
   FormatVariadic.cpp
   GlobPattern.cpp
   GraphWriter.cpp
   Hashing.cpp
-  HTTPClient.cpp
   InitLLVM.cpp
   InstructionCost.cpp
   IntEqClasses.cpp
   IntervalMap.cpp
   ItaniumManglingCanonicalizer.cpp
   JSON.cpp
   KnownBits.cpp
   LEB128.cpp

llvm/lib/Support/InitLLVM.cpp

 //===-- InitLLVM.cpp -----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/HTTPClient.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include <string>
 
 #ifdef _WIN32
 #include "llvm/Support/Windows/WindowsSupport.h"
#ifdef _WIN32
 
   // GetCommandLineArguments doesn't terminate the vector with a
   // nullptr.  Do it to make it compatible with the real argv.
   Args.push_back(nullptr);
 
   Argc = Args.size() - 1;
   Argv = Args.data();
 #endif
-
-  HTTPClient::initialize();
 }
 
-InitLLVM::~InitLLVM() {
-  HTTPClient::cleanup();
-  llvm_shutdown();
-}
+InitLLVM::~InitLLVM() { llvm_shutdown(); }

llvm/unittests/Debuginfod/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Debuginfod
   )
 
 add_llvm_unittest(DebuginfodTests
+  HTTPClientTests.cpp
   DebuginfodTests.cpp
   )
 
 target_link_libraries(DebuginfodTests PRIVATE LLVMTestingSupport)

llvm/unittests/Debuginfod/DebuginfodTests.cpp

-//===-- llvm/unittest/Support/DebuginfodTests.cpp - unit tests --*- C++ -*-===//
+//===-- llvm/unittest/Support/DebuginfodTests.cpp - unit tests ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Debuginfod/Debuginfod.h"
+#include "llvm/Debuginfod/HTTPClient.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/HTTPClient.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
 // Check that the Debuginfod client can find locally cached artifacts.
 TEST(DebuginfodClient, CacheHit) {

llvm/unittests/Debuginfod/HTTPClientTests.cpp

-//===-- llvm/unittest/Support/HTTPClient.cpp - unit tests -------*- C++ -*-===//
+//===-- llvm/unittest/Debuginfod/HTTPClientTests.cpp - unit tests ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/Support/HTTPClient.h"
+#include "llvm/Debuginfod/HTTPClient.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
 TEST(BufferedHTTPResponseHandler, Lifecycle) {
   BufferedHTTPResponseHandler Handler;

llvm/unittests/Support/CMakeLists.txt

add_llvm_unittest(SupportTests
   FileCollectorTest.cpp
   FileOutputBufferTest.cpp
   FileUtilitiesTest.cpp
   FormatVariadicTest.cpp
   FSUniqueIDTest.cpp
   GlobPatternTest.cpp
   HashBuilderTest.cpp
   Host.cpp
-  HTTPClient.cpp
   IndexedAccessorTest.cpp
   InstructionCostTest.cpp
   ItaniumManglingCanonicalizerTest.cpp
   JSONTest.cpp
   KnownBitsTest.cpp
   LEB128Test.cpp
   LinearPolyBaseTest.cpp
   LineIteratorTest.cpp