[PATCH v2 0/2] Teach git version --build-options about zlib+libcurl

[PATCH v2 0/2] Teach git version --build-options about zlib+libcurl

[Randall S. Becker]

This simple series adds the zlib and libcurl versions, if any, used
during a git build.

As an example, the following is appended to the git version --build-options
report:
	libcurl: 8.7.1
	zlib: 1.3.1

Randall S. Becker (2):
  Teach git version --build-options to know about libcurl versions.
  Teach git version --build-options to know about zlib versions.

 help.c | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.43.0

[PATCH v2 1/2] Teach git version --build-options about libcurl

[Randall S. Becker]

This change uses the libcurl LIBCURL_VERSION #define text macro. No
stringification is required for the variable's use. If the #define
is not present, that version is not reported.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 help.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/help.c b/help.c
index 1d057aa607..bf74e935b9 100644
--- a/help.c
+++ b/help.c
@@ -1,4 +1,5 @@
 #include "git-compat-util.h"
+#include "git-curl-compat.h" /* For LIBCURL_VERSION only */
 #include "config.h"
 #include "builtin.h"
 #include "exec-cmd.h"
@@ -757,6 +758,9 @@ 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
 	}
 }
 
-- 
2.43.0

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Johannes Schindelin]

Hi Randall,

On Fri, 21 Jun 2024, Randall S. Becker wrote:

> This change uses the libcurl LIBCURL_VERSION #define text macro. No
> stringification is required for the variable's use. If the #define
> is not present, that version is not reported.
>
> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> ---
>  help.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/help.c b/help.c
> index 1d057aa607..bf74e935b9 100644
> --- a/help.c
> +++ b/help.c
> @@ -1,4 +1,5 @@
>  #include "git-compat-util.h"
> +#include "git-curl-compat.h" /* For LIBCURL_VERSION only */
>  #include "config.h"
>  #include "builtin.h"
>  #include "exec-cmd.h"
> @@ -757,6 +758,9 @@ 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);

I am not sure that this is the most helpful information Git can provide:
It reports the version against which Git was _compiled_, whereas the
version it is _running against_ might be quite different.

Wouldn't calling `curl_version()` make more sense here?

Ciao,
Johannes

> +#endif
>  	}
>  }
>
> --
> 2.43.0
>
>
>

RE: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Randall Becker]

On Monday, June 24, 2024 10:13 AM, Johannes Schindelin wrote:
>On Fri, 21 Jun 2024, Randall S. Becker wrote:
>
>> This change uses the libcurl LIBCURL_VERSION #define text macro. No
>> stringification is required for the variable's use. If the #define is
>> not present, that version is not reported.
>>
>> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
>> ---
>>  help.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/help.c b/help.c
>> index 1d057aa607..bf74e935b9 100644
>> --- a/help.c
>> +++ b/help.c
>> @@ -1,4 +1,5 @@
>>  #include "git-compat-util.h"
>> +#include "git-curl-compat.h" /* For LIBCURL_VERSION only */
>>  #include "config.h"
>>  #include "builtin.h"
>>  #include "exec-cmd.h"
>> @@ -757,6 +758,9 @@ 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);
>
>I am not sure that this is the most helpful information Git can provide:
>It reports the version against which Git was _compiled_, whereas the version it is
>_running against_ might be quite different.
>
>Wouldn't calling `curl_version()` make more sense here?

I think the more important information is the build used. My reasoning is that one can call run curl --version to see the current curl install. However, different versions of curl have potential API changes - same argument with OpenSSL. What initiated this for me (the use case) started with a customer who incorrectly installed a git build for OpenSSL 3.2 (and its libcurl friend). Git would then get a compatibility issue when attempting to use either library. The customer did not know (!) they had the git for OpenSSL 3.2 version and I had no way to determine which one they had without seeing their path - hard in an email support situation. Having git version --build-options report what was used for the build *at a compatibility level* would have easily shown that the available library (after running openssl version or curl --version) reported different values. Otherwise, we are back to guessing what they installed. The goal is to compare what git expects with what git has available. The above series makes this comparative information available.

>Ciao,
>Johannes
>
>> +#endif
>>  	}
>>  }
>>
>> --
>> 2.43.0

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Dragan Simic]

