* RE: [BUG] Re: Git 2.35.0-rc0
2022-01-11 3:51 ` rsbecker
@ 2022-01-11 12:51 ` rsbecker
2022-01-11 19:53 ` Taylor Blau
2022-01-12 19:18 ` Junio C Hamano
2 siblings, 0 replies; 14+ messages in thread
From: rsbecker @ 2022-01-11 12:51 UTC (permalink / raw)
To: rsbecker, 'Taylor Blau'; +Cc: git
On January 10, 2022 10:52 PM, Taylor Blau wrote:
> On January 10, 2022 10:00 PM, Taylor Blau wrote:
> > On Mon, Jan 10, 2022 at 09:57:57PM -0500, rsbecker@nexbridge.com
> > wrote:
> > > > If your system doesn't have a modern-ish zlib, you may try
> > > > building with that knob, or upgrading your system's copy of zlib.
> > > > And if NonStop doesn't have a modern zlib available at all, we
> > > > should modify the NonStop section of config.mak.uname.
> > >
> > > There is no provision in reftable/block.c to avoid using
> > > uncompress2, so the knob will not help. Our zlibc is not that recent
> > > (as in it does not have uncompress2) and we cannot make the 2.35.0
> > > timeframe to upgrade it. The current zlib seems to require gcc and
> > > is very difficult to port at this stage. This is a blocker situation.
> >
> > NO_UNCOMPRESS2 does not avoid calling uncompress2, but instead
> > compiles a copy-and-pasted implementation in compat/ so that the function
> is available.
> >
> > Looking through it, I can't imagine that it wouldn't compile just fine
> > even on NonStop.
> >
> > Have you tried building with NO_UNCOMPRESS2?
>
> The patch for NonStop to make this compile is as follows. Test is running - will
> be 2 days:
>
> diff --git a/config.mak.uname b/config.mak.uname index
> a3a779327f..9b3e9bff5f 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -576,6 +576,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> NO_SETENV = YesPlease
> NO_UNSETENV = YesPlease
> NO_MKDTEMP = YesPlease
> + NO_UNCOMPRESS2 = YesPlease
> # Currently libiconv-1.9.1.
> OLD_ICONV = UnfortunatelyYes
> NO_REGEX = NeedsStartEnd
>
> Could we get that into rc1?
The above is working. We're at t2015 right now. Much relieved.
--Randall
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] Re: Git 2.35.0-rc0
2022-01-11 3:51 ` rsbecker
2022-01-11 12:51 ` rsbecker
@ 2022-01-11 19:53 ` Taylor Blau
2022-01-11 20:39 ` rsbecker
2022-01-12 19:18 ` Junio C Hamano
2 siblings, 1 reply; 14+ messages in thread
From: Taylor Blau @ 2022-01-11 19:53 UTC (permalink / raw)
To: rsbecker; +Cc: 'Taylor Blau', git
On Mon, Jan 10, 2022 at 10:51:39PM -0500, rsbecker@nexbridge.com wrote:
> I now have a different issue:
>
> make -C t/ all
> make[1]: Entering directory '/home/git/git/t'
> rm -f -r 'test-results'
> /usr/coreutils/bin/bash: /usr/bin/perl: Argument list too long
>
> Is there anyway to move to xargs? I am not sure why /usr/bin/perl is
> having issues with the build.
Tracing through t/Makefile, I am _pretty_ sure that this Perl invocation
comes from the test-lint-shell-syntax recipe.
I wonder if something like this would do the trick?
--- 8< ---
diff --git a/t/Makefile b/t/Makefile
index 46cd5fc527..d959119133 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -90,7 +90,7 @@ test-lint-executable:
echo >&2 "non-executable tests:" $$bad; exit 1; }
test-lint-shell-syntax:
- @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
+ @echo $(T) $(THELPERS) $(TPERF) | xargs '$(PERL_PATH_SQ)' check-non-portable-shell.pl
test-lint-filenames:
@# We do *not* pass a glob to ls-files but use grep instead, to catch
--- >8 ---
Thanks,
Taylor
^ permalink raw reply related [flat|nested] 14+ messages in thread
* RE: [BUG] Re: Git 2.35.0-rc0
2022-01-11 19:53 ` Taylor Blau
@ 2022-01-11 20:39 ` rsbecker
0 siblings, 0 replies; 14+ messages in thread
From: rsbecker @ 2022-01-11 20:39 UTC (permalink / raw)
To: 'Taylor Blau'; +Cc: git
On January 11, 2022 2:53 PM, Taylor Blau wrote:
> On Mon, Jan 10, 2022 at 10:51:39PM -0500, rsbecker@nexbridge.com
> wrote:
> > I now have a different issue:
> >
> > make -C t/ all
> > make[1]: Entering directory '/home/git/git/t'
> > rm -f -r 'test-results'
> > /usr/coreutils/bin/bash: /usr/bin/perl: Argument list too long
> >
> > Is there anyway to move to xargs? I am not sure why /usr/bin/perl is
> > having issues with the build.
>
> Tracing through t/Makefile, I am _pretty_ sure that this Perl invocation comes
> from the test-lint-shell-syntax recipe.
>
> I wonder if something like this would do the trick?
>
> --- 8< ---
>
> diff --git a/t/Makefile b/t/Makefile
> index 46cd5fc527..d959119133 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -90,7 +90,7 @@ test-lint-executable:
> echo >&2 "non-executable tests:" $$bad; exit 1; }
>
> test-lint-shell-syntax:
> - @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS)
> $(TPERF)
> + @echo $(T) $(THELPERS) $(TPERF) | xargs '$(PERL_PATH_SQ)'
> +check-non-portable-shell.pl
>
> test-lint-filenames:
> @# We do *not* pass a glob to ls-files but use grep instead, to catch
>
> --- >8 ---
I think we don't need this yet. My CI/CD system is a bit more robust and does not get stuck here.
--Randall
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] Re: Git 2.35.0-rc0
2022-01-11 3:51 ` rsbecker
2022-01-11 12:51 ` rsbecker
2022-01-11 19:53 ` Taylor Blau
@ 2022-01-12 19:18 ` Junio C Hamano
2022-01-12 19:21 ` Taylor Blau
2022-01-13 13:21 ` Ævar Arnfjörð Bjarmason
2 siblings, 2 replies; 14+ messages in thread
From: Junio C Hamano @ 2022-01-12 19:18 UTC (permalink / raw)
To: rsbecker; +Cc: 'Taylor Blau', git
<rsbecker@nexbridge.com> writes:
> diff --git a/config.mak.uname b/config.mak.uname
> index a3a779327f..9b3e9bff5f 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -576,6 +576,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> NO_SETENV = YesPlease
> NO_UNSETENV = YesPlease
> NO_MKDTEMP = YesPlease
> + NO_UNCOMPRESS2 = YesPlease
> # Currently libiconv-1.9.1.
> OLD_ICONV = UnfortunatelyYes
> NO_REGEX = NeedsStartEnd
>
> Could we get that into rc1?
Sure, with an appliable patch that is properly signed off.
----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
Date: Mon, 10 Jan 2022 22:51:39 -0500
From: "Randall S. Becker" <rsbecker@nexbridge.com>
Subject: [PATCH] build: NonStop ships with an older zlib
Notably, it lacks uncompress2(); use the fallback we ship in our
tree instead.
not-Signed-off-yet-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
config.mak.uname | 1 +
1 file changed, 1 insertion(+)
diff --git a/config.mak.uname b/config.mak.uname
index a3a779327f..9b3e9bff5f 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -576,6 +576,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
NO_SETENV = YesPlease
NO_UNSETENV = YesPlease
NO_MKDTEMP = YesPlease
+ NO_UNCOMPRESS2 = YesPlease
# Currently libiconv-1.9.1.
OLD_ICONV = UnfortunatelyYes
NO_REGEX = NeedsStartEnd
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [BUG] Re: Git 2.35.0-rc0
2022-01-12 19:18 ` Junio C Hamano
@ 2022-01-12 19:21 ` Taylor Blau
2022-01-13 13:21 ` Ævar Arnfjörð Bjarmason
1 sibling, 0 replies; 14+ messages in thread
From: Taylor Blau @ 2022-01-12 19:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: rsbecker, 'Taylor Blau', git
On Wed, Jan 12, 2022 at 11:18:20AM -0800, Junio C Hamano wrote:
> <rsbecker@nexbridge.com> writes:
>
> > diff --git a/config.mak.uname b/config.mak.uname
> > index a3a779327f..9b3e9bff5f 100644
> > --- a/config.mak.uname
> > +++ b/config.mak.uname
> > @@ -576,6 +576,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> > NO_SETENV = YesPlease
> > NO_UNSETENV = YesPlease
> > NO_MKDTEMP = YesPlease
> > + NO_UNCOMPRESS2 = YesPlease
> > # Currently libiconv-1.9.1.
> > OLD_ICONV = UnfortunatelyYes
> > NO_REGEX = NeedsStartEnd
> >
> > Could we get that into rc1?
>
> Sure, with an appliable patch that is properly signed off.
I'm happy to attach my s-o-b to the below as well (and in general, that
it's safe to assume my sign-off is provided to everything I post to the
list, unless I explicitly say otherwise), since I originally suggested
the diff.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] Re: Git 2.35.0-rc0
2022-01-12 19:18 ` Junio C Hamano
2022-01-12 19:21 ` Taylor Blau
@ 2022-01-13 13:21 ` Ævar Arnfjörð Bjarmason
2022-01-13 18:03 ` Junio C Hamano
1 sibling, 1 reply; 14+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-01-13 13:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: rsbecker, 'Taylor Blau', git, Han-Wen Nienhuys
On Wed, Jan 12 2022, Junio C Hamano wrote:
> <rsbecker@nexbridge.com> writes:
>
>> diff --git a/config.mak.uname b/config.mak.uname
>> index a3a779327f..9b3e9bff5f 100644
>> --- a/config.mak.uname
>> +++ b/config.mak.uname
>> @@ -576,6 +576,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
>> NO_SETENV = YesPlease
>> NO_UNSETENV = YesPlease
>> NO_MKDTEMP = YesPlease
>> + NO_UNCOMPRESS2 = YesPlease
>> # Currently libiconv-1.9.1.
>> OLD_ICONV = UnfortunatelyYes
>> NO_REGEX = NeedsStartEnd
>>
>> Could we get that into rc1?
>
> Sure, with an appliable patch that is properly signed off.
>
> ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
> Date: Mon, 10 Jan 2022 22:51:39 -0500
> From: "Randall S. Becker" <rsbecker@nexbridge.com>
> Subject: [PATCH] build: NonStop ships with an older zlib
>
> Notably, it lacks uncompress2(); use the fallback we ship in our
> tree instead.
>
> not-Signed-off-yet-by: Randall S. Becker <rsbecker@nexbridge.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> config.mak.uname | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/config.mak.uname b/config.mak.uname
> index a3a779327f..9b3e9bff5f 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -576,6 +576,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> NO_SETENV = YesPlease
> NO_UNSETENV = YesPlease
> NO_MKDTEMP = YesPlease
> + NO_UNCOMPRESS2 = YesPlease
> # Currently libiconv-1.9.1.
> OLD_ICONV = UnfortunatelyYes
> NO_REGEX = NeedsStartEnd
I've run into some other cases on testing on platforms hwere the
NO_UNCOMPRESS2 is now needed.
It's a bit of an annoyance, as git would previously compile cleanly on
almost any system with NO_CURL=Y NO_OPENSSL=Y, most of those with a C
compiler have a libz, even if it's ancient.
As I noted in
https://lore.kernel.org/git/87wnn62nhp.fsf@evledraar.gmail.com/ this use
of uncompress2() is just saving a few lines of boilerplate instead of
using the underlying zlib functions, which every other in-tree user
uses.
So I think it would be most welcome if someone wanted to work on moving
to that API use, and IMO such a change could even happen before the
final release. After all we don't use reftable for anything except the
tests it runs, so as long as those pass it IMO outweighs the new
annyonce in compiling git on those platforms.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [BUG] Re: Git 2.35.0-rc0
2022-01-13 13:21 ` Ævar Arnfjörð Bjarmason
@ 2022-01-13 18:03 ` Junio C Hamano
0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2022-01-13 18:03 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: rsbecker, 'Taylor Blau', git, Han-Wen Nienhuys
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> https://lore.kernel.org/git/87wnn62nhp.fsf@evledraar.gmail.com/ this use
> of uncompress2() is just saving a few lines of boilerplate instead of
> using the underlying zlib functions, which every other in-tree user
> uses.
I looked at the source of uncompress2(), and I tend to agree that,
we do NOT HAVE TO add dependency on it. You should be able to
rewrite the new caller without using it.
But this is a 5-year old API function, that first appeared how many
years ago in their public release? In a few years, I would say that
we would be laughed at if we avoid it with "it is not available
everywhere".
And in the meantime, we ship a copy lifted from upstream, so those
with older zlib would be OK with it, too.
That is how I view what we have today. To me, the logical
conclusion of the observation is that, we do not have good reason
to avoid it.
When we are adding a new piece of code that drives inflate(), we
should remember that we now have uncompress2() available in the
codebase, and see if we can simplify it by using uncompress2(). The
same is true for a case where we refactor existing code that can
become simpler with uncompress2().
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread