"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: storing regress test data outside of /tmp
To:
Christian Weisgerber <naddy@mips.inka.de>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 1 Oct 2020 20:58:21 +0200

Download raw body.

Thread
On Thu, Oct 01, 2020 at 06:30:16PM -0000, Christian Weisgerber wrote:
> On 2020-09-30, Stefan Sperling <stsp@stsp.name> wrote:
> 
> > This patch adds a flag which can be used to override the default path.
> 
> Comments below:
> 
> > --- regress/cmdline/Makefile
> > +++ regress/cmdline/Makefile
> > @@ -3,79 +3,81 @@ REGRESS_TARGETS=checkout update status log add rm diff
> >  	integrate stage unstage cat clone fetch tree
> >  NOOBJ=Yes
> >  
> > +GOT_TEST_ROOT=/tmp
> > +
> >  checkout:
> > -	./checkout.sh -q
> > +	./checkout.sh -q -r $(GOT_TEST_ROOT)
> 
> I'd prefer a quoted variable:
>         ./checkout.sh -q -r "$(GOT_TEST_ROOT)"
> 
> > --- regress/cmdline/common.sh
> > +++ regress/cmdline/common.sh
> > @@ -21,6 +21,7 @@ export GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL"
> >  export GOT_AUTHOR="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
> >  export GOT_AUTHOR_8="flan_hac"
> >  export GOT_LOG_DEFAULT_LIMIT=0
> > +export GOT_TEST_ROOT="/tmp"
> >  
> >  export MALLOC_OPTIONS=S
> >  
> > @@ -168,7 +169,7 @@ test_init()
> >  		echo "No test name provided" >&2
> >  		return 1
> >  	fi
> > -	local testroot=`mktemp -p /tmp -d got-test-$testname-XXXXXXXX`
> > +	local testroot=`mktemp -p $GOT_TEST_ROOT -d got-test-$testname-XXXXXXXX`
> 
> Quotes here, too.

Sure, we can add quotes. Note though that paths with spaces do not
work regardless because getopt(1) does not process such arguments
correctly (see the BUGS section in its man page).