On 2024-06-24 16:33, Randall Becker wrote:
> On Monday, June 24, 2024 10:13 AM, Johannes Schindelin wrote:
>> I am not sure that this is the most helpful information Git can 
>> provide:
>> It reports the version against which Git was _compiled_, whereas the 
>> version it is
>> _running against_ might be quite different.
>> 
>> Wouldn't calling `curl_version()` make more sense here?
> 
> I think the more important information is the build used. My reasoning
> is that one can call run curl --version to see the current curl
> install. However, different versions of curl have potential API
> changes - same argument with OpenSSL. What initiated this for me (the
> use case) started with a customer who incorrectly installed a git
> build for OpenSSL 3.2 (and its libcurl friend). Git would then get a
> compatibility issue when attempting to use either library. The
> customer did not know (!) they had the git for OpenSSL 3.2 version and
> I had no way to determine which one they had without seeing their path
> - hard in an email support situation. Having git version
> --build-options report what was used for the build *at a compatibility
> level* would have easily shown that the available library (after
> running openssl version or curl --version) reported different values.
> Otherwise, we are back to guessing what they installed. The goal is to
> compare what git expects with what git has available. The above series
> makes this comparative information available.

How about announcing both versions of the library if they differ, and
only one version if they're the same?  We're building this to serve
as a way for debugging various issues, having that information available
could only be helpful.

RE: [PATCH v2 1/2] Teach git version --build-options about libcurl

[rsbecker]

On Monday, June 24, 2024 1:09 PM, Dragan Simic wrote:
>On 2024-06-24 16:33, Randall Becker wrote:
>> On Monday, June 24, 2024 10:13 AM, Johannes Schindelin wrote:
>>> I am not sure that this is the most helpful information Git can
>>> provide:
>>> It reports the version against which Git was _compiled_, whereas the
>>> version it is _running against_ might be quite different.
>>>
>>> Wouldn't calling `curl_version()` make more sense here?
>>
>> I think the more important information is the build used. My reasoning
>> is that one can call run curl --version to see the current curl
>> install. However, different versions of curl have potential API
>> changes - same argument with OpenSSL. What initiated this for me (the
>> use case) started with a customer who incorrectly installed a git
>> build for OpenSSL 3.2 (and its libcurl friend). Git would then get a
>> compatibility issue when attempting to use either library. The
>> customer did not know (!) they had the git for OpenSSL 3.2 version and
>> I had no way to determine which one they had without seeing their path
>> - hard in an email support situation. Having git version
>> --build-options report what was used for the build *at a compatibility
>> level* would have easily shown that the available library (after
>> running openssl version or curl --version) reported different values.
>> Otherwise, we are back to guessing what they installed. The goal is to
>> compare what git expects with what git has available. The above series
>> makes this comparative information available.
>
>How about announcing both versions of the library if they differ, and only
one
>version if they're the same?  We're building this to serve as a way for
debugging
>various issues, having that information available could only be helpful.

I don't have a huge problem with that except it will significantly decrease
performance. We do not currently have to load libcurl/openssl to obtain the
build version (it is the --build-options flag), so adding additional load on
this command is not really what the series is about. Doing this run-time
check might be something someone else may want to take on separately, but
from a support use-case standpoint, it should be covered as is. Doing a
comparison is a separate use case.
--Randall

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Dragan Simic]

On 2024-06-24 23:15, rsbecker@nexbridge.com wrote:
> On Monday, June 24, 2024 1:09 PM, Dragan Simic wrote:
>> On 2024-06-24 16:33, Randall Becker wrote:
>>> On Monday, June 24, 2024 10:13 AM, Johannes Schindelin wrote:
>>>> I am not sure that this is the most helpful information Git can
>>>> provide:
>>>> It reports the version against which Git was _compiled_, whereas the
>>>> version it is _running against_ might be quite different.
>>>> 
>>>> Wouldn't calling `curl_version()` make more sense here?
>>> 
>>> I think the more important information is the build used. My 
>>> reasoning
>>> is that one can call run curl --version to see the current curl
>>> install. However, different versions of curl have potential API
>>> changes - same argument with OpenSSL. What initiated this for me (the
>>> use case) started with a customer who incorrectly installed a git
>>> build for OpenSSL 3.2 (and its libcurl friend). Git would then get a
>>> compatibility issue when attempting to use either library. The
>>> customer did not know (!) they had the git for OpenSSL 3.2 version 
>>> and
>>> I had no way to determine which one they had without seeing their 
>>> path
>>> - hard in an email support situation. Having git version
>>> --build-options report what was used for the build *at a 
>>> compatibility
>>> level* would have easily shown that the available library (after
>>> running openssl version or curl --version) reported different values.
>>> Otherwise, we are back to guessing what they installed. The goal is 
>>> to
>>> compare what git expects with what git has available. The above 
>>> series
>>> makes this comparative information available.
>> 
>> How about announcing both versions of the library if they differ, and 
>> only
> one
>> version if they're the same?  We're building this to serve as a way 
>> for
> debugging
>> various issues, having that information available could only be 
>> helpful.
> 
> I don't have a huge problem with that except it will significantly 
> decrease
> performance. We do not currently have to load libcurl/openssl to obtain 
> the
> build version (it is the --build-options flag), so adding additional 
> load on
> this command is not really what the series is about. Doing this 
> run-time
> check might be something someone else may want to take on separately, 
> but
> from a support use-case standpoint, it should be covered as is. Doing a
> comparison is a separate use case.

