From 4a48392e1794d95f0dc6356deca0ada6ea713f48 Mon Sep 17 00:00:00 2001 From: Cedric BAIL Date: Thu, 4 May 2023 15:49:11 -0600 Subject: [PATCH 1/7] Propagate GOFLAGS from OS environment. --- internal/command/context.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/command/context.go b/internal/command/context.go index 43264de..accdd3b 100644 --- a/internal/command/context.go +++ b/internal/command/context.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "fmt" + "os" "path/filepath" "runtime" "strconv" @@ -173,6 +174,10 @@ func makeDefaultContext(flags *CommonFlags, args []string) (Context, error) { return ctx, err } + if os.Getenv("GOFLAGS") != "" { + ctx.Env["GOFLAGS"] = os.Getenv("GOFLAGS") + } + if len(flags.Ldflags) > 0 { goflags := "" for _, ldflags := range strings.Fields(flags.Ldflags) { From 6e1f6a1e8af9d532422e164a77091517fc11146d Mon Sep 17 00:00:00 2001 From: Cedric BAIL Date: Thu, 4 May 2023 15:49:25 -0600 Subject: [PATCH 2/7] Correctly escape just the value when necessary --- internal/command/docker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/command/docker.go b/internal/command/docker.go index e3a8e5b..40a0a16 100644 --- a/internal/command/docker.go +++ b/internal/command/docker.go @@ -79,7 +79,7 @@ func AppendEnv(args []string, environs map[string]string, quoteNeeded bool) []st if quoteNeeded && strings.Contains(v, "=") { // engine requires to double quote the env var when value contains // the `=` char - env = fmt.Sprintf("%q", env) + env = fmt.Sprintf("%s=%q", k, v) } args = append(args, "-e", env) } From a4c8f0a86c647583ac0e20c801bfb5a03bc98338 Mon Sep 17 00:00:00 2001 From: Cedric BAIL Date: Sat, 6 May 2023 08:12:20 -0600 Subject: [PATCH 3/7] Update internal/command/context.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jacob Alzén --- internal/command/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/command/context.go b/internal/command/context.go index accdd3b..ae45a66 100644 --- a/internal/command/context.go +++ b/internal/command/context.go @@ -174,8 +174,8 @@ func makeDefaultContext(flags *CommonFlags, args []string) (Context, error) { return ctx, err } - if os.Getenv("GOFLAGS") != "" { - ctx.Env["GOFLAGS"] = os.Getenv("GOFLAGS") + if env := os.Getenv("GOFLAGS"); env != "" { + ctx.Env["GOFLAGS"] = env } if len(flags.Ldflags) > 0 { From 8adafbfb9c333b13b3ff2b5bd3c74838fef715bd Mon Sep 17 00:00:00 2001 From: Cedric BAIL Date: Fri, 5 Jan 2024 21:25:43 -0700 Subject: [PATCH 4/7] Add unit test. --- internal/command/docker_test.go | 103 ++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/internal/command/docker_test.go b/internal/command/docker_test.go index abf8097..f81776a 100644 --- a/internal/command/docker_test.go +++ b/internal/command/docker_test.go @@ -268,6 +268,109 @@ func TestCmdEnginePodman(t *testing.T) { } } +func TestAppendEnv(t *testing.T) { + type args struct { + args []string + env map[string]string + quoteNeeded bool + } + tests := []struct { + name string + args args + wantStart []string + wantEnd [][2]string + }{ + { + name: "empty", + args: args{ + args: []string{}, + env: map[string]string{}, + quoteNeeded: true, + }, + wantStart: []string{}, + wantEnd: [][2]string{}, + }, + { + name: "quote needed", + args: args{ + args: []string{}, + env: map[string]string{"VAR": "value"}, + quoteNeeded: true, + }, + wantStart: []string{}, + wantEnd: [][2]string{{"-e", "VAR=value"}}, + }, + { + name: "quote not needed", + args: args{ + args: []string{}, + env: map[string]string{"VAR": "value"}, + quoteNeeded: false, + }, + wantStart: []string{}, + wantEnd: [][2]string{{"-e", "VAR=value"}}, + }, + { + name: "multiple", + args: args{ + args: []string{}, + env: map[string]string{"VAR": "value", "VAR2": "value2"}, + quoteNeeded: true, + }, + wantStart: []string{}, + wantEnd: [][2]string{{"-e", "VAR=value"}, {"-e", "VAR2=value2"}}, + }, + { + name: "multiple with args", + args: args{ + args: []string{"arg1", "arg2"}, + env: map[string]string{"VAR": "value", "VAR2": "value2"}, + quoteNeeded: true, + }, + wantStart: []string{"arg1", "arg2"}, + wantEnd: [][2]string{{"-e", "VAR=value"}, {"-e", "VAR2=value2"}}, + }, + { + name: "multiple with args and equal sign require quoting values", + args: args{ + args: []string{"arg1", "arg2"}, + env: map[string]string{"VAR": "value", "VAR2": "value2=2"}, + quoteNeeded: true, + }, + wantStart: []string{"arg1", "arg2"}, + wantEnd: [][2]string{{"-e", "VAR=value"}, {"-e", "VAR2=\"value2=2\""}}, + }, + { + name: "multiple with args and equal sign do not require quoting values", + args: args{ + args: []string{"arg1", "arg2"}, + env: map[string]string{"VAR": "value", "VAR2": "value2=2"}, + quoteNeeded: false, + }, + wantStart: []string{"arg1", "arg2"}, + wantEnd: [][2]string{{"-e", "VAR=value"}, {"-e", "VAR2=value2=2"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AppendEnv(tt.args.args, tt.args.env, tt.args.quoteNeeded) + for i, v := range tt.wantStart { + assert.Equal(t, v, got[i]) + } + for i := len(tt.wantStart); i < len(got); i += 2 { + found := false + for _, v := range tt.wantEnd { + if v[0] == got[i] && v[1] == got[i+1] { + found = true + break + } + } + assert.Equal(t, true, found) + } + }) + } +} + func TestMain(m *testing.M) { os.Unsetenv("SSH_AUTH_SOCK") os.Exit(m.Run()) From 80fe656dd9087ee46d905a1e0541eed4a718b96b Mon Sep 17 00:00:00 2001 From: Cedric BAIL Date: Fri, 5 Jan 2024 21:25:51 -0700 Subject: [PATCH 5/7] Try to make the comment less confusing. --- internal/command/docker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/command/docker.go b/internal/command/docker.go index 40a0a16..3193224 100644 --- a/internal/command/docker.go +++ b/internal/command/docker.go @@ -77,7 +77,7 @@ func AppendEnv(args []string, environs map[string]string, quoteNeeded bool) []st for k, v := range environs { env := k + "=" + v if quoteNeeded && strings.Contains(v, "=") { - // engine requires to double quote the env var when value contains + // engine requires to double quote the value when it contains // the `=` char env = fmt.Sprintf("%s=%q", k, v) } From 4eaf13ab44d79917d276ca10092c7ebff927958b Mon Sep 17 00:00:00 2001 From: Cedric BAIL Date: Wed, 10 Jan 2024 22:25:12 -0700 Subject: [PATCH 6/7] Make sure that we actually return all expected argument. --- internal/command/docker_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/command/docker_test.go b/internal/command/docker_test.go index 28a5ab8..cb36fa4 100644 --- a/internal/command/docker_test.go +++ b/internal/command/docker_test.go @@ -364,14 +364,16 @@ func TestAppendEnv(t *testing.T) { } for i := len(tt.wantStart); i < len(got); i += 2 { found := false - for _, v := range tt.wantEnd { + for k, v := range tt.wantEnd { if v[0] == got[i] && v[1] == got[i+1] { + tt.wantEnd = append(tt.wantEnd[:k], tt.wantEnd[k+1:]...) found = true break } } assert.Equal(t, true, found) } + assert.Equal(t, 0, len(tt.wantEnd)) }) } } From fb831a1171c53d62bb0892c31fe8d6c839ed59f0 Mon Sep 17 00:00:00 2001 From: Cedric BAIL Date: Sat, 27 Jan 2024 17:18:28 -0700 Subject: [PATCH 7/7] Use one uniq `i` for both loop. --- internal/command/docker_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/command/docker_test.go b/internal/command/docker_test.go index cb36fa4..895c471 100644 --- a/internal/command/docker_test.go +++ b/internal/command/docker_test.go @@ -359,10 +359,12 @@ func TestAppendEnv(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := AppendEnv(tt.args.args, tt.args.env, tt.args.quoteNeeded) - for i, v := range tt.wantStart { + var i int + for _, v := range tt.wantStart { assert.Equal(t, v, got[i]) + i++ } - for i := len(tt.wantStart); i < len(got); i += 2 { + for ; i < len(got); i += 2 { found := false for k, v := range tt.wantEnd { if v[0] == got[i] && v[1] == got[i+1] {