From: <rsbecker@nexbridge•com>
To: "'Junio C Hamano'" <gitster@pobox•com>,
"'Randall S. Becker'" <the.n.e.key@gmail•com>
Cc: <git@vger•kernel.org>,
"'Randall S. Becker'" <randall.becker@nexbridge•ca>
Subject: RE: [PATCH v0 1/1] Teach git version --build-options about zlib+libcurl
Date: Fri, 21 Jun 2024 14:33:14 -0400 [thread overview]
Message-ID: <016501dac409$7dd5bc00$79813400$@nexbridge.com> (raw)
In-Reply-To: <xmqqmsnekvir.fsf@gitster.g>
On Friday, June 21, 2024 2:20 PM, Junio C Hamano wrote:
>"Randall S. Becker" <the.n.e.key@gmail•com> writes:
>
>> This change uses the zlib supplied ZLIB_VERSION #define supplied text
>> macro and the libcurl LIBCURL_VERSION #define text macro. No
>> stringification is required for either variable's use. If either of
>> the #define is not present, that version is not reported.
>
>"the zlib supplied ZLIB_VERSION #define supplied text macro" is quite a
mouthful.
>Something like
>
> version: --build-options reports zlib and libcurl version information
>
> Use ZLIB_VERSION and LIBCURL_VERSION to show them, if defined, in
> "git version --build-options" output.
>
>should be sufficient.
Do you want me to reissue the merge? This looks fine to me.
>We will assume that
>
> (1) LIBFROTZ_VERSION, if defined, will always be of the same type
> (luckily, all three we are dealing with use a C-string so
> "strbuf_addf(buf, "%s", LIBFROTZ_VERSION)" is good), and that
>
> (2) no random origin other than the frotz project will define the
> CPP macro LIBFROTZ_VERSION to confuse us.
>
>Both are sensible assumptions that would allow us to trust a hardcoded
>strbuf_addf() invocation per each library is sufficient If a library uses
>LIBFROTZ_MAJOR and LIBFROTZ_MINOR we may have to do "strbuf_addf(buf,
>"%s.%s" LIBFROTZ_MAJOR, LIBFROTZ_MINOR)" that is different from others, but
>the point is the version identification scheme would be constant across
different
>versions of the same library.
>
>The actual code to report versions should be trivial, once we get the
mechanism to
>make necessary CPP macros available (when present) right, but the latter
needs a
>bit more work than this patch shows.
>
>Here is the first change your patch does:
>
>> #include "git-compat-util.h"
>> +#include "git-curl-compat.h"
>
>The file <git-curl-compat.h> begins like so:
>
> #ifndef GIT_CURL_COMPAT_H
> #define GIT_CURL_COMPAT_H
> #include <curl/curl.h>
> ...
>
In this case, I was modelling the include after http.c, and remote-curl.c,
which would have the same problem. I was going for consistency. Would not
all three have to be fixed in a separate patch?
>If you do not have any <curl/curl.h> anywhere on your system, I suspect
this will
>break the build, instead of silently leaving LIBCURL_VERSION undefined.
>
>> #include "config.h"
>> #include "builtin.h"
>> #include "exec-cmd.h"
>> @@ -757,6 +758,12 @@ void get_version_info(struct strbuf *buf, int
>> show_build_options)
>>
>> if (fsmonitor_ipc__is_supported())
>> strbuf_addstr(buf, "feature: fsmonitor--daemon\n");
>> +#if defined LIBCURL_VERSION
>> + strbuf_addf(buf, "libcurl: %s\n", LIBCURL_VERSION); #endif
#if
>> +defined ZLIB_VERSION
>> + strbuf_addf(buf, "zlib: %s\n", ZLIB_VERSION); #endif
>
>FYI, in the merged result, I would prefer to order these entries
semi-alphabetically,
>e.g. perhaps stripping possible "lib" prefix or suffix and comparing the
rest to result
>in curl < openssl < z or something like that. Then we know where to add a
new one,
>whose name we do not know yet, in the future.
I think that is logical. Do you need this redone? Although the OpenSSL
inclusion is already merged from what I can see.
>Thanks.
next prev parent reply other threads:[~2024-06-21 18:33 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-21 15:45 [PATCH v0 0/1] Teach git version --build-options about zlib+libcurl Randall S. Becker
2024-06-21 15:45 ` [PATCH v0 1/1] " Randall S. Becker
2024-06-21 18:20 ` Junio C Hamano
2024-06-21 18:28 ` Junio C Hamano
2024-06-21 18:33 ` rsbecker [this message]
2024-06-21 18:58 ` Junio C Hamano
2024-06-21 19:20 ` Junio C Hamano
2024-06-21 19:32 ` Randall Becker
2024-06-22 5:11 ` Junio C Hamano
2024-06-25 19:29 ` rsbecker
2024-06-25 20:58 ` Junio C Hamano
2024-06-25 22:02 ` rsbecker
2024-06-26 20:42 ` Jeff King
2024-06-26 22:28 ` Junio C Hamano
2024-06-26 20:46 ` Jeff King
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='016501dac409$7dd5bc00$79813400$@nexbridge.com' \
--to=rsbecker@nexbridge$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=randall.becker@nexbridge$(echo .)ca \
--cc=the.n.e.key@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox