Skip to content

Commit

Permalink
git p4 test: do not pollute /tmp
Browse files Browse the repository at this point in the history
Generating the submit template for p4 uses tempfile.mkstemp(),
which by default puts files in /tmp.  For a test that fails,
possibly on purpose, this is not cleaned up.  Run with TMPDIR
pointing into the trash directory so the temp files go away
with the test results.

To do this required some other minor changes.  First, the editor
is launched using system(editor + " " + template_file), using
shell expansion to build the command string.  This doesn't work
if editor has a space in it.  And is generally unwise as it's
easy to fool the shell into doing extra work.  Exec the args
directly, without shell expansion.

Second, without shell expansion, the trick of "P4EDITOR=:" used
in the tests doesn't work.  Use a real command, true, as the
non-interactive editor for testing.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Pete Wyckoff authored and Junio C Hamano committed Jan 22, 2014
1 parent 0055b56 commit 0cf1b72
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
2 changes: 1 addition & 1 deletion git-p4.py
Original file line number Diff line number Diff line change
Expand Up @@ -1220,7 +1220,7 @@ def edit_template(self, template_file):
editor = os.environ.get("P4EDITOR")
else:
editor = read_pipe("git var GIT_EDITOR").strip()
system(editor + " " + template_file)
system([editor, template_file])

# If the file was not saved, prompt to see if this patch should
# be skipped. But skip this verification step if configured so.
Expand Down
8 changes: 7 additions & 1 deletion t/lib-git-p4.sh
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
P4PORT=localhost:$P4DPORT
P4CLIENT=client
P4USER=author
P4EDITOR=:
P4EDITOR=true
unset P4CHARSET
export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET

Expand All @@ -57,6 +57,12 @@ cli="$TRASH_DIRECTORY/cli"
git="$TRASH_DIRECTORY/git"
pidfile="$TRASH_DIRECTORY/p4d.pid"

# git p4 submit generates a temp file, which will
# not get cleaned up if the submission fails. Don't
# clutter up /tmp on the test machine.
TMPDIR="$TRASH_DIRECTORY"
export TMPDIR

start_p4d() {
mkdir -p "$db" "$cli" "$git" &&
rm -f "$pidfile" &&
Expand Down
6 changes: 4 additions & 2 deletions t/t9805-git-p4-skip-submit-edit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ test_expect_success 'init depot' '
)
'

# this works because EDITOR is set to :
# this works because P4EDITOR is set to true
test_expect_success 'no config, unedited, say yes' '
git p4 clone --dest="$git" //depot &&
test_when_finished cleanup_git &&
Expand Down Expand Up @@ -90,7 +90,9 @@ test_expect_success 'no config, edited' '
cd "$git" &&
echo line >>file1 &&
git commit -a -m "change 5" &&
P4EDITOR="" EDITOR="\"$TRASH_DIRECTORY/ed.sh\"" git p4 submit &&
P4EDITOR="$TRASH_DIRECTORY/ed.sh" &&
export P4EDITOR &&
git p4 submit &&
p4 changes //depot/... >wc &&
test_line_count = 5 wc
)
Expand Down

0 comments on commit 0cf1b72

Please sign in to comment.