git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Philippe Blain <levraiphilippeblain@gmail.com>
Cc: "Elijah Newren" <newren@gmail.com>,
	"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>,
	"Git List" <git@vger.kernel.org>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Jens Lehmann" <Jens.Lehmann@web.de>,
	"Carlo Arenas" <carenas@gmail.com>,
	"Taylor Blau" <me@ttaylorr.com>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [PATCH 0/2] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug'
Date: Fri, 20 Aug 2021 11:50:09 -0400	[thread overview]
Message-ID: <CAPig+cR7HhWN8F1G7_rWpD_jXfaKqaj7c4fMbkgAGvcjf1pX6g@mail.gmail.com> (raw)
In-Reply-To: <53752b7d-c294-9507-a7f0-af2aee491251@gmail.com>

On Fri, Aug 20, 2021 at 8:12 AM Philippe Blain
<levraiphilippeblain@gmail.com> wrote:
> Le 2021-08-19 à 16:11, Eric Sunshine a écrit :
> > Oh, indeed. I didn't mean to imply that HOME is the only problematic
> > one; they all are since, as you say, they can impact correctness and
> > reproducibility of the tests themselves. I called out HOME specially
> > because of the potential danger involved with pointing it at the
> > user's real home directory since it could very well lead to clobbering
> > of precious files and other settings belonging to the user.
>
> I also did not think about the implications of changing $HOME that could lead to the
> test framework overwriting stuff in my home. I checked the tests and there are only
> a handful of them that seem to reference HOME, but still, for those tests it would be
> undesirable to reset HOME.

It's not just tests which reference HOME explicitly which are
problematic. Git commands themselves access files and configuration
pointed at by HOME. Worse, Git commands invoked by tests can alter
configuration, so the potential for damage is wider than the few tests
which reference HOME explicitly.

> In light of this I'm thinking of simply adding flags to 'test_pause' and 'debug' to signal
> that one wants to use their original TERM, HOME and SHELL, with appropriate  caveats in
> the description of the functions:
>
> test_pause     # original behaviour
> test_pause -t  # use USER_TERM
> test_pause -s  # use SHELL instead of TEST_SHELL_PATH
> test_pause -h  # use USER_HOME

A (very) stray thought I had: Rather than mucking with the environment
variables -- which can impact test reproducibility and correctness and
can potentially damage the user's precious files and configuration --
another possibility might be to detect which shell is being used
(whether it be bash, zsh, etc.), and then enable certain useful
options specific to the shell, such as tab-completion, colors, etc.
This approach doesn't give developers the customized shell experience
they're used to (it wouldn't have their aliases or configuration, for
instance), but it at least might make the test_pause() experience a
bit more pleasant.

  reply	other threads:[~2021-08-20 15:50 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19 17:16 [PATCH 0/2] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Philippe Blain via GitGitGadget
2021-08-19 17:16 ` [PATCH 1/2] test-lib-functions: use user's SHELL, HOME and TERM for 'test_pause' Philippe Blain via GitGitGadget
2021-08-20  3:08   ` Carlo Arenas
2021-08-20 12:14     ` Philippe Blain
2021-08-19 17:16 ` [PATCH 2/2] test-lib-functions: use user's TERM and HOME for 'debug' Philippe Blain via GitGitGadget
2021-08-19 19:24   ` Taylor Blau
2021-08-20  3:18     ` Carlo Arenas
2021-08-19 18:10 ` [PATCH 0/2] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Eric Sunshine
2021-08-19 19:57   ` Junio C Hamano
2021-08-19 20:14     ` Eric Sunshine
2021-08-19 20:03   ` Elijah Newren
2021-08-19 20:11     ` Eric Sunshine
2021-08-20 12:12       ` Philippe Blain
2021-08-20 15:50         ` Eric Sunshine [this message]
2021-08-20 18:23         ` Jeff King
2021-08-28  0:47 ` [PATCH v2 0/3] " Philippe Blain via GitGitGadget
2021-08-28  0:47   ` [PATCH v2 1/3] test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause' Philippe Blain via GitGitGadget
2021-08-28  0:47   ` [PATCH v2 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL " Philippe Blain via GitGitGadget
2021-08-28  7:27     ` Elijah Newren
2021-08-28 14:50       ` Philippe Blain
2021-08-28  0:47   ` [PATCH v2 3/3] test-lib-functions: optionally keep HOME and TERM in 'debug' Philippe Blain via GitGitGadget
2021-09-01 13:31   ` [PATCH v3 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Philippe Blain via GitGitGadget
2021-09-01 13:31     ` [PATCH v3 1/3] test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause' Philippe Blain via GitGitGadget
2021-09-01 20:04       ` Junio C Hamano
2021-09-01 13:31     ` [PATCH v3 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL " Philippe Blain via GitGitGadget
2021-09-01 20:26       ` Junio C Hamano
2021-09-01 21:52         ` Elijah Newren
2021-09-01 23:09           ` Junio C Hamano
2021-09-02 13:10             ` Philippe Blain
2021-09-01 13:31     ` [PATCH v3 3/3] test-lib-functions: optionally keep HOME and TERM in 'debug' Philippe Blain via GitGitGadget
2021-09-06  4:20     ` [PATCH v4 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Philippe Blain via GitGitGadget
2021-09-06  4:20       ` [PATCH v4 1/3] test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause' Philippe Blain via GitGitGadget
2021-09-06  4:20       ` [PATCH v4 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL " Philippe Blain via GitGitGadget
2021-09-06  4:20       ` [PATCH v4 3/3] test-lib-functions: keep user's debugger config files and TERM in 'debug' Philippe Blain via GitGitGadget
2021-09-06  4:38       ` [PATCH v5 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Philippe Blain via GitGitGadget
2021-09-06  4:38         ` [PATCH v5 1/3] test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause' Philippe Blain via GitGitGadget
2021-09-06  4:38         ` [PATCH v5 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL " Philippe Blain via GitGitGadget
2021-09-06  4:39         ` [PATCH v5 3/3] test-lib-functions: keep user's debugger config files and TERM in 'debug' Philippe Blain via GitGitGadget
2021-09-07  6:24         ` [PATCH v5 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Elijah Newren

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=CAPig+cR7HhWN8F1G7_rWpD_jXfaKqaj7c4fMbkgAGvcjf1pX6g@mail.gmail.com \
    --to=sunshine@sunshineco.com \
    --cc=Jens.Lehmann@web.de \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=levraiphilippeblain@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=newren@gmail.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).