Augment CMakeLists.txt & Fix generated config file

[laitingsheng]

I hope this simple fix could pass the CI tests (as a follow-up of #6378 and #5602).

Resolves #5863.

[laitingsheng]

Apologies for the misleading conclusion I made in the last PR (#6378). I finally realised that LibPNG and LibJPEG were not statically linked to the TorchVision target. It's necessary to instruct CMake to use FindXXX to search for the correct dynamic libraries in the output CMake configuration files.

Because I believe the actual dynamic libraries in use were installed by conda, adding Python target to the linkage dependency will coincidentally provide the correct search path of shared objects for LibPNG and LibJPEG, thus the original generated configuration file is working anyway.

[datumbox]

@laitingsheng Thank you very much for your patience and tenacity to fix this. Your changes seem good to me. The CI is failing due to network problems, so this seems unrelated. I've manually restarted the failing jobs but if that persists we might have to restart all CI jobs to confirm everything works as expected.

@fmassa @bmanga If you could have also a look on the propose changes, I would appreciate it.

[laitingsheng]

Thank you very much for your patience and tenacity to fix this.

My pleasure!

The last commit should be working correctly now.

[fmassa]

Changes LGTM (although I'm not a CMake expert).

Thanks!

[datumbox]

LGTM, thanks a lot for your work @laitingsheng.

I've pinged a couple of people that have more experience with CMake to let us know what they think. I'll merge immediately after I get their input. I apologise for any delay, please bear with me; it shouldn't take long.

[Nayef211]

Changes LGTM as well!

As a followup item, we noticed that the torchtext build failed when CUDA enabled torch is installed in the build environment (see pytorch/text#1797). The fix involved linking the torchtext.so lib with specific libraries from torch (see this PR) instead of doing find_package(Torch REQUIRED) (which is how torchvision is currently setup). I'm wondering if torchvision build ever ran into a similar failure?

[datumbox]

@Nayef211 Thanks a lot for giving your insights and expertise. We've never tried that, possibly because of lack of expertise working with CMake. Are there any other benefits from linking against specific torch libraries? I would love to understand the pros/cons to perhaps follow up/adopting a similar approach as TorchText.

@laitingsheng Merged, thanks a lot for your contribution!

[laitingsheng]

Merged, thanks a lot for your contribution!

@datumbox My pleasure!

we noticed that the torchtext build failed when CUDA enabled torch is installed in the build environment

@Nayef211 I don't think this is the case for torchvision as torchvision requires CUDA if it's available, which follows what the Torch library requires. I think someone has pointed out that torchtext doesn't need CUDA even if the Torch library in use is compiled and linked against CUDA. If the torch library has been instructed to link against CUDA, the TORCH_LIBRARIES variable will always include the torch_cuda target (which requires CUDA), and I guess this is the root cause of the problem.

[Nayef211]

If the torch library has been instructed to link against CUDA, the TORCH_LIBRARIES variable will always include the torch_cuda target (which requires CUDA), and I guess this is the root cause of the problem.

@laitingsheng thanks for the insight and I think you're spot on here. I wasn't too familiar with how torchvision was handling building with CUDA. If it requires CUDA if it's available then this error shouldn't affect torchvision. 😄