Re: [PATCH 1/3] t/lib-httpd: avoid using BSD's sed

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Wed, 26 Feb 2020, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >
> >> diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
> >> index 9e16512fe31..4f10057e9f1 100755
> >> --- a/t/t5537-fetch-shallow.sh
> >> +++ b/t/t5537-fetch-shallow.sh
> >> @@ -237,7 +237,7 @@ test_expect_success 'shallow fetches check connectivity before writing shallow f
> >>  	git -C "$REPO" config protocol.version 2 &&
> >>  	git -C client config protocol.version 2 &&
> >>
> >> -	git -C client fetch --depth=2 "$HTTPD_URL/one_time_sed/repo" master:a_branch &&
> >> +	git -C client fetch --depth=2 "$HTTPD_URL/one_time_perl/repo" master:a_branch &&
> >>
> >>  	# Craft a situation in which the server sends back an unshallow request
> >>  	# with an empty packfile. This is done by refetching with a shorter
> >> @@ -246,13 +246,13 @@ test_expect_success 'shallow fetches check connectivity before writing shallow f
> >>  	printf "$(test_oid sed)" \
> >
> > Hmm, shouldn't the test-oid token "sed" whose value is set up in the
> > setup section of this test script also be renamed to "perl"?

Ooops...

> > Or, if we are actively taking advantage of the fact that the syntax of
> > the replacement operator is the same between the languages, perhaps
> > "sed" is better renamed to something more language agnostic and
> > reflects the purpose/reason why we extend the packet header by two
> > bytes with the one-time munging process?
> >
> >>  	       "$(git -C "$REPO" rev-parse HEAD)" \
> >>  	       "$(git -C "$REPO" rev-parse HEAD^)" \
> >> -	       >"$HTTPD_ROOT_PATH/one-time-sed" &&
> >> +	       >"$HTTPD_ROOT_PATH/one-time-perl" &&
> >
> > Other than that, this step looked quite sensible.  Thanks.
>
> Hmm, is it because you wanted to backport this down to 'maint'
> (otherwise, your tests will start failing in a month) that you left
> the "test_oid sed" thing untouched?  If so, that makes sort-of
> sense.

That's a good point. I target `maint` in v2, and offered an add-on patch
meant to be applied on top of the merge into `master` (or `next`, or
`pu`).

> I expect that the series will be rerolled, if only for s/BSD/macOS/
> mentioned elsewhere in the thread, but in the meantime, I'll rebase
> them on 'maint' "as a practice" while queuing.

Thanks ;-)

Ciao,
Dscho