Yes, the additional load is actually a bit concerning.  Perhaps we could
wrap the current series up as-is and leave the possible improvements to
the follow-up patches?

RE: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Randall Becker]

On Monday, June 24, 2024 5:53 PM, Dragan Simic wrote:
>On 2024-06-24 23:15, rsbecker@nexbridge.com wrote:
>> On Monday, June 24, 2024 1:09 PM, Dragan Simic wrote:
>>> On 2024-06-24 16:33, Randall Becker wrote:
>>>> On Monday, June 24, 2024 10:13 AM, Johannes Schindelin wrote:
>>>>> I am not sure that this is the most helpful information Git can
>>>>> provide:
>>>>> It reports the version against which Git was _compiled_, whereas
>>>>> the version it is _running against_ might be quite different.
>>>>>
>>>>> Wouldn't calling `curl_version()` make more sense here?
>>>>
>>>> I think the more important information is the build used. My
>>>> reasoning is that one can call run curl --version to see the current
>>>> curl install. However, different versions of curl have potential API
>>>> changes - same argument with OpenSSL. What initiated this for me
>>>> (the use case) started with a customer who incorrectly installed a
>>>> git build for OpenSSL 3.2 (and its libcurl friend). Git would then
>>>> get a compatibility issue when attempting to use either library. The
>>>> customer did not know (!) they had the git for OpenSSL 3.2 version
>>>> and I had no way to determine which one they had without seeing
>>>> their path
>>>> - hard in an email support situation. Having git version
>>>> --build-options report what was used for the build *at a
>>>> compatibility
>>>> level* would have easily shown that the available library (after
>>>> running openssl version or curl --version) reported different values.
>>>> Otherwise, we are back to guessing what they installed. The goal is
>>>> to compare what git expects with what git has available. The above
>>>> series makes this comparative information available.
>>>
>>> How about announcing both versions of the library if they differ, and
>>> only
>> one
>>> version if they're the same?  We're building this to serve as a way
>>> for
>> debugging
>>> various issues, having that information available could only be
>>> helpful.
>>
>> I don't have a huge problem with that except it will significantly
>> decrease performance. We do not currently have to load libcurl/openssl
>> to obtain the build version (it is the --build-options flag), so
>> adding additional load on this command is not really what the series
>> is about. Doing this run-time check might be something someone else
>> may want to take on separately, but from a support use-case
>> standpoint, it should be covered as is. Doing a comparison is a
>> separate use case.
>
>Yes, the additional load is actually a bit concerning.  Perhaps we could wrap the
>current series up as-is and leave the possible improvements to the follow-up
>patches?

I think that's where we've gone, yes.

RE: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Johannes Schindelin]

Hi Randall,

On Mon, 24 Jun 2024, Randall Becker wrote:

> On Monday, June 24, 2024 10:13 AM, Johannes Schindelin wrote:
> >On Fri, 21 Jun 2024, Randall S. Becker wrote:
> >
> >> This change uses the libcurl LIBCURL_VERSION #define text macro. No
> >> stringification is required for the variable's use. If the #define is
> >> not present, that version is not reported.
> >>
> >> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> >> ---
> >>  help.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/help.c b/help.c
> >> index 1d057aa607..bf74e935b9 100644
> >> --- a/help.c
> >> +++ b/help.c
> >> @@ -1,4 +1,5 @@
> >>  #include "git-compat-util.h"
> >> +#include "git-curl-compat.h" /* For LIBCURL_VERSION only */
> >>  #include "config.h"
> >>  #include "builtin.h"
> >>  #include "exec-cmd.h"
> >> @@ -757,6 +758,9 @@ 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);
> >
> >I am not sure that this is the most helpful information Git can provide:
> >It reports the version against which Git was _compiled_, whereas the version it is
> >_running against_ might be quite different.
> >
> >Wouldn't calling `curl_version()` make more sense here?
>
> I think the more important information is the build used.

