Fixing the build and install trees

[anthonyalayo]

Background

Upon trying to use the nghttp2 target through fetchcontent, I saw the following:

CMake Error in build/_deps/nghttp2-src/lib/CMakeLists.txt:
  Target "nghttp2" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/Users/anthony.alayo/git/ALHttp-Cpp/build/_deps/nghttp2-build/lib/includes"

  which is prefixed in the build directory.

Taking a look, I realized that proper generator statements weren't setup for target_include_directories. I fixed that, so that both the build and install trees work.

While I was at it, I upgraded CMake to 3.14 (released April 2019 so it should be supported by others without issue) to reduce the complexity of the install statements. CMake 3.14 added default destinations for executables, static libraries, and shared libraries.

I added proper export statements to the installations, so that upstream projects don't need to do it themselves. Otherwise, CMake will throw the error target "nghttp2" that is not in any export set.

Lastly, I addressed issue #2050, where a properly aliased library is emitted from the project. It follows the same pattern that curl has. Using BUILD_SHARED_LIBS instead of ENABLE_SHARED_LIB aligns with CMake's global variable that is used in modern projects. BUILD_STATIC_LIBS is named for consistency.

Testing

Build

~/git/nghttp2 feat/fixingBuildInstallTrees* ❯ cmake -B build -DENABLE_LIB_ONLY=ON -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON
~/git/nghttp2 feat/fixingBuildInstallTrees* 7s ❯ cmake --build build
[  3%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_pq.c.o
[  3%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_map.c.o
[  6%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_queue.c.o
[  9%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_frame.c.o
[  9%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_buf.c.o
[ 12%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_stream.c.o
[ 12%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_outbound_item.c.o
[ 15%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_session.c.o
[ 18%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_submit.c.o
[ 18%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_helper.c.o
[ 21%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_alpn.c.o
[ 24%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_hd.c.o
[ 24%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_hd_huffman.c.o
[ 27%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_hd_huffman_data.c.o
[ 27%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_version.c.o
[ 30%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_priority_spec.c.o
[ 33%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_option.c.o
[ 33%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_callbacks.c.o
[ 36%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_mem.c.o
[ 36%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_http.c.o
[ 39%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_rcbuf.c.o
[ 42%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_extpri.c.o
[ 42%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_ratelim.c.o
[ 45%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_time.c.o
[ 48%] Building C object lib/CMakeFiles/nghttp2.dir/nghttp2_debug.c.o
[ 48%] Building C object lib/CMakeFiles/nghttp2.dir/sfparse.c.o
[ 51%] Linking C shared library libnghttp2.dylib
[ 51%] Built target nghttp2
[ 51%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_pq.c.o
[ 54%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_map.c.o
[ 57%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_queue.c.o
[ 57%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_frame.c.o
[ 60%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_buf.c.o
[ 60%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_stream.c.o
[ 63%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_outbound_item.c.o
[ 66%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_session.c.o
[ 66%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_submit.c.o
[ 69%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_helper.c.o
[ 72%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_alpn.c.o
[ 72%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_hd.c.o
[ 75%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_hd_huffman.c.o
[ 75%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_hd_huffman_data.c.o
[ 78%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_version.c.o
[ 81%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_priority_spec.c.o
[ 81%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_option.c.o
[ 84%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_callbacks.c.o
[ 84%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_mem.c.o
[ 87%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_http.c.o
[ 90%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_rcbuf.c.o
[ 90%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_extpri.c.o
[ 93%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_ratelim.c.o
[ 93%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_time.c.o
[ 96%] Building C object lib/CMakeFiles/nghttp2_static.dir/nghttp2_debug.c.o
[100%] Building C object lib/CMakeFiles/nghttp2_static.dir/sfparse.c.o
[100%] Linking C static library libnghttp2.a
[100%] Built target nghttp2_static

Install

~/git/nghttp2 feat/fixingBuildInstallTrees* ❯ sudo cmake --install build
-- Install configuration: "RelWithDebInfo"
-- Installing: /usr/local/share/doc/nghttp2/README.rst
-- Installing: /usr/local/include/nghttp2/nghttp2.h
-- Installing: /usr/local/include/nghttp2/nghttp2ver.h
-- Installing: /usr/local/lib/libnghttp2.14.26.0.dylib
-- Installing: /usr/local/lib/libnghttp2.14.dylib
-- Installing: /usr/local/lib/libnghttp2.dylib
-- Installing: /usr/local/lib/libnghttp2.a
-- Installing: /usr/local/lib/pkgconfig/libnghttp2.pc
-- Installing: /usr/local/lib/cmake/nghttp2/nghttp2-targets.cmake
-- Installing: /usr/local/lib/cmake/nghttp2/nghttp2-targets-relwithdebinfo.cmake
-- Installing: /usr/local/share/man/man1/nghttp.1
-- Installing: /usr/local/share/man/man1/nghttpd.1
-- Installing: /usr/local/share/man/man1/nghttpx.1
-- Installing: /usr/local/share/man/man1/h2load.1
-- Installing: /usr/local/share/nghttp2/fetch-ocsp-response

[anthonyalayo]

@tatsuhiro-t mind taking a look?

[anthonyalayo]

the heart of the issue

[tatsuhiro-t]

Thank you for PR. Merged now.

[jmckenna]

@anthonyalayo these pull request changes cause the lib to be built as static on Windows (with Visual Studio) even though BUILD_SHARED_LIBS is set , causing blocking errors downstream for Curl.

The if(BUILD_STATIC_LIBS) line at https://github.com/nghttp2/nghttp2/blob/master/lib/CMakeLists.txt#L85 should be moved up to https://github.com/nghttp2/nghttp2/blob/master/lib/CMakeLists.txt#L69

[jmckenna]

With the above change, all is well on Windows with the 1.60.0 release.

Have a nice weekend.

[anthonyalayo]

@jmckenna sorry just to make sure I'm following, you're saying the fix is still needed, or not needed? The messaging was a bit mixed there.

[jmckenna]

Your changes made through this pull request (that were merged into the 1.60.0 release) cause Windows/Visual Studio to always build static lib. In order to fix that, I described the one line change above.

[anthonyalayo]

@jmckenna I took a look at the changes but I can't see how this regression came around. I simply renamed ENABLE_STATIC_LIB to BUILD_STATIC_LIBS. Could you explain where in the code flow the change occurred such that the lib is always built static on Windows?

[jmckenna]

I pointed to the exact lines above

[jmckenna]

In your pull request here, you removed that if statement around the static part. I re-added it.

[vszakats]

Due to the rename, builds that set ENABLE_{STATIC,SHARED}_LIB variables to something non-default, now need updating.

[jmckenna]

@vszakats Nothing non-default, I am speaking of users who need to set BUILD_STATIC_LIBS, and in return get a static lib mistakenly, with 1.60.0 (through the pull request made here). They can now follow my step above to move that if statement.

[jmckenna]

ok ok. No worries. Closing here :)

[anthonyalayo]

Nice catch @vszakats , I didn't see the shell script there, that explains it