Re: Runaway sed memory use in test on older sed+glibc (was "Re: [PATCH v6 1/3] test: add helper functions for git-bundle")
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jun 1, 2021, 11:50 UTC
- Message-ID
- <874keh94ga.fsf@evledraar.gmail.com>
- In-Reply-To
- <CANYiYbG1vVqN-De0n2Ukexh0Jn8e_b2G1CPv24T1fbYgyhKjfg@mail.gmail.com>
On Tue, Jun 01 2021, Jiang Xin wrote:
Show 53 quoted lines
> Æ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*
Show 10 quoted lines
>> 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.