From: Junio C Hamano <gitster@pobox.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: "Randall S. Becker" <rsbecker@nexbridge.com>, git@vger.kernel.org
Subject: Re: [Breakage] t0410 - subtests report unable to remove non-existent file.
Date: Wed, 11 Mar 2020 10:51:28 -0700 [thread overview]
Message-ID: <xmqq5zfaixun.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200310110008.GA3122@szeder.dev> ("SZEDER Gábor"'s message of "Tue, 10 Mar 2020 12:00:08 +0100")
SZEDER Gábor <szeder.dev@gmail.com> writes:
> promise_and_delete () {
> HASH=$(git -C repo rev-parse "$1") &&
> <...>
> git -C repo rev-parse my_annotated_tag | pack_as_from_promisor &&
> <...>
> delete_object repo "$HASH"
> }
>
> The failing 'rm' is in the 'delete_object_repo' helper function.
> The 'pack_as_from_promisor' does the following:
>
> pack_as_from_promisor () {
> HASH=$(git -C repo pack-objects .git/objects/pack/pack) &&
> >repo/.git/objects/pack/pack-$HASH.promisor &&
> echo $HASH
> }
>
> Notice that both 'promise_and_delete' and 'pack_as_from_promisor' set
> the $HASH variable. This is usually not an issue, because
> 'pack_as_from_promisor' is invoked in a pipe, and thus most shells
> execute it in a subshell environment.
>
> However, apparently 'ksh' doesn't run this helper function in a
> subshell environment, and the value set in 'pack_as_from_promisor'
> overwrites the value set in its caller, thus 'promise_and_delete' ends
> up trying to delete a non-existing object (it's the SHA1 of a
> packfile).
> ...
> Note, however, that 'ksh' is not utterly wrong in doing so, because
> POSIX does allow this behavior: POSIX 2.12 Shell Execution Environment,
> the last paragraph of the section:
>
> "each command of a multi-command pipeline is in a subshell
> environment; as an extension, however, any or all commands in a
> pipeline may be executed in the current environment."
>
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_12
>
> So apparently 'ksh' implements this extension.
>
> The trivial fix would be to mark $HASH as 'local' in both helper
> functions,
Or
git -C repo rev-parse my_annotated_tag |
(pack_as_from_promisor) &&
that would not require local?
> but this would not help 'ksh', of course, as it doesn't
> support 'local'. However, since we use more and more 'local's in our
> testsuite, 'ksh' might be considered a lost cause anyway.
That's somewhat sad, especially since "local" is outside POSIX, but
probably re-whipping our test suite into shape to be usable with
POSIX shells is too much effort for little benefit X-<.
> Or we could rename these HASH variables to something more specific to
> prevent this name collision, e.g. PACK_HASH in 'pack_as_from_promisor'.
>
> Note that there are tests in t0410 that set and use the $HASH variable
> outside of these helper function, and, worse, there is a test that uses
> the $HASH variable set in the previous test. Luckily, none of those
> tests use 'promise_and_delete' or 'pack_as_from_promisor'.
next prev parent reply other threads:[~2020-03-11 17:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-28 22:32 [Breakage] t0410 - subtests report unable to remove non-existent file Randall S. Becker
2020-03-10 11:00 ` SZEDER Gábor
2020-03-10 14:23 ` Randall S. Becker
2020-03-11 17:51 ` Junio C Hamano [this message]
2020-03-12 0:03 ` brian m. carlson
-- strict thread matches above, loose matches on Subject: below --
2020-03-04 22:13 Randall S. Becker
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq5zfaixun.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=rsbecker@nexbridge.com \
--cc=szeder.dev@gmail.com \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).