| Message ID | 20240621180947.64419-3-randall.becker@nexbridge.ca (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Teach git version --build-options about zlib+libcurl | expand |
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 > > >
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
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
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
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.
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
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 } }
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(+)