> While you're here: There are at least three variants of mktemp(1)
> around (OpenBSD, FreeBSD, GNU), which various differences and an
> overly complicated temporary directory handling. E.g. -p path will
> actually be overriden by TMPDIR.  How about using a simple, explicit
> form:
> 
>     local testroot=$(mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX")

Nice. I like this. I used backticks instead of $() in order to keep
the line length limited to 79 bytes instead of 80.

diff refs/heads/main refs/heads/regress
blob - 6fbe310018be9c6d976c853669c68b9cd4a49689
blob + 6f504d14b445f1805f5feadda1decb3b12206fb6
--- README
+++ README
@@ -39,6 +39,14 @@ To test with packed repositories, run:
 
  $ make regress GOT_TEST_PACK=1
 
+Because got unveils the /tmp directory by default using the /tmp directory
+for test data can hide bugs. However, /tmp remains the default because
+there is no better alternative that works out of the box. In order to
+store test data in a directory other than /tmp, such as ~/got-test, run:
+
+ $ mkdir ~/got-test
+ $ make regress GOT_TEST_ROOT=~/got-test
+
 Man page files in the Got source tree can be viewed with 'man -l':
 
  $ man -l got/got.1
blob - 81d463f7c384a8358cd95fa4934dc5b1c013e531
blob + e32b5a142455da777776713187383ad1166a01d4
--- regress/cmdline/Makefile
+++ regress/cmdline/Makefile
@@ -3,79 +3,81 @@ REGRESS_TARGETS=checkout update status log add rm diff
 	integrate stage unstage cat clone fetch tree
 NOOBJ=Yes
 
+GOT_TEST_ROOT=/tmp
+
 checkout:
-	./checkout.sh -q
+	./checkout.sh -q -r "$(GOT_TEST_ROOT)"
 
 update:
-	./update.sh -q
+	./update.sh -q -r "$(GOT_TEST_ROOT)"
 
 status:
-	./status.sh -q
+	./status.sh -q -r "$(GOT_TEST_ROOT)"
 
 log:
-	./log.sh -q
+	./log.sh -q -r "$(GOT_TEST_ROOT)"
 
 add:
-	./add.sh -q
+	./add.sh -q -r "$(GOT_TEST_ROOT)"
 
 rm:
-	./rm.sh -q
+	./rm.sh -q -r "$(GOT_TEST_ROOT)"
 
 diff:
-	./diff.sh -q
+	./diff.sh -q -r "$(GOT_TEST_ROOT)"
 
 blame:
-	./blame.sh -q
+	./blame.sh -q -r "$(GOT_TEST_ROOT)"
 
 branch:
-	./branch.sh -q
+	./branch.sh -q -r "$(GOT_TEST_ROOT)"
 
 tag:
-	./tag.sh -q
+	./tag.sh -q -r "$(GOT_TEST_ROOT)"
 
 ref:
-	./ref.sh -q
+	./ref.sh -q -r "$(GOT_TEST_ROOT)"
 
 commit:
-	./commit.sh -q
+	./commit.sh -q -r "$(GOT_TEST_ROOT)"
 
 revert:
-	./revert.sh -q
+	./revert.sh -q -r "$(GOT_TEST_ROOT)"
 
 cherrypick:
-	./cherrypick.sh -q
+	./cherrypick.sh -q -r "$(GOT_TEST_ROOT)"
 
 backout:
-	./backout.sh -q
+	./backout.sh -q -r "$(GOT_TEST_ROOT)"
 
 rebase:
-	./rebase.sh -q
+	./rebase.sh -q -r "$(GOT_TEST_ROOT)"
 
 import:
-	./import.sh -q
+	./import.sh -q -r "$(GOT_TEST_ROOT)"
 
 histedit:
-	./histedit.sh -q
+	./histedit.sh -q -r "$(GOT_TEST_ROOT)"
 
 integrate:
-	./integrate.sh -q
+	./integrate.sh -q -r "$(GOT_TEST_ROOT)"
 
 stage:
-	./stage.sh -q
+	./stage.sh -q -r "$(GOT_TEST_ROOT)"
 
 unstage:
-	./unstage.sh -q
+	./unstage.sh -q -r "$(GOT_TEST_ROOT)"
 
 cat:
-	./cat.sh -q
+	./cat.sh -q -r "$(GOT_TEST_ROOT)"
 
 clone:
-	./clone.sh -q
+	./clone.sh -q -r "$(GOT_TEST_ROOT)"
 
 fetch:
-	./fetch.sh -q
+	./fetch.sh -q -r "$(GOT_TEST_ROOT)"
 
 tree:
-	./tree.sh -q
+	./tree.sh -q -r "$(GOT_TEST_ROOT)"
 
 .include <bsd.regress.mk>
blob - aa03589cae17845657ef3efe99e9b11b314d2dca
blob + cde580f8704a206fe481381cd52f87d471ee007f
--- regress/cmdline/common.sh
+++ regress/cmdline/common.sh
@@ -21,6 +21,7 @@ export GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL"
 export GOT_AUTHOR="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
 export GOT_AUTHOR_8="flan_hac"
 export GOT_LOG_DEFAULT_LIMIT=0
+export GOT_TEST_ROOT="/tmp"
 
 export MALLOC_OPTIONS=S
 
@@ -168,7 +169,7 @@ test_init()
 		echo "No test name provided" >&2
 		return 1
 	fi
-	local testroot=`mktemp -p /tmp -d got-test-$testname-XXXXXXXX`
+	local testroot=$(mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX")
 	mkdir $testroot/repo
 	git_init $testroot/repo
 	if [ -z "$no_tree" ]; then
@@ -199,10 +200,11 @@ test_cleanup()
 
 test_parseargs()
 {
-	args=`getopt q $*`
+	args=`getopt qr: $*`
 	if [ $? -ne 0 ]; then
 		echo "Supported options:"
 		echo "  -q: quiet mode"
+		echo "  -r PATH: use PATH as test data root directory"
 		exit 2
 	fi
 	set -- $args
@@ -211,6 +213,8 @@ test_parseargs()
 		in
 			-q)
 			   export GOT_TEST_QUIET=1; shift;;
+			-r)
+			   export GOT_TEST_ROOT="$2"; shift; shift;;
 			--)
 			   shift; break;;
 		esac
blob - 6d8e522f63d95a3fa30f3656a34b5f91f348db45
blob + 5d5816f7ae1012a37a6130b169e44eaf28fe5038
--- regress/cmdline/import.sh
+++ regress/cmdline/import.sh
@@ -18,7 +18,7 @@
 
 test_import_basic() {
 	local testname=import_basic
-	local testroot=`mktemp -p /tmp -d got-test-$testname-XXXXXXXX`
+	local testroot=`mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX"`
 
 	got init $testroot/repo
 
@@ -170,7 +170,7 @@ test_import_requires_new_branch() {
 
 test_import_ignores() {
 	local testname=import_ignores
-	local testroot=`mktemp -p /tmp -d got-test-$testname-XXXXXXXX`
+	local testroot=`mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX"`
 
 	got init $testroot/repo
 
@@ -200,7 +200,7 @@ test_import_ignores() {
 
 test_import_empty_dir() {
 	local testname=import_empty_dir
-	local testroot=`mktemp -p /tmp -d got-test-$testname-XXXXXXXX`
+	local testroot=`mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX"`
 
 	got init $testroot/repo
 
@@ -243,7 +243,7 @@ test_import_empty_dir() {
 
 test_import_symlink() {
 	local testname=import_symlink
-	local testroot=`mktemp -p /tmp -d got-test-$testname-XXXXXXXX`
+	local testroot=`mktemp -d "$GOT_TEST_ROOT/got-test-$testname-XXXXXXXX"`
 
 	got init $testroot/repo