[C++] Call find_package(Torch REQUIRED) more than one time in downstream project causes CMake configuration error

[jasjuang]

🐛 Bug

Please see below minimal code to reproduce the problem

CMakeLists.txt

cmake_minimum_required(VERSION 3.11)

project(sample)

find_package(Torch REQUIRED)
find_package(Torch REQUIRED)

add_executable(${PROJECT_NAME} main.cpp)

target_link_libraries(${PROJECT_NAME} torch)

main.cpp

#include <ATen/ATen.h>

int main(){
  return 0;
}

Expected behavior

After

mkdir build
cd build
cmake ..

I expect my downstream cmake configuration to work like usual, but here's the error message I get

CMake Error at /usr/share/cmake/Caffe2/public/mkl.cmake:3 (add_library):
  add_library cannot create imported target "caffe2::mkl" because another
  target with the same name already exists.
Call Stack (most recent call first):
  /usr/share/cmake/Caffe2/Caffe2Config.cmake:109 (include)
  /usr/share/cmake/Torch/TorchConfig.cmake:40 (find_package)
  CMakeLists.txt:6 (find_package)


-- Configuring incomplete, errors occurred!

I understand it's unusual to have the need to call find_package(Torch REQUIRED) twice but calling it twice for other popular libraries like OpenCV, TensorFlow, etc does not result in an error. Can someone please fix this?

Environment

• OS : Ubuntu
• How you installed PyTorch: source
• Build command you used:

cmake -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DBUILD_CUSTOM_PROTOBUF=OFF -DBUILD_PYTHON=OFF -DBUILDING_WITH_TORCH_LIBS=ON -DUSE_SYSTEM_NCCL=ON ..
make -j

• CMake version: 3.15.2
• Python version: 2.7
• CUDA/cuDNN version: 10.0.130
• GPU models and configuration: GeForce GTX TITAN Black

cc @ezyang

[ezyang]

Oof. We probably should guard the relevant lines of code in cmake/public/mkl.cmake so that it doesn't create it the second time. @jasjuang do you think you could cook up a patch? Add me as reviewer.

Also cc @xuhdev if you happen to know what the idiomatic way to solve a problem like this is.

[jasjuang]

@ezyang The current patch I have locally is adding an additional if condition in mkl.cmake

if(NOT TARGET caffe2::mkl)
  add_library(caffe2::mkl INTERFACE IMPORTED)
endif()

It solves the problem but I am not sure whether that's the right way to do it. If you think this is okay, let me know and I will submit a PR.

[xuhdev]

This is a bit weird because, it seems to me, that find_package(Torch) (TorchConfig.cmake) should be pretty minimum and shouldn't trigger the scripts for the build process of PyTorch itself (find_package(Torch) should find a PyTorch installation, which might not be in a source code form). Are you aware of any reason that mkl.cmake is conjured in the first place?

[ezyang]

mkl.cmake came along for the ride when we merged in Caffe2's cmake scripts. I think it's to solve situations where downstream apps also have to link against mkl (e.g., if you're linking statically); in that case, the Torch cmake config has to know something about mkl to get people to link against it.

[xuhdev]

@ezyang If this is case, I would say the best way (and probably the most reliable and idiomatic way) is to store information about the MKL with which PyTorch was linked somewhere (e.g., version.py) and let find_package(Torch) return that information. For this particular case though, I'm not sure what to do; seems to me that guarding TorchConfig.cmake might be the best temporary (although hacky) workaround.

[xuhdev]

Or we just leave it as it is and let the user to guard if they truly have the need to call find_package(Torch) twice.

[jasjuang]

Here's a real-world scenario where find_package(Torch) would need to be called twice. Library A links to Torch and has a find_dependency(Torch) in its config file. Another Library B also links to Torch and has a find_dependency(Torch) in its config file. Now my project depends on both Library A and B. In my CMakeLists there's find_package(LibraryA) and find_package(LibraryB), when I run cmake .. I got the same error message as reported. In fact, this is how I discovered this bug in the first place. I don't think it's a good idea to let the downstream user guard it. It should behave like all other libraries where this kind of problem doesn't exist.

[ezyang]

Yes, let's accept the hack for now. Or if someone wants to redo how we do TorchConfig.cmake packaging, I'd also be down too :)

[jasjuang]

@ezyang please see the PR #25167

[xuhdev]

Can we make the hack simply guard TorchConfig.cmake itself? As this seems to be a "cleaner" guard hacking :)