all: add support for txtar extension and prefer it (#159)

mvdan · web-flow · commit af73bbc5c731 · 2022-06-24T11:42:57.000+01:00
For backwards compatibility, both testscript and goproxytest, which used to glob on `*.txt`, now look for both file extensions. Note that this required a bit of a refactor in testscript, as we cannot use a single glob expression to accomplish this. Code which produces files, such as txtar-addmod, now produces `*.txtar` rather than `*.txt`. Similarly, the public docs now use `*.txtar` too. Note that we leave many txt files in tests untouched; it's unnecessary to change them given the backwards compatibility, and it has zero benefit to the user as they aren't public. Moreover, the diff churn would make this patch harder to review. If a future version of go-internal only supports txtar extensions, then it could replace all of those extensions accordingly. Fixes #126.
diff --git a/.gitattributes b/.gitattributes
@@ -7,3 +7,4 @@
 # example, we want to allow a .txt file to contain a CRLF line ending
 *.go    text eol=lf
 *.txt   text eol=lf
+*.txtar text eol=lf
diff --git a/cmd/testscript/README.md b/cmd/testscript/README.md
@@ -37,7 +37,7 @@ script, and does not remove that directory when testscript exits.
 Examples
 ========
 
-The following example, fruit.txt, shows a simple reproduction that includes
+The following example, fruit.txtar, shows a simple reproduction that includes
 .gomodproxy supporting files:
 
     go get -m fruit.com
@@ -58,7 +58,7 @@ The following example, fruit.txt, shows a simple reproduction that includes
 
     const Name = "Apple"
 
-Running testscript -v fruit.txt we get:
+Running testscript -v fruit.txtar we get:
 
     ...
     > go get -m fruit.com
@@ -76,7 +76,7 @@ Running testscript -v fruit.txt we get:
     PASS
 
 
-The following example, goimports.txt, shows a simple reproduction involving
+The following example, goimports.txtar, shows a simple reproduction involving
 goimports:
 
     go install golang.org/x/tools/cmd/goimports
@@ -95,7 +95,7 @@ goimports:
 
     const Pi = math.Pi
 
-Running testscript -v goimports.txt we get:
+Running testscript -v goimports.txtar we get:
 
     ...
     > go install golang.org/x/tools/cmd/goimports
diff --git a/cmd/testscript/help.go b/cmd/testscript/help.go
@@ -48,7 +48,7 @@ script, and does not remove that directory when testscript exits.
 Examples
 ========
 
-The following example, fruit.txt, shows a simple reproduction that includes
+The following example, fruit.txtar, shows a simple reproduction that includes
 .gomodproxy supporting files:
 
     go get -m fruit.com
@@ -69,7 +69,7 @@ The following example, fruit.txt, shows a simple reproduction that includes
 
     const Name = "Apple"
 
-Running testscript -v fruit.txt we get:
+Running testscript -v fruit.txtar we get:
 
     ...
     > go get -m fruit.com
@@ -87,7 +87,7 @@ Running testscript -v fruit.txt we get:
     PASS
 
 
-The following example, goimports.txt, shows a simple reproduction involving
+The following example, goimports.txtar, shows a simple reproduction involving
 goimports:
 
     go install golang.org/x/tools/cmd/goimports
@@ -106,7 +106,7 @@ goimports:
 
     const Pi = math.Pi
 
-Running testscript -v goimports.txt we get:
+Running testscript -v goimports.txtar we get:
 
     ...
     > go install golang.org/x/tools/cmd/goimports
diff --git a/cmd/testscript/main.go b/cmd/testscript/main.go
@@ -196,7 +196,7 @@ func (tr *testRunner) run(runDir, filename string) error {
 		return fmt.Errorf("failed to write .gomodproxy files: %v", err)
 	}
 
-	scriptFile := filepath.Join(runDir, "script.txt")
+	scriptFile := filepath.Join(runDir, "script.txtar")
 
 	if err := ioutil.WriteFile(scriptFile, txtar.Format(&script), 0666); err != nil {
 		return fmt.Errorf("failed to write script for %v: %v", renderFilename(filename), err)
diff --git a/cmd/testscript/testdata/work.txt b/cmd/testscript/testdata/work.txt
@@ -12,8 +12,8 @@ testscript -v -work file.txt dir/file.txt
 stderr '^temporary work directory: \Q'$WORK'\E[/\\]\.tmp[/\\]'
 stderr '^temporary work directory for file.txt: \Q'$WORK'\E[/\\]\.tmp[/\\]'
 stderr '^temporary work directory for dir[/\\]file.txt: \Q'$WORK'\E[/\\]\.tmp[/\\]'
-expandone $WORK/.tmp/testscript*/file.txt/script.txt
-expandone $WORK/.tmp/testscript*/file.txt1/script.txt
+expandone $WORK/.tmp/testscript*/file.txt/script.txtar
+expandone $WORK/.tmp/testscript*/file.txt1/script.txtar
 
 -- file.txt --
 >exec true
diff --git a/cmd/txtar-addmod/addmod.go b/cmd/txtar-addmod/addmod.go
@@ -202,7 +202,7 @@ func main1() int {
 				break
 			}
 		} else {
-			if err := ioutil.WriteFile(filepath.Join(targetDir, modDir+".txt"), data, 0666); err != nil {
+			if err := ioutil.WriteFile(filepath.Join(targetDir, modDir+".txtar"), data, 0666); err != nil {
 				log.Printf("%s: %v", arg, err)
 				exitCode = 1
 				continue
diff --git a/cmd/txtar-addmod/testdata/encode.txt b/cmd/txtar-addmod/testdata/encode.txt
@@ -2,4 +2,4 @@ mkdir $WORK/out
 txtar-addmod $WORK/out github.com/shurcooL/httpfs@v0.0.0-20171119174359-809beceb2371
 ! stdout .+
 ! stderr .+
-exists $WORK/out/github.com_shurcoo!l_httpfs_v0.0.0-20171119174359-809beceb2371.txt
+exists $WORK/out/github.com_shurcoo!l_httpfs_v0.0.0-20171119174359-809beceb2371.txtar
diff --git a/cmd/txtar-addmod/testdata/txtar-addmod-self.txt b/cmd/txtar-addmod/testdata/txtar-addmod-self.txt
@@ -2,8 +2,8 @@ mkdir $WORK/out
 txtar-addmod $WORK/out github.com/gobin-testrepos/simple-main
 ! stdout .+
 ! stderr .+
-exists $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txt
-! grep foobar $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txt
+exists $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txtar
+! grep foobar $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txtar
 
 txtar-addmod -all $WORK/out github.com/gobin-testrepos/simple-main
-grep '-- foobar --' $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txt
+grep '-- foobar --' $WORK/out/github.com_gobin-testrepos_simple-main_v1.0.0.txtar
diff --git a/cmd/txtar-c/savedir.go b/cmd/txtar-c/savedir.go
@@ -6,7 +6,7 @@
 //
 // Usage:
 //
-//	txtar-c /path/to/dir >saved.txt
+//	txtar-c /path/to/dir >saved.txtar
 //
 // See https://godoc.org/github.com/rogpeppe/go-internal/txtar for details of the format
 // and how to parse a txtar file.
@@ -30,7 +30,7 @@ import (
 var flag = stdflag.NewFlagSet(os.Args[0], stdflag.ContinueOnError)
 
 func usage() {
-	fmt.Fprintf(os.Stderr, "usage: txtar-c dir >saved.txt\n")
+	fmt.Fprintf(os.Stderr, "usage: txtar-c dir >saved.txtar\n")
 	flag.PrintDefaults()
 }
 
diff --git a/goproxytest/proxy.go b/goproxytest/proxy.go
@@ -7,9 +7,9 @@ Package goproxytest serves Go modules from a proxy server designed to run on
 localhost during tests, both to make tests avoid requiring specific network
 servers and also to make them significantly faster.
 
-Each module archive is either a file named path_vers.txt or a directory named
-path_vers, where slashes in path have been replaced with underscores. The
-archive or directory must contain two files ".info" and ".mod", to be served as
+Each module archive is either a file named path_vers.txtar or path_vers.txt, or
+a directory named path_vers, where slashes in path have been replaced with underscores.
+The archive or directory must contain two files ".info" and ".mod", to be served as
 the info and mod files in the proxy protocol (see
 https://research.swtch.com/vgo-module).  The remaining files are served as the
 content of the module zip file.  The path@vers prefix required of files in the
@@ -96,10 +96,15 @@ func (srv *Server) readModList() error {
 	}
 	for _, info := range infos {
 		name := info.Name()
-		if !strings.HasSuffix(name, ".txt") && !info.IsDir() {
+		switch {
+		case strings.HasSuffix(name, ".txt"):
+			name = strings.TrimSuffix(name, ".txt")
+		case strings.HasSuffix(name, ".txtar"):
+			name = strings.TrimSuffix(name, ".txtar")
+		case info.IsDir():
+		default:
 			continue
 		}
-		name = strings.TrimSuffix(name, ".txt")
 		i := strings.LastIndex(name, "_v")
 		if i < 0 {
 			continue
@@ -281,13 +286,17 @@ func (srv *Server) readArchive(path, vers string) *txtar.Archive {
 	}
 
 	prefix := strings.Replace(enc, "/", "_", -1)
-	name := filepath.Join(srv.dir, prefix+"_"+encVers+".txt")
+	name := filepath.Join(srv.dir, prefix+"_"+encVers)
+	txtName := name + ".txt"
+	txtarName := name + ".txtar"
 	a := srv.archiveCache.Do(name, func() interface{} {
-		a, err := txtar.ParseFile(name)
+		a, err := txtar.ParseFile(txtarName)
 		if os.IsNotExist(err) {
-			// we fallback to trying a directory
-			name = strings.TrimSuffix(name, ".txt")
-
+			// fall back to trying with the .txt extension
+			a, err = txtar.ParseFile(txtName)
+		}
+		if os.IsNotExist(err) {
+			// fall back to trying a directory
 			a = new(txtar.Archive)
 
 			err = filepath.Walk(name, func(path string, info os.FileInfo, err error) error {
diff --git a/testscript/doc.go b/testscript/doc.go
@@ -14,10 +14,10 @@ To invoke the tests, call testscript.Run. For example:
 		})
 	}
 
-A testscript directory holds test scripts *.txt run during 'go test'.
+A testscript directory holds test scripts with extension txtar or txt run during 'go test'.
 Each script defines a subtest; the exact set of allowable commands in a
 script are defined by the parameters passed to the Run function.
-To run a specific script foo.txt
+To run a specific script foo.txtar or foo.txt, run
 
 	go test cmd/go -run=TestName/^foo$
 
@@ -68,7 +68,7 @@ systems, ".exe" on Windows.
 The script's supporting files are unpacked relative to $WORK
 and then the script begins execution in that
 directory as well. Thus the example above runs in $WORK
-with $WORK/hello.txt containing the listed contents.
+with $WORK/hello.txtar containing the listed contents.
 
 The lines at the top of the script are a sequence of commands to be
 executed by a small script engine in the testscript package (not the system
diff --git a/testscript/testdata/testscript_update_script.txt b/testscript/testdata/testscript_update_script.txt
@@ -1,7 +1,13 @@
+# Check that we support both txt and txtar extensions.
+
 unquote scripts/testscript.txt
 unquote testscript-new.txt
+
+cp scripts/testscript.txt scripts/testscript2.txtar
+
 testscript -update scripts
 cmp scripts/testscript.txt testscript-new.txt
+cmp scripts/testscript2.txtar testscript-new.txt
 
 -- scripts/testscript.txt --
 >fprintargs stdout right
diff --git a/testscript/testscript.go b/testscript/testscript.go
@@ -108,8 +108,8 @@ func (e *Env) T() T {
 // Params holds parameters for a call to Run.
 type Params struct {
 	// Dir holds the name of the directory holding the scripts.
-	// All files in the directory with a .txt suffix will be considered
-	// as test scripts. By default the current directory is used.
+	// All files in the directory with a .txtar or .txt suffix will be
+	// considered as test scripts. By default the current directory is used.
 	// Dir is interpreted relative to the current test directory.
 	Dir string
 
@@ -158,7 +158,7 @@ type Params struct {
 }
 
 // RunDir runs the tests in the given directory. All files in dir with a ".txt"
-// are considered to be test files.
+// or ".txtar" extension are considered to be test files.
 func Run(t *testing.T, p Params) {
 	RunT(tshim{t}, p)
 }
@@ -200,13 +200,22 @@ func (t tshim) Verbose() bool {
 // RunT is like Run but uses an interface type instead of the concrete *testing.T
 // type to make it possible to use testscript functionality outside of go test.
 func RunT(t T, p Params) {
-	glob := filepath.Join(p.Dir, "*.txt")
-	files, err := filepath.Glob(glob)
-	if err != nil {
+	entries, err := os.ReadDir(p.Dir)
+	if os.IsNotExist(err) {
+		// Continue so we give a helpful error on len(files)==0 below.
+	} else if err != nil {
 		t.Fatal(err)
 	}
+	var files []string
+	for _, entry := range entries {
+		name := entry.Name()
+		if strings.HasSuffix(name, ".txtar") || strings.HasSuffix(name, ".txt") {
+			files = append(files, filepath.Join(p.Dir, name))
+		}
+	}
+
 	if len(files) == 0 {
-		t.Fatal(fmt.Sprintf("no scripts found matching glob: %v", glob))
+		t.Fatal(fmt.Sprintf("no txtar nor txt scripts found in dir %s", p.Dir))
 	}
 	testTempDir := p.WorkdirRoot
 	if testTempDir == "" {
@@ -814,7 +823,7 @@ func (ts *TestScript) Getenv(key string) string {
 // parse parses a single line as a list of space-separated arguments
 // subject to environment variable expansion (but not resplitting).
 // Single quotes around text disable splitting and expansion.
-// To embed a single quote, double it: 'Don''t communicate by sharing memory.'
+// To embed a single quote, double it: 'Don”t communicate by sharing memory.'
 func (ts *TestScript) parse(line string) []string {
 	ts.line = line
 
diff --git a/testscript/testscript_test.go b/testscript/testscript_test.go
@@ -312,7 +312,7 @@ func TestBadDir(t *testing.T) {
 	if got := len(ft.failMsgs); got != wantCount {
 		t.Fatalf("expected %v fail message; got %v", wantCount, got)
 	}
-	wantMsg := regexp.MustCompile(`no scripts found matching glob: thiswillnevermatch[/\\]\*\.txt`)
+	wantMsg := regexp.MustCompile(`no txtar nor txt scripts found in dir thiswillnevermatch`)
 	if got := ft.failMsgs[0]; !wantMsg.MatchString(got) {
 		t.Fatalf("expected msg to match `%v`; got:\n%v", wantMsg, got)
 	}

Original file line number	Diff line number	Diff line change
`@@ -196,7 +196,7 @@ func (tr *testRunner) run(runDir, filename string) error {`
`196`	`196`	`return fmt.Errorf("failed to write .gomodproxy files: %v", err)`
`197`	`197`	`}`
`198`	`198`
`199`		`- scriptFile := filepath.Join(runDir, "script.txt")`
	`199`	`+ scriptFile := filepath.Join(runDir, "script.txtar")`
`200`	`200`
`201`	`201`	`if err := ioutil.WriteFile(scriptFile, txtar.Format(&script), 0666); err != nil {`
`202`	`202`	`return fmt.Errorf("failed to write script for %v: %v", renderFilename(filename), err)`
Original file line number	Diff line number	Diff line change
`@@ -202,7 +202,7 @@ func main1() int {`
`202`	`202`	`break`
`203`	`203`	`}`
`204`	`204`	`} else {`
`205`		`- if err := ioutil.WriteFile(filepath.Join(targetDir, modDir+".txt"), data, 0666); err != nil {`
	`205`	`+ if err := ioutil.WriteFile(filepath.Join(targetDir, modDir+".txtar"), data, 0666); err != nil {`
`206`	`206`	`log.Printf("%s: %v", arg, err)`
`207`	`207`	`exitCode = 1`
`208`	`208`	`continue`