It may be informative, but more informative would be what libcurl version
is actually used, not what was current at build time. There may
occasionally have been a mismatch between the build-time and the runtime
version that may have led to bugs, after all.

> My reasoning is that one can call run curl --version to see the current
> curl install.

That assumes quite a bit. It assumes that the libcurl that is used by the
`curl` executable is the same as the one that is used by
`git-remote-https`.

A very large chunk (by some recent internal measurement, roughly two
thirds) of the Git users use Windows, where this assumption would be
incorrect.

Ciao,
Johannes

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Jeff King]

On Mon, Jun 24, 2024 at 04:13:23PM +0200, Johannes Schindelin wrote:

> > @@ -757,6 +758,9 @@ 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);
> 
> I am not sure that this is the most helpful information Git can provide:
> It reports the version against which Git was _compiled_, whereas the
> version it is _running against_ might be quite different.
> 
> Wouldn't calling `curl_version()` make more sense here?

I had a similar thought (and possibly even mentioning both the build
version and runtime version could be useful). But I don't think we can
call curl_version() here, as the main Git binary is (intentionally) not
linked against libcurl at all.

Even #including curl.h feels a little iffy to me, as it is declaring a
bunch of symbols that we will not have access to. It works here because
LIBCURL_VERSION is presumably a string literal, and not a reference to a
symbol. But if anybody mistakenly mentioned another symbol, the
compilation would work OK, but we'd fail at link time. Maybe an
acceptable risk, though.

-Peff

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> +#if defined LIBCURL_VERSION
>> +		strbuf_addf(buf, "libcurl: %s\n", LIBCURL_VERSION);
>
> I am not sure that this is the most helpful information Git can provide:
> It reports the version against which Git was _compiled_, whereas the
> version it is _running against_ might be quite different.

An unstated question is that we are not reporting the version of
openssl we happened to find at runtime, and if it is better to
report it, not the version Git was compiled to work with?

I obviously do not care about runtime performance of "git version
--build-options", but I am not enthused by the idea of forcing
dynamic-linking with libcURL and all of its dependencies when a
script runs a simple "git rev-parse HEAD".

        $ ldd git | wc -l
        6
        $ ldd git-remote-http | wc -l
        34

> Wouldn't calling `curl_version()` make more sense here?

I wouldn't give that question an outright "no", but unless "git
version" is split out of the builtin suite of commands and made into
a standalone binary, I would *not* be able to give an unconditional
"yes".

For now, let's stop at the simplest solution---if the library
project gives us a CPP macro to use for _this exact purpose_, let's
take the offer.

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Jeff King]

On Mon, Jun 24, 2024 at 09:06:03AM -0700, Junio C Hamano wrote:

> > Wouldn't calling `curl_version()` make more sense here?
> 
> I wouldn't give that question an outright "no", but unless "git
> version" is split out of the builtin suite of commands and made into
> a standalone binary, I would *not* be able to give an unconditional
> "yes".
> 
> For now, let's stop at the simplest solution---if the library
> project gives us a CPP macro to use for _this exact purpose_, let's
> take the offer.

Here's another point of view: libcurl is not a dependency of the git
binary at all! It is a dependency of the "curl" remote helper. Would it
make sense for "git remote-https --build-options" to exist?

I'm not sure. It resolves the linking problem and matches how the actual
programs are structured. But it is also not something that normal users
would tend to think about (even if you are having trouble with https
remotes, you might not know that is implemented as a remote helper).

But we could also have "git version --build-options" call "remote-https
--build-options" automatically, and just let it dump to the shared
stdout stream.

-Peff

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> But we could also have "git version --build-options" call "remote-https
> --build-options" automatically, and just let it dump to the shared
> stdout stream.

Yes, if we were to care the version of libcURL dynamic library that
gets linked at runtime, what you describe does sound like the right
way to go.  The separation between "git" (which does not depend on
libcURL at all) and "git-remote-http" (which happens to depend on
it) is our business and it is our responsibility to hide that from
the end users.

It certainly is MUCH better than making "git-help" a standalone
binary that links with everything we link with, only for version
reporting.

Thanks.

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Johannes Schindelin]

Hi Jeff,

On Mon, 24 Jun 2024, Jeff King wrote:

> On Mon, Jun 24, 2024 at 09:06:03AM -0700, Junio C Hamano wrote:
>
> > > Wouldn't calling `curl_version()` make more sense here?
> >
> > I wouldn't give that question an outright "no", but unless "git
> > version" is split out of the builtin suite of commands and made into
> > a standalone binary, I would *not* be able to give an unconditional
> > "yes".
> >
> > For now, let's stop at the simplest solution---if the library
> > project gives us a CPP macro to use for _this exact purpose_, let's
> > take the offer.
>
> Here's another point of view: libcurl is not a dependency of the git
> binary at all! It is a dependency of the "curl" remote helper. Would it
> make sense for "git remote-https --build-options" to exist?

I preemptively agreed with this approach a couple of years ago:
https://lore.kernel.org/git/nycvar.QRO.7.76.6.1911200027460.15956@tvgsbejvaqbjf.bet/

Even Junio agreed with this, in a manner of speaking:
https://lore.kernel.org/git/xmqq7dzebq4j.fsf@gitster.c.googlers.com/

And there is a patch by Emily already that implements it:
https://lore.kernel.org/git/20200214015343.201946-8-emilyshaffer@google.com/

This patch seems not to have made it into Git.

> I'm not sure. It resolves the linking problem and matches how the actual
> programs are structured. But it is also not something that normal users
> would tend to think about (even if you are having trouble with https
> remotes, you might not know that is implemented as a remote helper).
>
> But we could also have "git version --build-options" call "remote-https
> --build-options" automatically, and just let it dump to the shared
> stdout stream.

Teaching `git version` to show the cURL version may not be the best idea,
especially when it comes to the version used at runtime and using the
command-line option `--build-options` (with the option being specifically
about the build, not the runtime, version that was used).

Wouldn't it be better to go with Emily's approach to surface this
information via `git bugreport` instead of `git version`, potentially
enhanced to show both build-time and runtime version of libcurl?

Ciao,
Johannes

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Jeff King]

On Wed, Jul 24, 2024 at 12:48:20PM +0200, Johannes Schindelin wrote:

> > But we could also have "git version --build-options" call "remote-https
> > --build-options" automatically, and just let it dump to the shared
> > stdout stream.
> 
> Teaching `git version` to show the cURL version may not be the best idea,
> especially when it comes to the version used at runtime and using the
> command-line option `--build-options` (with the option being specifically
> about the build, not the runtime, version that was used).
> 
> Wouldn't it be better to go with Emily's approach to surface this
> information via `git bugreport` instead of `git version`, potentially
> enhanced to show both build-time and runtime version of libcurl?

I don't have a strong preference either way. I naturally turned towards
"git version" because that's what this thread was about, and also
because it predates git-bugreport.

It feels like there may be some duplication / overlap between what those
two things inspect, and we should perhaps unify them. One thing I
notice about bugreport is that it doesn't have a way to just dump
information without trying to start a bugreport. I'd be very unlikely to
use it myself for reporting a bug, but I may want to dump information
about git while debugging.

So whether that is in the form of "git bugreport --dump", or if all of
the collection is moved to "git version --build-info" and then bugreport
uses that to fill out its template, I don't care.

-Peff

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Johannes Schindelin]

Hi Jeff,

On Wed, 24 Jul 2024, Jeff King wrote:

> On Wed, Jul 24, 2024 at 12:48:20PM +0200, Johannes Schindelin wrote:
>
> > > But we could also have "git version --build-options" call "remote-https
> > > --build-options" automatically, and just let it dump to the shared
> > > stdout stream.
> >
> > Teaching `git version` to show the cURL version may not be the best idea,
> > especially when it comes to the version used at runtime and using the
> > command-line option `--build-options` (with the option being specifically
> > about the build, not the runtime, version that was used).
> >
> > Wouldn't it be better to go with Emily's approach to surface this
> > information via `git bugreport` instead of `git version`, potentially
> > enhanced to show both build-time and runtime version of libcurl?
>
> I don't have a strong preference either way. I naturally turned towards
> "git version" because that's what this thread was about, and also
> because it predates git-bugreport.

All true, but the name `git version` also sets some expectations. Users
who run `<command> version` will expect to see the version of the command
they are actually using currently.

For example, `curl -V` will list something like:

	curl 7.81.0 (x86_64-pc-linux-gnu) libcurl/7.81.0 OpenSSL/3.0.2
	zlib/1.2.11 brotli/1.0.9 zstd/1.4.8 libidn2/2.3.2 libpsl/0.21.0
	(+libidn2/2.3.2) libssh/0.9.6/openssl/zlib nghttp2/1.43.0
	librtmp/2.3 OpenLDAP/2.5.18

Those are the versions of the components that are actually used when
invoking `curl` commands, not versions that were present on the machine
that built the `curl` package.

