git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Derrick Stolee <stolee@gmail.com>,
	git@vger.kernel.org, Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH] multi-pack-index: fix --object-dir from outside repo
Date: Mon, 23 Aug 2021 09:06:02 -0700	[thread overview]
Message-ID: <xmqq1r6krvrp.fsf@gitster.g> (raw)
In-Reply-To: <caafaf945ec43ba606b054bf4c4faa42e35a8db1.camel@sipsolutions.net> (Johannes Berg's message of "Mon, 23 Aug 2021 10:10:10 +0200")

Johannes Berg <johannes@sipsolutions.net> writes:

> Makes sense. FWIW, the test *did* restore the CWD so things worked, and
> subshells are actually ugly (need to import test-lib-functions.sh again
> if you want to use those), but I'll make it work somehow.

You do not need to dot-include test-lib-functions or anything ugly
or special.  The variables (not only the exported ones but regular
shell variables) and shell functions that is visible immediately
before you enter the opening "(" are all visible in the subshell.

The only notable difference you need to keep in mind when using
subshell is that you cannot affect variables and environment in
general of the calling shell.  In this case, you are taking
advantage of it---no matter where you chdir to, the main test
procedure that spawned the subshell will not be affected even if
your tests fail inside a subshell.  But it also disallows you from
doing certain things that rely on the ability to modify shell
variables, like setting up test_when_finished clean-up routine.

> More importantly, how do you feel about the "cd /"?

Please don't.  If somebody had a repository in /.git and the program
you are testing is buggy, you'd risk destroying it.  In general, it
is not a good idea to step outside the test directory you are given,
especially if you are *not* limiting yourself to read-only operation.

> The tests are always run in a place where there's a parent git folder
> (even if it's git itself), so you cannot reproduce the segfault in a
> test without the "cd /"

There is a "nongit" test helper in the test suite.  Would that work
for your case?

Thanks.

  parent reply	other threads:[~2021-08-23 16:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20 19:35 [PATCH] multi-pack-index: fix --object-dir from outside repo Johannes Berg
2021-08-22 23:51 ` Derrick Stolee
2021-08-23  7:21   ` Johannes Berg
2021-08-23  8:05     ` Junio C Hamano
2021-08-23  8:10       ` Johannes Berg
2021-08-23 13:19         ` Derrick Stolee
2021-08-23 13:40           ` Johannes Berg
2021-08-23 16:16             ` Junio C Hamano
2021-08-23 16:06         ` Junio C Hamano [this message]
2021-08-23  0:54 ` Taylor Blau
2021-08-23  7:32   ` Johannes Berg

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=xmqq1r6krvrp.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johannes@sipsolutions.net \
    --cc=me@ttaylorr.com \
    --cc=stolee@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.
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).