[PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

Thu Jul 14 14:21:13 PDT 2022

jasonmolenda added a comment.

In D128465#3653216 <https://reviews.llvm.org/D128465#3653216>, @ckissane wrote:

> In D128465#3653051 <https://reviews.llvm.org/D128465#3653051>, @jasonmolenda wrote:
>
>> In D128465#3652525 <https://reviews.llvm.org/D128465#3652525>, @ckissane wrote:
>>
>>> In D128465#3651025 <https://reviews.llvm.org/D128465#3651025>, @aemerson wrote:
>>>
>>>> I just reverted this in 6e6be5f9504d <https://reviews.llvm.org/rG6e6be5f9504d3de7a5e94bdc3c8ba96d2f6a88a8> because it seems to have broken macOS builds:
>>>>
>>>>   llvm/lib/Support/Compression.cpp:24:10: fatal error: 'zstd.h' file not found
>>>>   #include <zstd.h>
>>>>            ^~~~~~~~
>>>
>>> @aemerson Could you provide the output of your cmake command?
>>> (I can't easily reproduce because I don't have a mac on hand)
>>
>> FWIW I hit this last night on my mac desktop - it looked like I had zstd installed by homebrew (as a dependency on something I installed), and cmake was able to find it in /opt/homebrew (somehow) but when Compression.cpp was built, nothing had added -I/opt/homebrew/include so the header wasn't found.  I hacked the FindZSTD.cmake to not find it, to finish what I was working on before bedtime.  This might not be related to what @aemerson saw though.
>
> @jasonmolenda Thank you for the info, I believe it is similar, could you try: https://reviews.llvm.org/D129786 for me?

I applied the patch from https://reviews.llvm.org/D129786 on a clean checkout, ` cmake ../llvm -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_ASSERTIONS=1 -DLLVM_ENABLE_PROJECTS='llvm;lldb;clang' '-DLLVM_ENABLE_RUNTIMES=libcxx;libcxxabi' -DLLDB_ENABLE_PYTHON=1` (my normal cmake line) and got

  -- Found ZSTD: /opt/homebrew/lib/libzstd.dylib  
  -- Looking for ZSTD_compress
  -- Looking for ZSTD_compress - found

  [...]

  FAILED: lib/Support/CMakeFiles/LLVMSupport.dir/Compression.cpp.o 
  /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/Users/jason/k/llvm/llvm-project/build/lib/Support -I/Users/jason/k/llvm/llvm-project/llvm/lib/Support -I/Users/jason/k/llvm/llvm-project/build/include -I/Users/jason/k/llvm/llvm-project/llvm/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color  -g -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk -std=c++14  -fno-exceptions -fno-rtti -MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/Compression.cpp.o -MF lib/Support/CMakeFiles/LLVMSupport.dir/Compression.cpp.o.d -o lib/Support/CMakeFiles/LLVMSupport.dir/Compression.cpp.o -c /Users/jason/k/llvm/llvm-project/llvm/lib/Support/Compression.cpp
  /Users/jason/k/llvm/llvm-project/llvm/lib/Support/Compression.cpp:24:10: fatal error: 'zstd.h' file not found
  #include <zstd.h>
           ^~~~~~~~
  1 error generated.

  % file /opt/homebrew/lib/libzstd.dylib 
  /opt/homebrew/lib/libzstd.dylib: Mach-O 64-bit dynamically linked shared library arm64
  % head -2 /opt/homebrew/include/zstd.h 
  /*
   * Copyright (c) Yann Collet, Facebook, Inc.
  %

  % grep -i zstd CMakeCache.txt 
  //Use zstd for compression/decompression if available. Can be ON,
  LLVM_ENABLE_ZSTD:STRING=ON
  ZSTD_INCLUDE_DIR:PATH=/opt/homebrew/include
  ZSTD_LIBRARY:FILEPATH=/opt/homebrew/lib/libzstd.dylib
  //Details about finding ZSTD
  FIND_PACKAGE_MESSAGE_DETAILS_ZSTD:INTERNAL=[/opt/homebrew/lib/libzstd.dylib][/opt/homebrew/include][v()]
  //Have symbol ZSTD_compress
  HAVE_ZSTD:INTERNAL=1
  //ADVANCED property for variable: ZSTD_INCLUDE_DIR
  ZSTD_INCLUDE_DIR-ADVANCED:INTERNAL=1
  //ADVANCED property for variable: ZSTD_LIBRARY
  ZSTD_LIBRARY-ADVANCED:INTERNAL=1

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465