Compare that to what we're experiencing with Git for Windows v2.46.0-rc2:
`git version --build-options` lists `libcurl: 8.8.0`. But running `git
fetch` will actually use libcurl v8.9.0, not v8.8.0. And the output does
not mention that this is the compile-time version. It lists only one
version, as if it was the one that the Git executable were using.

I suspect that this behavior will confuse users, and that this kind of
user experience will come back to haunt the Git project.

> It feels like there may be some duplication / overlap between what those
> two things inspect, and we should perhaps unify them. One thing I
> notice about bugreport is that it doesn't have a way to just dump
> information without trying to start a bugreport. I'd be very unlikely to
> use it myself for reporting a bug, but I may want to dump information
> about git while debugging.
>
> So whether that is in the form of "git bugreport --dump", or if all of
> the collection is moved to "git version --build-info" and then bugreport
> uses that to fill out its template, I don't care.

I feel that we may need a different command for that than `bugreport
--dump`, something that reflects that the user wants to gather data to
investigate an issue, but not necessarily report a bug to the Git project,
and that we should guide users to use that command instead of `git
version` when investigating such issues.

A command with a name along the lines of `git diagnose`, I'd say.

Ciao,
Johannes

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Jeff King]

On Thu, Jul 25, 2024 at 12:17:08AM +0200, Johannes Schindelin wrote:

> All true, but the name `git version` also sets some expectations. Users
> who run `<command> version` will expect to see the version of the command
> they are actually using currently.
> 
> For example, `curl -V` will list something like:
> 
> 	curl 7.81.0 (x86_64-pc-linux-gnu) libcurl/7.81.0 OpenSSL/3.0.2
> 	zlib/1.2.11 brotli/1.0.9 zstd/1.4.8 libidn2/2.3.2 libpsl/0.21.0
> 	(+libidn2/2.3.2) libssh/0.9.6/openssl/zlib nghttp2/1.43.0
> 	librtmp/2.3 OpenLDAP/2.5.18
> 
> Those are the versions of the components that are actually used when
> invoking `curl` commands, not versions that were present on the machine
> that built the `curl` package.
> 
> Compare that to what we're experiencing with Git for Windows v2.46.0-rc2:
> `git version --build-options` lists `libcurl: 8.8.0`. But running `git
> fetch` will actually use libcurl v8.9.0, not v8.8.0. And the output does
> not mention that this is the compile-time version. It lists only one
> version, as if it was the one that the Git executable were using.

Well, yes. The whole point of farming it out to remote-curl was so that
we could show that run-time version, which was what I said in the
message you were responding to. So I think we agree.

I would be fine showing _both_ the run-time and compile-time versions,
if they are clearly marked.

> > So whether that is in the form of "git bugreport --dump", or if all of
> > the collection is moved to "git version --build-info" and then bugreport
> > uses that to fill out its template, I don't care.
> 
> I feel that we may need a different command for that than `bugreport
> --dump`, something that reflects that the user wants to gather data to
> investigate an issue, but not necessarily report a bug to the Git project,
> and that we should guide users to use that command instead of `git
> version` when investigating such issues.
> 
> A command with a name along the lines of `git diagnose`, I'd say.

OK. I don't really care much either way how it is spelled, though my
inclination is that we already have a confusing number of commands and
should avoid adding more.

But my main point was that we have two ways of collecting data now, and
it would be easier for users if they were unified, however the result is
invoked.

-Peff

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> A command with a name along the lines of `git diagnose`, I'd say.
>
> OK. I don't really care much either way how it is spelled, though my
> inclination is that we already have a confusing number of commands and
> should avoid adding more.

I think what you are responding to is an oblique reference to a
command that already exists, and there is no need to worry about
adding more ;-).  If "bugreport" is not farming out the task of
collecting the information to it, "bugreport" need to be corrected
to do so (while "diagnose" may have to learn to collect more, if
"bugreport" collects things that "diagnose" does not (yet)).

> But my main point was that we have two ways of collecting data now, and
> it would be easier for users if they were unified, however the result is
> invoked.

Yup.

Re: [PATCH v2 1/2] Teach git version --build-options about libcurl

[Jeff King]

On Thu, Jul 25, 2024 at 08:28:58AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >> A command with a name along the lines of `git diagnose`, I'd say.
> >
> > OK. I don't really care much either way how it is spelled, though my
> > inclination is that we already have a confusing number of commands and
> > should avoid adding more.
> 
> I think what you are responding to is an oblique reference to a
> command that already exists, and there is no need to worry about
> adding more ;-).  If "bugreport" is not farming out the task of
> collecting the information to it, "bugreport" need to be corrected
> to do so (while "diagnose" may have to learn to collect more, if
> "bugreport" collects things that "diagnose" does not (yet)).

