Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Roman Khabibov <roman.habibov@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 2/2] build: install libCURL headers
Date: Wed, 31 Mar 2021 02:14:02 +0300	[thread overview]
Message-ID: <20210330231402.ukhamua3lcamoa4l@tkn_work_nb> (raw)
In-Reply-To: <20210319134555.71396-3-roman.habibov@tarantool.org>

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.

> 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.

> 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.

[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?

  reply	other threads:[~2021-03-30 23:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-19 13:45 [Tarantool-patches] [PATCH 0/2] Install curl headers and enable smtp Roman Khabibov via Tarantool-patches
2021-03-19 13:45 ` [Tarantool-patches] [PATCH 1/2] build: " Roman Khabibov via Tarantool-patches
2021-03-30 23:13   ` Alexander Turenko via Tarantool-patches
2021-04-09 19:55     ` Roman Khabibov via Tarantool-patches
2021-04-12 10:01   ` Leonid Vasiliev via Tarantool-patches
2021-03-19 13:45 ` [Tarantool-patches] [PATCH 2/2] build: install libCURL headers Roman Khabibov via Tarantool-patches
2021-03-30 23:14   ` Alexander Turenko via Tarantool-patches [this message]
2021-04-09 19:55     ` Roman Khabibov via Tarantool-patches
2021-04-12 13:14       ` Leonid Vasiliev via Tarantool-patches
2021-04-15  0:28 ` [Tarantool-patches] [PATCH 0/2] Install curl headers and enable smtp Alexander Turenko via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210330231402.ukhamua3lcamoa4l@tkn_work_nb \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=roman.habibov@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2/2] build: install libCURL headers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox