From: Junio C Hamano <gitster@pobox.com>
To: Jacob Abel <jacobabel@nullpo.dev>
Cc: phillip.wood@dunelm.org.uk, git@vger.kernel.org
Subject: Re: [PATCH] t2400: Fix test failures when using grep 2.5
Date: Sat, 15 Jul 2023 18:08:33 -0700 [thread overview]
Message-ID: <xmqqilakll2m.fsf@gitster.g> (raw)
In-Reply-To: <vn5sylull5lqpitsanlyan5fafxj5dhrxgo6k65c462dhqjbno@uwghfyfdixtk> (Jacob Abel's message of "Sat, 15 Jul 2023 23:15:28 +0000")
Jacob Abel <jacobabel@nullpo.dev> writes:
>> > @@ -998,8 +998,8 @@ test_dwim_orphan () {
>> > headpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&
>>
>> I'm a bit confused by the --sq here - why does it need to be shell
>> quoted when it is always used inside double quotes?
>
> To be honest I can't remember if this specifically needs to be in
> quotes or not however I had a lot of trouble during the development of
> that patchset with things escaping quotes and causing breakages in the
> tests so if it isn't currently harmful I'd personally prefer to leave
> it as is.
Quoting is sometimes tricky enough that "this happens to work for me
but I do not know why it works" is asking for trouble in somebody
else's environment. If the form in the patch is correct, but tricky
for others to understand, you'd need to pick it apart and document
how it works (and if you cannot do so, ask for help by somebody who
can, or simplify it enough so that you can explain it yourself).
headpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&
In this case, "--sq" is a noop that only confuses readers, I think,
and I would drop it if I were you. "--git-path HEAD" is given by
this call chain:
builtin/rev-parse.c:cmd_rev_parse()
-> builtin/rev-parse.c:print_path()
-> transform path depending on the path format
-> puts()
and nowhere in this chain "output_sq" (which is set by "--sq") is
even checked. The transformations are all about relative, prefix,
etc., and never about quoting.
The original test script t2400 (before your patch) does look crappy
with full of long lines and coding style violations (none of which
is your fault), and it may need to be cleaned up once this patch
settles.
Thanks.
next prev parent reply other threads:[~2023-07-16 1:08 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-15 2:55 [PATCH] t2400: Fix test failures when using grep 2.5 Jacob Abel
2023-07-15 8:59 ` Phillip Wood
2023-07-15 23:15 ` Jacob Abel
2023-07-16 1:08 ` Junio C Hamano [this message]
2023-07-16 2:55 ` Jacob Abel
2023-07-16 15:34 ` Phillip Wood
2023-07-17 2:38 ` Junio C Hamano
2023-07-18 0:44 ` Jacob Abel
2023-07-18 13:36 ` Phillip Wood
2023-07-21 4:35 ` Jacob Abel
2023-07-15 23:36 ` Jacob Abel
2023-07-16 3:38 ` [PATCH v2] " Jacob Abel
2023-07-21 4:40 ` [PATCH v3 0/3] " Jacob Abel
2023-07-21 4:40 ` [PATCH v3 1/3] t2400: drop no-op `--sq` from rev-parse call Jacob Abel
2023-07-21 4:40 ` [PATCH v3 2/3] builtin/worktree.c: convert tab in advice to space Jacob Abel
2023-07-21 4:40 ` [PATCH v3 3/3] t2400: rewrite regex to avoid unintentional PCRE Jacob Abel
2023-07-21 15:16 ` Junio C Hamano
2023-07-21 15:49 ` Junio C Hamano
2023-07-22 2:36 ` Jacob Abel
2023-07-26 21:42 ` [PATCH v4 0/3] t2400: Fix test failures when using grep 2.5 Jacob Abel
2023-07-26 21:42 ` [PATCH v4 1/3] t2400: drop no-op `--sq` from rev-parse call Jacob Abel
2023-07-26 21:42 ` [PATCH v4 2/3] builtin/worktree.c: convert tab in advice to space Jacob Abel
2023-07-26 21:42 ` [PATCH v4 3/3] t2400: rewrite regex to avoid unintentional PCRE Jacob Abel
2023-07-26 22:09 ` [PATCH v4 0/3] t2400: Fix test failures when using grep 2.5 Junio C Hamano
2023-07-28 13:09 ` Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqilakll2m.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jacobabel@nullpo.dev \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).