Oh, sorry, I somehow missed when diagnose was added. Yeah, that seems
like a perfectly reasonable place to put it, though like bugreport it
has the same annoyance that I just want to dump the info to stdout, not
make a zipfile.

Curiously, it seems to segfault for me when run outside a repository. :-/
The issue is a NULL the_repository->objects->odb pointer passed into
dir_file_stats(). But obviously that bug is orthogonal to the discussion
here.

-Peff

[PATCH v2 2/2] Teach git version --build-options about zlib versions.

[Randall S. Becker]

This change uses the zlib ZLIB_VERSION #define text macro. No
stringification is required for the variable's use. If the #define
is not present, that version is not reported.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 help.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/help.c b/help.c
index bf74e935b9..f378750af4 100644
--- a/help.c
+++ b/help.c
@@ -760,6 +760,9 @@ void get_version_info(struct strbuf *buf, int show_build_options)
 			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
 	}
 }
-- 
2.43.0

Re: [PATCH v2 2/2] Teach git version --build-options about zlib versions.

[Johannes Schindelin]

Hi Randall,

On Fri, 21 Jun 2024, Randall S. Becker wrote:

> This change uses the zlib ZLIB_VERSION #define text macro. No
> stringification is required for the variable's use. If the #define
> is not present, that version is not reported.
>
> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> ---
>  help.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/help.c b/help.c
> index bf74e935b9..f378750af4 100644
> --- a/help.c
> +++ b/help.c
> @@ -760,6 +760,9 @@ void get_version_info(struct strbuf *buf, int show_build_options)
>  			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);

This reports what zlib version Git was linked against, at compile time.
That may be misleading e.g. when running with a different version that
has a bug. Would `zlibVersion()` be more useful here?

Ciao,
Johannes

>  #endif
>  	}
>  }
> --
> 2.43.0
>
>
>

RE: [PATCH v2 2/2] Teach git version --build-options about zlib versions.

[Randall Becker]

On Monday, June 24, 2024 10:16 AM, Johannes Schindelin wrote:
>On Fri, 21 Jun 2024, Randall S. Becker wrote:
>
>> This change uses the zlib ZLIB_VERSION #define text macro. No
>> stringification is required for the variable's use. If the #define is
>> not present, that version is not reported.
>>
>> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
>> ---
>>  help.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/help.c b/help.c
>> index bf74e935b9..f378750af4 100644
>> --- a/help.c
>> +++ b/help.c
>> @@ -760,6 +760,9 @@ void get_version_info(struct strbuf *buf, int
>show_build_options)
>>  			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);
>
>This reports what zlib version Git was linked against, at compile time.
>That may be misleading e.g. when running with a different version that has a bug.
>Would `zlibVersion()` be more useful here?

Please see my comments on the libcurl sub-thread. Same logic applies here.

>
>Ciao,
>Johannes
>
>>  #endif
>>  	}
>>  }
>> --
>> 2.43.0

RE: [PATCH v2 2/2] Teach git version --build-options about zlib versions.

[Johannes Schindelin]

Hi Randall,

On Mon, 24 Jun 2024, Randall Becker wrote:

> On Monday, June 24, 2024 10:16 AM, Johannes Schindelin wrote:
> >On Fri, 21 Jun 2024, Randall S. Becker wrote:
> >
> >> This change uses the zlib ZLIB_VERSION #define text macro. No
> >> stringification is required for the variable's use. If the #define is
> >> not present, that version is not reported.
> >>
> >> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
> >> ---
> >>  help.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/help.c b/help.c
> >> index bf74e935b9..f378750af4 100644
> >> --- a/help.c
> >> +++ b/help.c
> >> @@ -760,6 +760,9 @@ void get_version_info(struct strbuf *buf, int
> >show_build_options)
> >>  			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);
> >
> >This reports what zlib version Git was linked against, at compile time.
> >That may be misleading e.g. when running with a different version that has a bug.
> >Would `zlibVersion()` be more useful here?
>
> Please see my comments on the libcurl sub-thread. Same logic applies here.

Let's summarize the part of your comments in that sub-thread that is
actually relevant here, okay? Here goes my attempt:

	The scenario of concern involved a customer having installed a Git
	build targeting the wrong library version.

Does that sound like a valid summary?

I am quite puzzled what exactly your answer is meant to tell me here,
though. Is it meant to say:

- Yes, it should be `zlibVersion()`, it is the version that is used and
  that might cause troubles after all,

