Re: Runaway sed memory use in test on older sed+glibc (was "Re: [PATCH v6 1/3] test: add helper functions for git-bundle")

>
>
> On Tue, Jun 01 2021, Jiang Xin wrote:
>
> > Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2021年5月27日周四 下午8:49写道：
> >>
> >> But no, the issue as it turns out is not Perl v.s. Sed, it's that
> >> there's some bug in the shellscript / tooling version (happens with both
> >> dash 0.5.7-4 and bash 4.3-11+deb8u2 on that box) where those expansions
> >> like ${A%${A#??????0?}} resolve to nothing.
> >
> > That's the root cause.  It can be reproduced by running the following
> > test script:
> >
> > ```
> > #!/bin/sh
> > # test script: test.sh
> >
> > test_commit_setvar () {
> >         var=$1 &&
> >         oid=1234567890123456789012345678901234567890 &&
> >         eval $var=$oid
> > }
> >
> > test_commit_setvar A
> > echo "A: $A"
> > echo "Abbrev of A: ${A%${A#???????}}"
> > ```
> >
> > By running different version of dash, we can see that dash 0.5.7 fail the test:
> >
> > ```
> > $ /opt/dash/0.5.11/bin/dash test.sh
> > A: 1234567890123456789012345678901234567890
> > Abbrev of A: 1234567
> >
> > $ /opt/dash/0.5.7/bin/dash test.sh
> > A: 1234567890123456789012345678901234567890
> > Abbrev of A:
> > ```
> >
> > This issue can be fixed using the following example:
> >
> > ```
> > #!/bin/sh
> >
> > test_commit_setvar () {
> >         var=$1 &&
> >         oid=1234567890123456789012345678901234567890 &&
> >         suffix=${oid#???????} &&
> >         oid=${oid%$suffix} &&
> >         eval $var=$oid
> > }
> >
> > test_commit_setvar A
> > echo "Abbrev of A: $A"
> > ```
>
> *nod*
>
> >> Anyway, looking at this whole test file with fresh eyes this pattern
> >> seems very strange. You duplicated most of test_commit with this
> >> test_commit_setvar. It's a bit more verbosity but why not just use:
> >>
> >>     test_commit ...
> >>     A=$(git rev-parse HEAD)
> >
> > The function "test_commit()" in "test-lib-function.sh" always creates
> > tags and it cannot make merge commit. So I rewrite a new function
> > which reuse the scaffold of "test_commit".
>
> It's had a --no-tag since 3803a3a099 (t: add --no-tag option to
> test_commit, 2021-02-09). I also have patches in "next" to add more
> options, you can just add more, having a --merge and maybe a way to tell
> it to eval the rev-parse into a given variable seem like sensible
> additions.
>
> > BTW, sorry for the late reply, will send patch later.
>
> My main point was that looking at this I think it's very much over the
> complexity v.s. benefit line on the "complexity" side.
>
> Even if there wasn't a --no-tag just using "test_commit" with a "git tag
> -d" and "commit_X=$(git rev-parse HEAD)" is less magical and more
> readable.
>
> I.e. the mostly copy/pasted from test-lib-functions.sh function is ~70
> lines, the whole setup function is 50 lines.
>
> And as I noted with the whitespace getting lost in the munging the end
> result is actually less reliable than just doing a test_cmp with $(git
> rev-parse ...) instead of <COMMIT-XYZ>.
>
> If you were trying to avoid the whitespace warnings then see the
> "'s/Z$//'" pattern in t0000-basic.sh for how we've usually tested that,
> i.e. had a "Z" at the end mark intentional whitespace for
> test_cmp-alike.
>
> There's a big value in the test suite being mostly consistent (which it
> somewhat isn't, but we're hopefully getting there). I.e. the goal isn't
> to optimize each test file to be as small as possible, but to e.g. have
> the next person maintaining it not wondering where <COMMIT-P> comes
> from, understanding some test_commit-alike that eval's variables into
> existence, how it's subtly different (if at all) from test_commit etc.