Re: [Tarantool-patches] [PATCH 2/2] build: install libCURL headers

[Roman Khabibov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the review.

> On Mar 31, 2021, at 02:14, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
> 
> LGTM after fixes (no need to re-review with me).
> 
> Please, update and proceed with the next reviewer.
> 
> On Fri, Mar 19, 2021 at 04:45:55PM +0300, Roman Khabibov wrote:
>> Ship libCURL headers to system path "include/tarantool" in the
>> case of libCURL included as bundled library or static build.
> 
> Please, reflect comments to the first patch here: the library naming,
> motivation of the change, the issue number.
> 
> Nit: I suggest to refer include directory as
> "${PREFIX}/include/tarantool" -- it makes quite clear that it may be
> /usr/include/tarantool, /usr/local/include/tarantool or something of
> this kind.
Done.

>> diff --git a/changelogs/unreleased/install-headers.md b/changelogs/unreleased/install-headers.md
>> new file mode 100755
>> index 000000000..4494a14c8
>> --- /dev/null
>> +++ b/changelogs/unreleased/install-headers.md
>> @@ -0,0 +1,4 @@
>> +## feature/build
>> +
>> +* Ship libCURL headers to system path "include/tarantool" in the
>> +case of libCURL included as bundled library or static build (gh-####).
>> \ No newline at end of file
> 
> No newline at end of file.
Added.

>> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
>> index 92e693955..d19df8925 100644
>> --- a/rpm/tarantool.spec
>> +++ b/rpm/tarantool.spec
>> @@ -268,6 +268,15 @@ fi
>> %{_includedir}/tarantool/luajit.h
>> %{_includedir}/tarantool/lualib.h
>> %{_includedir}/tarantool/module.h
>> +%{_includedir}/tarantool/curl/curl.h
>> +%{_includedir}/tarantool/curl/curlver.h
>> +%{_includedir}/tarantool/curl/easy.h
>> +%{_includedir}/tarantool/curl/mprintf.h
>> +%{_includedir}/tarantool/curl/multi.h
>> +%{_includedir}/tarantool/curl/stdcheaders.h
>> +%{_includedir}/tarantool/curl/system.h
>> +%{_includedir}/tarantool/curl/typecheck-gcc.h
>> +%{_includedir}/tarantool/curl/urlapi.h
> 
> AFAIR, just %{_includedir}/tarantool/curl (without %dir) should work
> well and should install the whole directory. It'll allow us to update
> libcurl beyond 7.73.0 (see [1]) without a fear to forget to update those
> rules.
diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
index 92e693955..f8f6c124a 100644
--- a/rpm/tarantool.spec
+++ b/rpm/tarantool.spec
@@ -268,6 +268,7 @@ fi
 %{_includedir}/tarantool/luajit.h
 %{_includedir}/tarantool/lualib.h
 %{_includedir}/tarantool/module.h
+%{_includedir}/tarantool/curl

> [1]: https://github.com/curl/curl/commit/6ebe63fac23f38df911edc348e8ccc72280f9434
> 
> There is a risk to miss a problem with installing the headers (if cmake
> does not install it because of some problem), however partial installing
> looks even worse.
> 
> How about Debian based distributions? Nothing to change, the headers
> will be installed?

commit bec7a26a238dd1c03672d0f46ca082497a3027a0
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date:   Sun Dec 20 13:08:40 2020 +0500

    build: install libcurl headers
    
    Ship libcurl headers to system path "${PREFIX}/include/tarantool"
    in the case of libcurl included as bundled library.
    
    Closes #4559

diff --git a/CMakeLists.txt b/CMakeLists.txt
index feb56dfca..1196b65b9 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -428,6 +428,13 @@ else()
     find_package(CURL)
 endif()
 
+# Install headers.
+if (ENABLE_BUNDLED_LIBCURL)
+    install(DIRECTORY "${CURL_INCLUDE_DIRS}/curl"
+            DESTINATION ${MODULE_FULL_INCLUDEDIR}
+            FILES_MATCHING PATTERN "*.h")
+endif()
+
 #
 # Export libcurl symbols if the library is linked statically.
 #
diff --git a/changelogs/unreleased/install-headers.md b/changelogs/unreleased/install-headers.md
new file mode 100755
index 000000000..86363adb3
--- /dev/null
+++ b/changelogs/unreleased/install-headers.md
@@ -0,0 +1,4 @@
+## feature/build
+
+* Ship libcurl headers to system path "include/tarantool" in the
+case of libcurl included as bundled library or static build (gh-4559).
diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
index 92e693955..f8f6c124a 100644
--- a/rpm/tarantool.spec
+++ b/rpm/tarantool.spec
@@ -268,6 +268,7 @@ fi
 %{_includedir}/tarantool/luajit.h
 %{_includedir}/tarantool/lualib.h
 %{_includedir}/tarantool/module.h
+%{_includedir}/tarantool/curl
 
 %changelog
 * Tue Sep 12 2017 Roman Tsisyk <roman@tarantool.org> 1.7.5.46-1