- No, you want to continue showing the compile-time version, even if the
  user might actually use a different version depending what is installed
  in their setup,

- Yes _and_ no: both versions need to be displayed, as a discrepancy
  there might explain reported problems and could therefore be quite
  useful when handling bug reports.

In other words: Please understand that your answer to my question left me
wanting for an answer.

Ciao,
Johannes

RE: [PATCH v2 2/2] Teach git version --build-options about zlib versions.

[Randall Becker]

On Wednesday, July 24, 2024 7:02 AM, Johannes Schindelin wrote:
>On Mon, 24 Jun 2024, Randall Becker wrote:
>
>> On Monday, June 24, 2024 10:16 AM, Johannes Schindelin wrote:
>> >On Fri, 21 Jun 2024, Randall S. Becker wrote:
>> >
>> >> This change uses the zlib ZLIB_VERSION #define text macro. No
>> >> stringification is required for the variable's use. If the #define
>> >> is not present, that version is not reported.
>> >>
>> >> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
>> >> ---
>> >>  help.c | 3 +++
>> >>  1 file changed, 3 insertions(+)
>> >>
>> >> diff --git a/help.c b/help.c
>> >> index bf74e935b9..f378750af4 100644
>> >> --- a/help.c
>> >> +++ b/help.c
>> >> @@ -760,6 +760,9 @@ void get_version_info(struct strbuf *buf, int
>> >show_build_options)
>> >>  			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);
>> >
>> >This reports what zlib version Git was linked against, at compile time.
>> >That may be misleading e.g. when running with a different version that has a
>bug.
>> >Would `zlibVersion()` be more useful here?
>>
>> Please see my comments on the libcurl sub-thread. Same logic applies here.
>
>Let's summarize the part of your comments in that sub-thread that is actually
>relevant here, okay? Here goes my attempt:
>
>	The scenario of concern involved a customer having installed a Git
>	build targeting the wrong library version.
>
>Does that sound like a valid summary?
>
>I am quite puzzled what exactly your answer is meant to tell me here, though. Is it
>meant to say:
>
>- Yes, it should be `zlibVersion()`, it is the version that is used and
>  that might cause troubles after all,
>
>- No, you want to continue showing the compile-time version, even if the
>  user might actually use a different version depending what is installed
>  in their setup,
>
>- Yes _and_ no: both versions need to be displayed, as a discrepancy
>  there might explain reported problems and could therefore be quite
>  useful when handling bug reports.
>
>In other words: Please understand that your answer to my question left me
>wanting for an answer.

What I actually proposed was splitting --build-options with runtime (some representative argument). This would allow the headers used at build time (--build-options) to be reported *and* the runtime (probably) DLL versions (but would also report static linked library versions) to be reported. Both are useful from a support standpoint. However, the --build-options argument was intended to report an invariant set of values used during the build, so I would rather not conflate the two distinctly different semantic values.

--Randall

Re: [PATCH v2 2/2] Teach git version --build-options about zlib versions.

[Junio C Hamano]

Randall Becker <randall.becker@nexbridge.ca> writes:

> What I actually proposed was splitting --build-options with
> runtime (some representative argument). This would allow the
> headers used at build time (--build-options) to be reported *and*
> the runtime (probably) DLL versions (but would also report static
> linked library versions) to be reported. Both are useful from a
> support standpoint.

That certainly is a reasonable future plan.

> However, the --build-options argument was
> intended to report an invariant set of values used during the
> build, so I would rather not conflate the two distinctly different
> semantic values.

This reasoning makes sense to me, too.

Please wrap overly long lines to reasonable width, by the way.

RE: [PATCH v2 2/2] Teach git version --build-options about zlib versions.

[rsbecker]

On July 24, 2024 12:22 PM, Junio C Hamano wrote:
>Randall Becker <randall.becker@nexbridge.ca> writes:
>
>> What I actually proposed was splitting --build-options with runtime
>> (some representative argument). This would allow the headers used at
>> build time (--build-options) to be reported *and* the runtime
>> (probably) DLL versions (but would also report static linked library
>> versions) to be reported. Both are useful from a support standpoint.
>
>That certainly is a reasonable future plan.
>
>> However, the --build-options argument was intended to report an
>> invariant set of values used during the build, so I would rather not
>> conflate the two distinctly different semantic values.
>
>This reasoning makes sense to me, too.
>
>Please wrap overly long lines to reasonable width, by the way.

Sorry about that. Outlook is a pain in that regard, and different recipients
define different line lengths.

I will do my best in future for git.

--Randall