Skip to content
This repository has been archived by the owner on Jul 1, 2023. It is now read-only.

Commit

Permalink
Fix anew PyTorch build integration
Browse files Browse the repository at this point in the history
Summary:
Even after I thought I had fixed this (see D21357992) I was still getting errors in PyTorch's CI. In particular `clang-tidy` was failing with this error (see [here](https://github.com/pytorch/pytorch/pull/37729/checks?check_run_id=640154081)):
```
CMake Error: install(EXPORT "tensorpipe-targets" ...) includes target "tensorpipe" which requires target "uv" that is not in any export set.
```
(I hadn't noticed that before before the agent code was also causing failures in `clang-tidy`)
Note that `clang-tidy` was complaining even though all other jobs were successfully building and running the code. I'm not sure whether it was a false positive...

According to the internet (a Stackoverflow question I cannot find again), the inability to install a target defined in another directory was a limitation of CMake that has been removed only in recent versions, and there was no way around it.

As I was checking how other projects that depended on libuv did to deal with this I stumbled upon uvw (from which our libuv wrappers are inspired) and saw that they were building libuv as position-independent code. This was in fact the original problem I was trying to address by picking the shared version of libuv, so I tried uvw's solution and it seems to work: it makes PyTorch compile and it makes its `clang-tidy` happy.

Reviewed By: beauby

Differential Revision: D21377774

fbshipit-source-id: aa7ca412438e4fc57291b214794318347d36a8bd
  • Loading branch information
lw authored and facebook-github-bot committed May 4, 2020
1 parent 1892340 commit c733678
Showing 1 changed file with 2 additions and 5 deletions.
7 changes: 2 additions & 5 deletions cmake/Finduv.cmake
Expand Up @@ -40,11 +40,8 @@ if(NOT uv_FOUND)
add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/libuv
${PROJECT_BINARY_DIR}/third_party/libuv)

if (BUILD_SHARED_LIBS)
add_library(uv::uv ALIAS uv)
else()
add_library(uv::uv ALIAS uv_a)
endif()
add_library(uv::uv ALIAS uv_a)
set_target_properties(uv_a PROPERTIES POSITION_INDEPENDENT_CODE 1)
endif()

include(FindPackageHandleStandardArgs)
Expand Down

0 comments on commit c733678

Please sign in to comment.