git p4 test: do not pollute /tmp

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>

diff --git a/git-p4.py b/git-p4.py
index e798ecf48c6c85066c2b90706bc8026f82bd34fb..a4414b58b9e6e313d0789994a42b31ccabecf085 100755 (executable)
--- a/git-p4.py
+++ b/git-p4.py
@@ -1220,7 +1220,7 @@ def edit_template(self, template_file):
             editor = os.environ.get("P4EDITOR")
         else:
             editor = read_pipe("git var GIT_EDITOR").strip()
             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.
 
         # If the file was not saved, prompt to see if this patch should
         # be skipped.  But skip this verification step if configured so.

diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 4ff2bb1bf254a932a6c2d19c0fe58e6c42f62c40..5aa8adcf9c8a53b8a643a37b9474886448f6a45b 100644 (file)
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -48,7 +48,7 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
 P4PORT=localhost:$P4DPORT
 P4CLIENT=client
 P4USER=author
 P4PORT=localhost:$P4DPORT
 P4CLIENT=client
 P4USER=author

-P4EDITOR=:
+P4EDITOR=true

 unset P4CHARSET
 export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET
 
 unset P4CHARSET
 export P4PORT P4CLIENT P4USER P4EDITOR P4CHARSET
 


@@ -57,6 +57,12 @@ cli="$TRASH_DIRECTORY/cli"
 git="$TRASH_DIRECTORY/git"
 pidfile="$TRASH_DIRECTORY/p4d.pid"
 
 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" &&
 start_p4d() {
        mkdir -p "$db" "$cli" "$git" &&
        rm -f "$pidfile" &&

diff --git a/t/t9805-git-p4-skip-submit-edit.sh b/t/t9805-git-p4-skip-submit-edit.sh
index ff2cc79701cce5ff570ff3c11d1ad0b60459d9ef..89311886db168e6613fa98d9c7a340d460435585 100755 (executable)
--- a/t/t9805-git-p4-skip-submit-edit.sh
+++ b/t/t9805-git-p4-skip-submit-edit.sh
@@ -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 &&
 test_expect_success 'no config, unedited, say yes' '
        git p4 clone --dest="$git" //depot &&
        test_when_finished cleanup_git &&


@@ -90,7 +90,9 @@ test_expect_success 'no config, edited' '
                cd "$git" &&
                echo line >>file1 &&
                git commit -a -m "change 5" &&
                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
        )
                p4 changes //depot/... >wc &&
                test_line_count = 5 wc
        )