Only do new-style env parsing on unix-y OSes

buildkite · Oct 14, 2022 · d61a0a3 · d61a0a3
1 parent 29ef547
commit d61a0a3
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 25 deletions.
diff --git a/hook/scriptwrapper.go b/hook/scriptwrapper.go
@@ -20,19 +20,19 @@ const (
 
 	batchScript = `@echo off
 SETLOCAL ENABLEDELAYEDEXPANSION
-buildkite-agent env > "{{.BeforeEnvFileName}}"
+SET > "{{.BeforeEnvFileName}}"
 CALL "{{.PathToHook}}"
 SET BUILDKITE_HOOK_EXIT_STATUS=!ERRORLEVEL!
 SET BUILDKITE_HOOK_WORKING_DIR=%CD%
-buildkite-agent env > "{{.AfterEnvFileName}}"
+SET > "{{.AfterEnvFileName}}"
 EXIT %BUILDKITE_HOOK_EXIT_STATUS%`
 
 	powershellScript = `$ErrorActionPreference = "STOP"
-buildkite-agent env | Set-Content "{{.BeforeEnvFileName}}"
+Get-ChildItem Env: | Foreach-Object {"$($_.Name)=$($_.Value)"} | Set-Content "{{.BeforeEnvFileName}}"
 {{.PathToHook}}
 if ($LASTEXITCODE -eq $null) {$Env:BUILDKITE_HOOK_EXIT_STATUS = 0} else {$Env:BUILDKITE_HOOK_EXIT_STATUS = $LASTEXITCODE}
 $Env:BUILDKITE_HOOK_WORKING_DIR = $PWD | Select-Object -ExpandProperty Path
-buildkite-agent env | Set-Content "{{.AfterEnvFileName}}"
+Get-ChildItem Env: | Foreach-Object {"$($_.Name)=$($_.Value)"} | Set-Content "{{.AfterEnvFileName}}"
 exit $Env:BUILDKITE_HOOK_EXIT_STATUS`
 
 	bashScript = `buildkite-agent env > "{{.BeforeEnvFileName}}"
@@ -234,14 +234,21 @@ func (wrap *ScriptWrapper) Changes() (HookScriptChanges, error) {
 		beforeEnv env.Environment
 		afterEnv  env.Environment
 	)
-	err = json.Unmarshal(beforeEnvContents, &beforeEnv)
-	if err != nil {
-		return HookScriptChanges{}, fmt.Errorf("failed to unmarshal before env file: %w, file contents: %q", err, string(beforeEnvContents))
-	}
 
-	err = json.Unmarshal(afterEnvContents, &afterEnv)
-	if err != nil {
-		return HookScriptChanges{}, fmt.Errorf("failed to unmarshal after env file: %w, file contents: %q", err, string(afterEnvContents))
+	if runtime.GOOS == "windows" {
+		// TODO: Delete this code branch and parse everything from `buildkite-agent env` output. Everything should work fine on Windows.
+		beforeEnv = env.FromExport(string(beforeEnvContents))
+		afterEnv = env.FromExport(string(afterEnvContents))
+	} else {
+		err = json.Unmarshal(beforeEnvContents, &beforeEnv)
+		if err != nil {
+			return HookScriptChanges{}, fmt.Errorf("failed to unmarshal before env file: %w, file contents: %q", err, string(beforeEnvContents))
+		}
+
+		err = json.Unmarshal(afterEnvContents, &afterEnv)
+		if err != nil {
+			return HookScriptChanges{}, fmt.Errorf("failed to unmarshal after env file: %w, file contents: %q", err, string(afterEnvContents))
+		}
 	}
 
 	if afterEnv.Length() == 0 {

diff --git a/hook/scriptwrapper_test.go b/hook/scriptwrapper_test.go
@@ -38,10 +38,15 @@ func TestRunningHookDetectsChangedEnvironment(t *testing.T) {
 		}
 	}
 
-	agent, cleanup, err := mockAgent()
-	require.NoError(t, err)
+	var agent *bintest.Mock
+	if runtime.GOOS != "windows" {
+		var cleanup func()
+		var err error
+		agent, cleanup, err = mockAgent()
+		require.NoError(t, err)
 
-	defer cleanup()
+		defer cleanup()
+	}
 
 	wrapper := newTestScriptWrapper(t, script)
 	defer os.Remove(wrapper.Path())
@@ -76,8 +81,10 @@ func TestRunningHookDetectsChangedEnvironment(t *testing.T) {
 	// environment variables
 	assert.Equal(t, expected, actual)
 
-	err = agent.CheckAndClose(t)
-	require.NoError(t, err)
+	if runtime.GOOS != "windows" {
+		err = agent.CheckAndClose(t)
+		require.NoError(t, err)
+	}
 }
 
 func TestHookScriptsAreGeneratedCorrectlyOnWindowsBatch(t *testing.T) {
@@ -103,11 +110,11 @@ func TestHookScriptsAreGeneratedCorrectlyOnWindowsBatch(t *testing.T) {
 	// See: https://pkg.go.dev/fmt > ctrl-f for "%%"
 	scriptTemplate := `@echo off
 SETLOCAL ENABLEDELAYEDEXPANSION
-buildkite-agent env > "%s"
+SET > "%s"
 CALL "%s"
 SET BUILDKITE_HOOK_EXIT_STATUS=!ERRORLEVEL!
 SET BUILDKITE_HOOK_WORKING_DIR=%%CD%%
-buildkite-agent env > "%s"
+SET > "%s"
 EXIT %%BUILDKITE_HOOK_EXIT_STATUS%%`
 
 	assertScriptLike(t, scriptTemplate, hookFile.Name(), wrapper)
@@ -133,11 +140,11 @@ func TestHookScriptsAreGeneratedCorrectlyOnWindowsPowershell(t *testing.T) {
 	defer wrapper.Close()
 
 	scriptTemplate := `$ErrorActionPreference = "STOP"
-buildkite-agent env | Set-Content "%s"
+Get-ChildItem Env: | Foreach-Object {"$($_.Name)=$($_.Value)"} | Set-Content "%s"
 %s
 if ($LASTEXITCODE -eq $null) {$Env:BUILDKITE_HOOK_EXIT_STATUS = 0} else {$Env:BUILDKITE_HOOK_EXIT_STATUS = $LASTEXITCODE}
 $Env:BUILDKITE_HOOK_WORKING_DIR = $PWD | Select-Object -ExpandProperty Path
-buildkite-agent env | Set-Content "%s"
+Get-ChildItem Env: | Foreach-Object {"$($_.Name)=$($_.Value)"} | Set-Content "%s"
 exit $Env:BUILDKITE_HOOK_EXIT_STATUS`
 
 	assertScriptLike(t, scriptTemplate, hookFile.Name(), wrapper)
@@ -173,10 +180,15 @@ exit $BUILDKITE_HOOK_EXIT_STATUS`
 }
 
 func TestRunningHookDetectsChangedWorkingDirectory(t *testing.T) {
-	agent, cleanup, err := mockAgent()
-	require.NoError(t, err)
+	var agent *bintest.Mock
+	if runtime.GOOS != "windows" {
+		var cleanup func()
+		var err error
+		agent, cleanup, err = mockAgent()
+		require.NoError(t, err)
 
-	defer cleanup()
+		defer cleanup()
+	}
 
 	tempDir, err := ioutil.TempDir("", "hookwrapperdir")
 	if err != nil {
@@ -239,8 +251,10 @@ func TestRunningHookDetectsChangedWorkingDirectory(t *testing.T) {
 		t.Fatalf("Expected working dir of %q, got %q", expected, changesDir)
 	}
 
-	err = agent.CheckAndClose(t)
-	require.NoError(t, err)
+	if runtime.GOOS != "windows" {
+		err = agent.CheckAndClose(t)
+		require.NoError(t, err)
+	}
 }
 
 func newTestScriptWrapper(t *testing.T, script []string) *ScriptWrapper {