CMake: use EXCLUDE_FROM_ALL when declaring Fetch for deflate library
Commit af9ef0e made a change in the CMake setup to switch from using FetchContent_Populate() to FetchContent_MakeAvailable() to bring in libdeflate (and Imath) as dependencies. Using FetchContent_MakeAvailable() has the side effect that libdeflate is now being included as part of the build, causing libraries and headers to be built and installed.
The intent with fetching the deflate source is solely to copy select files into OpenEXRCore, so this change adds the EXCLUDE_FROM_ALL option to the call to FetchContent_Declare() to prevent deflate from being included in the build.
Testing this on Mac, I ran a build of the current tip of the main branch (3d34ea0) using these commands:
cd /Users/matt.johnson/Developer/openexr
mkdir build && cd build
cmake -DCMAKE_INSTALL_PREFIX="/Users/matt.johnson/Developer/openexr_inst" -DCMAKE_PREFIX_PATH="/Users/matt.johnson/Developer/openexr_inst" -DCMAKE_BUILD_TYPE=Release -DCMAKE_MACOSX_RPATH=ON -G "Xcode" -DOPENEXR_BUILD_TOOLS=OFF -DOPENEXR_BUILD_EXAMPLES=OFF -DOPENEXR_INSTALL_PKG_CONFIG=OFF -DBUILD_TESTING=OFF -DCMAKE_XCODE_ATTRIBUTE_ONLY_ACTIVE_ARCH=YES -DCMAKE_OSX_ARCHITECTURES=arm64 "/Users/matt.johnson/Developer/openexr"
cmake --build . --config Release --target install -- -j 10
And looking at the build products in the include and lib directories, I see this:
% ls -l /Users/matt.johnson/Developer/openexr_inst/include
total 32
drwxr-xr-x@ 37 matt.johnson staff 1184 Oct 11 09:48 Imath
drwxr-xr-x@ 148 matt.johnson staff 4736 Oct 11 09:48 OpenEXR
-rw-r--r--@ 1 matt.johnson staff 15748 Oct 11 09:47 libdeflate.h
% ls -l /Users/matt.johnson/Developer/openexr_inst/lib
total 10560
drwxr-xr-x@ 5 matt.johnson staff 160 Oct 11 09:48 cmake
-rwxr-xr-x@ 1 matt.johnson staff 963664 Oct 11 09:48 libIex-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 25 Oct 11 09:48 libIex-3_4.99.dylib -> libIex-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 19 Oct 11 09:48 libIex-3_4.dylib -> libIex-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 16 Oct 11 09:48 libIex.dylib -> libIex-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 85088 Oct 11 09:48 libIlmThread-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 31 Oct 11 09:48 libIlmThread-3_4.99.dylib -> libIlmThread-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 25 Oct 11 09:48 libIlmThread-3_4.dylib -> libIlmThread-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 22 Oct 11 09:48 libIlmThread.dylib -> libIlmThread-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 374624 Oct 11 09:48 libImath-3_2.30.3.2.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 27 Oct 11 09:48 libImath-3_2.30.dylib -> libImath-3_2.30.3.2.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 21 Oct 11 09:48 libImath-3_2.dylib -> libImath-3_2.30.dylib
lrwxr-xr-x@ 1 matt.johnson staff 18 Oct 11 09:48 libImath.dylib -> libImath-3_2.dylib
-rwxr-xr-x@ 1 matt.johnson staff 1154256 Oct 11 09:48 libOpenEXR-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 29 Oct 11 09:48 libOpenEXR-3_4.99.dylib -> libOpenEXR-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 23 Oct 11 09:48 libOpenEXR-3_4.dylib -> libOpenEXR-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 20 Oct 11 09:48 libOpenEXR.dylib -> libOpenEXR-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 2388240 Oct 11 09:48 libOpenEXRCore-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 33 Oct 11 09:48 libOpenEXRCore-3_4.99.dylib -> libOpenEXRCore-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 27 Oct 11 09:48 libOpenEXRCore-3_4.dylib -> libOpenEXRCore-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 24 Oct 11 09:48 libOpenEXRCore.dylib -> libOpenEXRCore-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 231248 Oct 11 09:48 libOpenEXRUtil-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 33 Oct 11 09:48 libOpenEXRUtil-3_4.99.dylib -> libOpenEXRUtil-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 27 Oct 11 09:48 libOpenEXRUtil-3_4.dylib -> libOpenEXRUtil-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 24 Oct 11 09:48 libOpenEXRUtil.dylib -> libOpenEXRUtil-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 120928 Oct 11 09:48 libdeflate.0.dylib
-rw-r--r--@ 1 matt.johnson staff 70496 Oct 11 09:48 libdeflate.a
lrwxr-xr-x@ 1 matt.johnson staff 18 Oct 11 09:48 libdeflate.dylib -> libdeflate.0.dylib
drwxr-xr-x@ 4 matt.johnson staff 128 Oct 11 09:48 pkgconfig
In particular, include contains libdeflate.h, and lib contains libdeflate.* libraries.
Applying the change in this PR and re-running the same build, those are no longer being produced and installed:
% ls -l /Users/matt.johnson/Developer/openexr_inst/include
total 0
drwxr-xr-x@ 37 matt.johnson staff 1184 Oct 11 09:50 Imath
drwxr-xr-x@ 148 matt.johnson staff 4736 Oct 11 09:50 OpenEXR
% ls -l /Users/matt.johnson/Developer/openexr_inst/lib
total 10176
drwxr-xr-x@ 4 matt.johnson staff 128 Oct 11 09:50 cmake
-rwxr-xr-x@ 1 matt.johnson staff 963664 Oct 11 09:50 libIex-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 25 Oct 11 09:50 libIex-3_4.99.dylib -> libIex-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 19 Oct 11 09:50 libIex-3_4.dylib -> libIex-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 16 Oct 11 09:50 libIex.dylib -> libIex-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 85088 Oct 11 09:50 libIlmThread-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 31 Oct 11 09:50 libIlmThread-3_4.99.dylib -> libIlmThread-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 25 Oct 11 09:50 libIlmThread-3_4.dylib -> libIlmThread-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 22 Oct 11 09:50 libIlmThread.dylib -> libIlmThread-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 374624 Oct 11 09:50 libImath-3_2.30.3.2.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 27 Oct 11 09:50 libImath-3_2.30.dylib -> libImath-3_2.30.3.2.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 21 Oct 11 09:50 libImath-3_2.dylib -> libImath-3_2.30.dylib
lrwxr-xr-x@ 1 matt.johnson staff 18 Oct 11 09:50 libImath.dylib -> libImath-3_2.dylib
-rwxr-xr-x@ 1 matt.johnson staff 1154256 Oct 11 09:50 libOpenEXR-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 29 Oct 11 09:50 libOpenEXR-3_4.99.dylib -> libOpenEXR-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 23 Oct 11 09:50 libOpenEXR-3_4.dylib -> libOpenEXR-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 20 Oct 11 09:50 libOpenEXR.dylib -> libOpenEXR-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 2388240 Oct 11 09:50 libOpenEXRCore-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 33 Oct 11 09:50 libOpenEXRCore-3_4.99.dylib -> libOpenEXRCore-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 27 Oct 11 09:50 libOpenEXRCore-3_4.dylib -> libOpenEXRCore-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 24 Oct 11 09:50 libOpenEXRCore.dylib -> libOpenEXRCore-3_4.dylib
-rwxr-xr-x@ 1 matt.johnson staff 231248 Oct 11 09:50 libOpenEXRUtil-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 33 Oct 11 09:50 libOpenEXRUtil-3_4.99.dylib -> libOpenEXRUtil-3_4.99.3.4.0.dylib
lrwxr-xr-x@ 1 matt.johnson staff 27 Oct 11 09:50 libOpenEXRUtil-3_4.dylib -> libOpenEXRUtil-3_4.99.dylib
lrwxr-xr-x@ 1 matt.johnson staff 24 Oct 11 09:50 libOpenEXRUtil.dylib -> libOpenEXRUtil-3_4.dylib
drwxr-xr-x@ 3 matt.johnson staff 96 Oct 11 09:50 pkgconfig
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: mattyjams / name: Matt Johnson (fb5e11848ed396d401c1520eb317354ce7d3b61a, 0b6a06d03373ff76098da912891a4ef5f2444619)
Oops, I accidentally pushed a commit to your branch, after cloning your PR here. How is that even possible?!?
I'm having a hard time reproducing your fix. Even with EXCLUDE_FROM_ALL, my build is still installing the deflate files.
The commit I just pushed forces an internal build of libdeflate in the CI so we'll see what it does. It was an oversight that the CI is not validating the behavior of the internal libdeflate.
No problem! I just re-pushed the branch, including what I think is a fix for the problem you're seeing.
I think it ended up being a CMake version issue. Now that I'm paying closer attention to the documentation, it looks like EXCLUDE_FROM_ALL only started having the desired effect in CMake 3.28. I had tested with the latest release (3.30.5), but when I dropped back to a 3.27 version of CMake I started seeing what you did where deflate was still being installed. I ended up addressing that by manually setting the EXCLUDE_FROM_ALL property on the directories that end up getting fetched when CMake is older than 3.28.
I wasn't able to test all the way back to CMake 3.14, which appears to be the current minimum, but I also don't see any other notable "New in ..." notes in the documentation that would be of concern.
One other oddity I noticed is that these CMake functions are actually setting their variables using a case-lowered version of the name you provide them, so the existing check of Deflate_POPULATED should have actually been deflate_POPULATED. I fixed that in a separate commit, and used deflate_... in the new code.
There seems to be yet another problem here, when attempting to build the website source code, to validate that it compiles and links, it's somehow expecting to find libdeflate.a, although I'm still having trouble reproducing that error locally.
There seems to be yet another problem here, when attempting to build the website source code, to validate that it compiles and links, it's somehow expecting to find libdeflate.a, although I'm still having trouble reproducing that error locally.
Thanks @cary-ilm! I was able to reproduce this even still on Mac when I switched to the Unix Makefiles generator and tested some older releases of CMake.
I tried a whole bunch of things to manually set properties on targets and set various policies, and I could manage to get deflate not to build, but I could not find a way around preventing CMake from thinking it needed to install those unbuilt libraries. The slightly unsatisfying, but simplest, solution was simply to keep using FetchContent_Populate() for CMake versions before 3.30, and FetchContent_MakeAvailable() for versions 3.30 and later. I left a bit of a paper trail in the comment to try to explain how I ended up there.
I tested with these versions of CMake: 3.14.0 3.19.3 3.20.2 3.20.3 3.27.9 3.28.0 3.30.5
And confirmed that deflate was not built with any of them, and the deprecation warning that af9ef0e68f6493dc2053c0489f558955e948c9d9 aimed to address still does not appear.
Let me know what you think now! And thanks for your patience with this one. :)
Thanks for the further investigation. That seems entirely reasonable, even if it is a bit messy. I also experimented with several other options but couldn't find anything that worked, so I was starting to come to the same conclusion, to revert to FetchContent_Populate.
This did spur me to add a validation step to our CI to ensure that we don't inadvertently introduce unexpected files into the install, which would have flagged this prior to release.
@mattyjams, thanks for working this out, we're happy to accept the contribution. Will you be able to sign the CLA? If that's too much of a hassle, I'm happy to replicate the change in a fresh PR myself, let me know which you prefer.
If you can get the CLA signed, our project policy is also to require signed commits, as noted in the DCO check. If you amend the commits with the -s option, that will satisfy it. Thanks!
Thanks again, @cary-ilm! I just pushed one more update with those commits signed off. Sorry I missed that earlier!
And I've pinged a few folks internally to try to get myself added to our (Epic Games) corporate CLA. I'll try to keep prodding that along.
@mattyjams, any progress on the legal front? If it looks like it's going to take much longer, I'd like to submit a parallel fix so we can move forward with a new patch release soon. Thanks!
@mattyjams, any progress on the legal front? If it looks like it's going to take much longer, I'd like to submit a parallel fix so we can move forward with a new patch release soon. Thanks!
Sorry for the delay @cary-ilm! The CLA stuff should be resolved now!