Skip to content

Commit 721eff6

Browse files
bchobcho
authored
Merge pull request #7 from b4fun/stderr-race
fix: fix with stderr data race
2 parents f7543cf + 14ed58e commit 721eff6

File tree

8 files changed

+240
-18
lines changed

8 files changed

+240
-18
lines changed

‎script/constructor.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
11
package script
22

3-
// TODO: generate constructrs from bitfield/script's source code
3+
// TODO: generate constructors from bitfield/script's source code
44

55
import (
66
"context"
77
"net/http"
8+
"sync"
89

910
"github.com/bitfield/script"
1011
)
1112

1213
func newPipeFrom(pipe *script.Pipe) *Pipe {
13-
return &Pipe{Pipe: pipe}
14+
return &Pipe{
15+
Pipe: pipe,
16+
mu: new(sync.Mutex),
17+
}
1418
}
1519

1620
func NewPipe() *Pipe {

‎script/contextual.go

+36-8
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"os/exec"
1515
"path/filepath"
1616
"strings"
17+
"sync"
1718
"text/template"
1819

1920
"github.com/bitfield/script"
@@ -25,15 +26,30 @@ var NewReadAutoCloser = script.NewReadAutoCloser
2526
type Pipe struct {
2627
*script.Pipe
2728

28-
stderr io.Writer // captured from WithStderr
29-
3029
// wd is the working directory for current pipe.
3130
wd string
31+
32+
mu *sync.Mutex // protects the following fields
33+
34+
stderr io.Writer // captured from WithStderr
35+
3236
// env is the environment variables for current pipe.
3337
// Non-empty value will be set to the exec.Command instance.
3438
env []string
3539
}
3640

41+
func (p *Pipe) environments() []string {
42+
p.mu.Lock()
43+
defer p.mu.Unlock()
44+
return p.env
45+
}
46+
47+
func (p *Pipe) stdErr() io.Writer {
48+
p.mu.Lock()
49+
defer p.mu.Unlock()
50+
return p.stderr
51+
}
52+
3753
func (p *Pipe) At(dir string) *Pipe {
3854
p.wd = dir
3955
return p
@@ -46,23 +62,35 @@ func (p *Pipe) WithCurrentEnv() *Pipe {
4662

4763
// WithEnv sets the environment variables for the current pipe.
4864
func (p *Pipe) WithEnv(env []string) *Pipe {
65+
p.mu.Lock()
66+
defer p.mu.Unlock()
67+
4968
p.env = env
5069
return p
5170
}
5271

5372
// AppendEnv appends the environment variables for the current pipe.
5473
func (p *Pipe) AppendEnv(env ...string) *Pipe {
74+
p.mu.Lock()
75+
defer p.mu.Unlock()
76+
5577
p.env = append(p.env, env...)
5678
return p
5779
}
5880

5981
// WithEnvKV sets the environment variable key-value pair for the current pipe.
6082
func (p *Pipe) WithEnvKV(key, value string) *Pipe {
83+
p.mu.Lock()
84+
defer p.mu.Unlock()
85+
6186
p.env = append(p.env, key+"="+value)
6287
return p
6388
}
6489

6590
func (p *Pipe) WithStderr(w io.Writer) *Pipe {
91+
p.mu.Lock()
92+
defer p.mu.Unlock()
93+
6694
p.stderr = w
6795
p.Pipe = p.Pipe.WithStderr(w)
6896
return p
@@ -132,8 +160,8 @@ func (p *Pipe) execContext(
132160
if p.wd != "" {
133161
cmd.Dir = p.wd
134162
}
135-
if len(p.env) > 0 {
136-
cmd.Env = p.env
163+
if envs := p.environments(); len(envs) > 0 {
164+
cmd.Env = envs
137165
}
138166

139167
return cmd
@@ -150,8 +178,8 @@ func (p *Pipe) ExecContext(ctx context.Context, cmdLine string) *Pipe {
150178
cmd.Stdin = r
151179
cmd.Stdout = w
152180
cmd.Stderr = w
153-
if p.stderr != nil {
154-
cmd.Stderr = p.stderr
181+
if stderr := p.stdErr(); stderr != nil {
182+
cmd.Stderr = stderr
155183
}
156184

157185
if err := cmd.Start(); err != nil {
@@ -189,8 +217,8 @@ func (p *Pipe) ExecForEachContext(ctx context.Context, cmdLine string) *Pipe {
189217
cmd := p.execContext(ctx, args[0], args[1:])
190218
cmd.Stdout = w
191219
cmd.Stderr = w
192-
if p.stderr != nil {
193-
cmd.Stderr = p.stderr
220+
if stderr := p.stdErr(); stderr != nil {
221+
cmd.Stderr = stderr
194222
}
195223
err = cmd.Start()
196224
if err != nil {

‎script/contextual_test.go

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package script
2+
3+
import (
4+
"context"
5+
"testing"
6+
)
7+
8+
func TestWithStdErr_IsConcurrencySafeAfterExec(t *testing.T) {
9+
t.Parallel()
10+
ctx := context.Background()
11+
err := ExecContext(ctx, "echo").WithStderr(nil).Wait()
12+
if err != nil {
13+
t.Fatal(err)
14+
}
15+
}

‎script/go.mod

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
module github.com/b4fun/script-contextual/script
22

3-
go 1.22.3
3+
go 1.22.7
44

55
require (
6-
github.com/bitfield/script v0.22.1
6+
github.com/bitfield/script v0.23.0
77
mvdan.cc/sh/v3 v3.7.0
88
)
99

‎script/go.sum

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
github.com/bitfield/script v0.22.1 h1:DphxoC5ssYciwd0ZS+N0Xae46geAD/0mVWh6a2NUxM4=
2-
github.com/bitfield/script v0.22.1/go.mod h1:fv+6x4OzVsRs6qAlc7wiGq8fq1b5orhtQdtW0dwjUHI=
1+
github.com/bitfield/script v0.23.0 h1:N0R5yLEl6wJIS9PR/A6xXwjMsplMubyxdi05N5l0X28=
2+
github.com/bitfield/script v0.23.0/go.mod h1:fv+6x4OzVsRs6qAlc7wiGq8fq1b5orhtQdtW0dwjUHI=
33
github.com/frankban/quicktest v1.14.5 h1:dfYrrRyLtiqT9GyKXgdh+k4inNeTvmGbuSgZ3lx3GhA=
44
github.com/frankban/quicktest v1.14.5/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
55
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=

‎tests/go.mod

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module tests
22

3-
go 1.22.3
3+
go 1.22.7
44

55
replace github.com/b4fun/script-contextual/script => ../script
66

@@ -11,7 +11,7 @@ require (
1111
)
1212

1313
require (
14-
github.com/bitfield/script v0.22.1 // indirect
14+
github.com/bitfield/script v0.23.0 // indirect
1515
github.com/itchyny/gojq v0.12.13 // indirect
1616
github.com/itchyny/timefmt-go v0.1.5 // indirect
1717
golang.org/x/sys v0.10.0 // indirect

‎tests/go.sum

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
github.com/bitfield/script v0.22.1 h1:DphxoC5ssYciwd0ZS+N0Xae46geAD/0mVWh6a2NUxM4=
2-
github.com/bitfield/script v0.22.1/go.mod h1:fv+6x4OzVsRs6qAlc7wiGq8fq1b5orhtQdtW0dwjUHI=
1+
github.com/bitfield/script v0.23.0 h1:N0R5yLEl6wJIS9PR/A6xXwjMsplMubyxdi05N5l0X28=
2+
github.com/bitfield/script v0.23.0/go.mod h1:fv+6x4OzVsRs6qAlc7wiGq8fq1b5orhtQdtW0dwjUHI=
33
github.com/frankban/quicktest v1.14.5 h1:dfYrrRyLtiqT9GyKXgdh+k4inNeTvmGbuSgZ3lx3GhA=
44
github.com/frankban/quicktest v1.14.5/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
55
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=

‎tests/script_test.go

+175
Original file line numberDiff line numberDiff line change
@@ -1768,6 +1768,40 @@ func TestWithStdout_SetsSpecifiedWriterAsStdout(t *testing.T) {
17681768
}
17691769
}
17701770

1771+
func TestWithEnv_UnsetsAllEnvVarsGivenEmptySlice(t *testing.T) {
1772+
t.Parallel()
1773+
p := script.NewPipe().WithEnv([]string{"ENV1=test1"}).Exec("sh -c 'echo ENV1=$ENV1'")
1774+
want := "ENV1=test1\n"
1775+
got, err := p.String()
1776+
if err != nil {
1777+
t.Fatal(err)
1778+
}
1779+
if got != want {
1780+
t.Fatalf("want %q, got %q", want, got)
1781+
}
1782+
got, err = p.Echo("").WithEnv([]string{}).Exec("sh -c 'echo ENV1=$ENV1'").String()
1783+
if err != nil {
1784+
t.Fatal(err)
1785+
}
1786+
want = "ENV1=\n"
1787+
if got != want {
1788+
t.Errorf("want %q, got %q", want, got)
1789+
}
1790+
}
1791+
1792+
func TestWithEnv_SetsGivenVariablesForSubsequentExec(t *testing.T) {
1793+
t.Parallel()
1794+
env := []string{"ENV1=test1", "ENV2=test2"}
1795+
got, err := script.NewPipe().WithEnv(env).Exec("sh -c 'echo ENV1=$ENV1 ENV2=$ENV2'").String()
1796+
if err != nil {
1797+
t.Fatal(err)
1798+
}
1799+
want := "ENV1=test1 ENV2=test2\n"
1800+
if got != want {
1801+
t.Errorf("want %q, got %q", want, got)
1802+
}
1803+
}
1804+
17711805
func TestErrorReturnsErrorSetByPreviousPipeStage(t *testing.T) {
17721806
t.Parallel()
17731807
p := script.File("testdata/nonexistent.txt")
@@ -1850,6 +1884,135 @@ func TestReadReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
18501884
}
18511885
}
18521886

1887+
func TestWait_ReturnsErrorPresentOnPipe(t *testing.T) {
1888+
t.Parallel()
1889+
p := script.Echo("a\nb\nc\n").ExecForEach("{{invalid template syntax}}")
1890+
if p.Wait() == nil {
1891+
t.Error("want error, got nil")
1892+
}
1893+
}
1894+
1895+
func TestWait_DoesNotReturnErrorForValidExecution(t *testing.T) {
1896+
t.Parallel()
1897+
p := script.Echo("a\nb\nc\n").ExecForEach("echo \"{{.}}\"")
1898+
if err := p.Wait(); err != nil {
1899+
t.Fatal(err)
1900+
}
1901+
}
1902+
1903+
var base64Cases = []struct {
1904+
name string
1905+
decoded string
1906+
encoded string
1907+
}{
1908+
{
1909+
name: "empty string",
1910+
decoded: "",
1911+
encoded: "",
1912+
},
1913+
{
1914+
name: "single line string",
1915+
decoded: "hello world",
1916+
encoded: "aGVsbG8gd29ybGQ=",
1917+
},
1918+
{
1919+
name: "multi line string",
1920+
decoded: "hello\nthere\nworld\n",
1921+
encoded: "aGVsbG8KdGhlcmUKd29ybGQK",
1922+
},
1923+
}
1924+
1925+
func TestEncodeBase64_CorrectlyEncodes(t *testing.T) {
1926+
t.Parallel()
1927+
for _, tc := range base64Cases {
1928+
t.Run(tc.name, func(t *testing.T) {
1929+
got, err := script.Echo(tc.decoded).EncodeBase64().String()
1930+
if err != nil {
1931+
t.Fatal(err)
1932+
}
1933+
if got != tc.encoded {
1934+
t.Logf("input %q incorrectly encoded:", tc.decoded)
1935+
t.Error(cmp.Diff(tc.encoded, got))
1936+
}
1937+
})
1938+
}
1939+
}
1940+
1941+
func TestDecodeBase64_CorrectlyDecodes(t *testing.T) {
1942+
t.Parallel()
1943+
for _, tc := range base64Cases {
1944+
t.Run(tc.name, func(t *testing.T) {
1945+
got, err := script.Echo(tc.encoded).DecodeBase64().String()
1946+
if err != nil {
1947+
t.Fatal(err)
1948+
}
1949+
if got != tc.decoded {
1950+
t.Logf("input %q incorrectly decoded:", tc.encoded)
1951+
t.Error(cmp.Diff(tc.decoded, got))
1952+
}
1953+
})
1954+
}
1955+
}
1956+
1957+
func TestEncodeBase64_FollowedByDecodeRecoversOriginal(t *testing.T) {
1958+
t.Parallel()
1959+
for _, tc := range base64Cases {
1960+
t.Run(tc.name, func(t *testing.T) {
1961+
decoded, err := script.Echo(tc.decoded).EncodeBase64().DecodeBase64().String()
1962+
if err != nil {
1963+
t.Fatal(err)
1964+
}
1965+
if decoded != tc.decoded {
1966+
t.Error("encode-decode round trip failed:", cmp.Diff(tc.decoded, decoded))
1967+
}
1968+
encoded, err := script.Echo(tc.encoded).DecodeBase64().EncodeBase64().String()
1969+
if err != nil {
1970+
t.Fatal(err)
1971+
}
1972+
if encoded != tc.encoded {
1973+
t.Error("decode-encode round trip failed:", cmp.Diff(tc.encoded, encoded))
1974+
}
1975+
})
1976+
}
1977+
}
1978+
1979+
func TestDecodeBase64_CorrectlyDecodesInputToBytes(t *testing.T) {
1980+
t.Parallel()
1981+
input := "CAAAEA=="
1982+
got, err := script.Echo(input).DecodeBase64().Bytes()
1983+
if err != nil {
1984+
t.Fatal(err)
1985+
}
1986+
want := []byte{8, 0, 0, 16}
1987+
if !bytes.Equal(want, got) {
1988+
t.Logf("input %#v incorrectly decoded:", input)
1989+
t.Error(cmp.Diff(want, got))
1990+
}
1991+
}
1992+
1993+
func TestEncodeBase64_CorrectlyEncodesInputBytes(t *testing.T) {
1994+
t.Parallel()
1995+
input := []byte{8, 0, 0, 16}
1996+
reader := bytes.NewReader(input)
1997+
want := "CAAAEA=="
1998+
got, err := script.NewPipe().WithReader(reader).EncodeBase64().String()
1999+
if err != nil {
2000+
t.Fatal(err)
2001+
}
2002+
if got != want {
2003+
t.Logf("input %#v incorrectly encoded:", input)
2004+
t.Error(cmp.Diff(want, got))
2005+
}
2006+
}
2007+
2008+
func TestWithStdErr_IsConcurrencySafeAfterExec(t *testing.T) {
2009+
t.Parallel()
2010+
err := script.Exec("echo").WithStderr(nil).Wait()
2011+
if err != nil {
2012+
t.Fatal(err)
2013+
}
2014+
}
2015+
18532016
func ExampleArgs() {
18542017
script.Args().Stdout()
18552018
// prints command-line arguments
@@ -1969,6 +2132,12 @@ func ExamplePipe_CountLines() {
19692132
// 3
19702133
}
19712134

2135+
func ExamplePipe_DecodeBase64() {
2136+
script.Echo("SGVsbG8sIHdvcmxkIQ==").DecodeBase64().Stdout()
2137+
// Output:
2138+
// Hello, world!
2139+
}
2140+
19722141
func ExamplePipe_Do() {
19732142
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
19742143
data, err := io.ReadAll(r.Body)
@@ -2004,6 +2173,12 @@ func ExamplePipe_Echo() {
20042173
// Hello, world!
20052174
}
20062175

2176+
func ExamplePipe_EncodeBase64() {
2177+
script.Echo("Hello, world!").EncodeBase64().Stdout()
2178+
// Output:
2179+
// SGVsbG8sIHdvcmxkIQ==
2180+
}
2181+
20072182
func ExamplePipe_ExitStatus() {
20082183
p := script.Exec("echo")
20092184
fmt.Println(p.ExitStatus())

0 commit comments

Comments
 (0)