git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 07/11] git p4 test: do not pollute /tmp

From
PWPete Wyckoff <pw@padd.com>
Date
Jan 21, 2014, 23:16 UTC
Message-ID
<1390346208-9207-8-git-send-email-pw@padd.com>
In-Reply-To
<1390346208-9207-1-git-send-email-pw@padd.com>

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>
---
 git-p4.py                          | 2 +-
 t/lib-git-p4.sh                    | 8 +++++++-
 t/t9805-git-p4-skip-submit-edit.sh | 6 ++++--
 3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index e798ecf..a4414b5 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1220,7 +1220,7 @@ class P4Submit(Command, P4UserMap):
             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.
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 4ff2bb1..5aa8adc 100644
--- 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
-P4EDITOR=:
+P4EDITOR=true
 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 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" &&
diff --git a/t/t9805-git-p4-skip-submit-edit.sh b/t/t9805-git-p4-skip-submit-edit.sh
index ff2cc79..8931188 100755
--- 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 &&
@@ -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
 	)
-- 
1.8.5.2.320.g99957e5
Previous: Eric SunshineNext: Pete Wyckoff
Message 10 of 27 in “git p4 tests and a few bug fixes”
  1. 00/11 git p4 tests and a few bug fixesPete Wyckoff, Jan 21, 2014
  2. 01/11 git p4 test: wildcards are supportedPete Wyckoff, Jan 21, 2014
  3. 02/11 git p4 test: ensure p4 symlink parsing worksPete Wyckoff, Jan 21, 2014
  4. 03/11 git p4: work around p4 bug that causes empty symlinksPete Wyckoff, Jan 21, 2014
  5. Eric SunshineJan 22, 2014
  6. 04/11 git p4 test: explicitly check p4 wildcard deletePete Wyckoff, Jan 21, 2014
  7. 05/11 git p4 test: is_cli_file_writeable succeedsPete Wyckoff, Jan 21, 2014
  8. 06/11 git p4 test: run as user "author"Pete Wyckoff, Jan 21, 2014
  9. Eric SunshineJan 22, 2014
  10. 07/11 git p4 test: do not pollute /tmpPete Wyckoff, Jan 21, 2014
  11. 08/11 git p4: handle files with wildcards when doing RCS scrubbingPete Wyckoff, Jan 21, 2014
  12. 09/11 git p4: fix an error message when "p4 where" failsPete Wyckoff, Jan 21, 2014
  13. 10/11 git p4 test: examine behavior with locked (+l) filesPete Wyckoff, Jan 21, 2014
  14. 11/11 git p4 doc: use two-line style for options with multiple spellingsPete Wyckoff, Jan 21, 2014
  15. Junio C HamanoJan 22, 2014
  16. Pete WyckoffJan 22, 2014
  17. 01/11 git p4 test: wildcards are supportedPete Wyckoff, Jan 22, 2014
  18. 02/11 git p4 test: ensure p4 symlink parsing worksPete Wyckoff, Jan 22, 2014
  19. 03/11 git p4: work around p4 bug that causes empty symlinksPete Wyckoff, Jan 22, 2014
  20. 04/11 git p4 test: explicitly check p4 wildcard deletePete Wyckoff, Jan 22, 2014
  21. 05/11 git p4 test: is_cli_file_writeable succeedsPete Wyckoff, Jan 22, 2014
  22. 06/11 git p4 test: run as user "author"Pete Wyckoff, Jan 22, 2014
  23. 07/11 git p4 test: do not pollute /tmpPete Wyckoff, Jan 22, 2014
  24. 08/11 git p4: handle files with wildcards when doing RCS scrubbingPete Wyckoff, Jan 22, 2014
  25. 09/11 git p4: fix an error message when "p4 where" failsPete Wyckoff, Jan 22, 2014
  26. 10/11 git p4 test: examine behavior with locked (+l) filesPete Wyckoff, Jan 22, 2014
  27. 11/11 git p4 doc: use two-line style for options with multiple spellingsPete Wyckoff, Jan 22, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.