Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: environment handling windows (host mode) #1732

Merged
merged 11 commits into from
Apr 18, 2023
2 changes: 2 additions & 0 deletions pkg/container/executions_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ type ExecutionsEnvironment interface {
DefaultPathVariable() string
JoinPathVariable(...string) string
GetRunnerContext(ctx context.Context) map[string]interface{}
// On windows PATH and Path are the same key
IsEnvironmentCaseInsensitive() bool
}
4 changes: 4 additions & 0 deletions pkg/container/host_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,3 +419,7 @@ func (e *HostEnvironment) ReplaceLogWriter(stdout io.Writer, stderr io.Writer) (
e.StdOut = stdout
return org, org
}

func (*HostEnvironment) IsEnvironmentCaseInsensitive() bool {
return runtime.GOOS == "windows"
}
4 changes: 4 additions & 0 deletions pkg/container/linux_container_environment_extensions.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,7 @@ func (*LinuxContainerEnvironmentExtensions) GetRunnerContext(ctx context.Context
"tool_cache": "/opt/hostedtoolcache",
}
}

func (*LinuxContainerEnvironmentExtensions) IsEnvironmentCaseInsensitive() bool {
return false
}
4 changes: 2 additions & 2 deletions pkg/runner/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,13 +319,13 @@ func evalDockerArgs(ctx context.Context, step step, action *model.Action, cmd *[
inputs[k] = eval.Interpolate(ctx, v)
}
}
mergeIntoMap(step.getEnv(), inputs)
mergeIntoMap(step, step.getEnv(), inputs)

stepEE := rc.NewStepExpressionEvaluator(ctx, step)
for i, v := range *cmd {
(*cmd)[i] = stepEE.Interpolate(ctx, v)
}
mergeIntoMap(step.getEnv(), action.Runs.Env)
mergeIntoMap(step, step.getEnv(), action.Runs.Env)

ee := rc.NewStepExpressionEvaluator(ctx, step)
for k, v := range *step.getEnv() {
Expand Down
14 changes: 8 additions & 6 deletions pkg/runner/action_composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,15 @@ func execAsComposite(step actionStep) common.Executor {
rc.Masks = append(rc.Masks, compositeRC.Masks...)
rc.ExtraPath = compositeRC.ExtraPath
// compositeRC.Env is dirty, contains INPUT_ and merged step env, only rely on compositeRC.GlobalEnv
for k, v := range compositeRC.GlobalEnv {
rc.Env[k] = v
if rc.GlobalEnv == nil {
rc.GlobalEnv = map[string]string{}
}
rc.GlobalEnv[k] = v
mergeIntoMap := mergeIntoMapCaseSensitive
if rc.JobContainer.IsEnvironmentCaseInsensitive() {
mergeIntoMap = mergeIntoMapCaseInsensitive
}
if rc.GlobalEnv == nil {
rc.GlobalEnv = map[string]string{}
}
mergeIntoMap(rc.GlobalEnv, compositeRC.GlobalEnv)
mergeIntoMap(rc.Env, compositeRC.GlobalEnv)

return err
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/runner/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,18 @@ func (rc *RunContext) setEnv(ctx context.Context, kvPairs map[string]string, arg
if rc.Env == nil {
rc.Env = make(map[string]string)
}
rc.Env[name] = arg
// for composite action GITHUB_ENV and set-env passing
if rc.GlobalEnv == nil {
rc.GlobalEnv = map[string]string{}
}
rc.GlobalEnv[name] = arg
newenv := map[string]string{
name: arg,
}
mergeIntoMap := mergeIntoMapCaseSensitive
if rc.JobContainer != nil && rc.JobContainer.IsEnvironmentCaseInsensitive() {
mergeIntoMap = mergeIntoMapCaseInsensitive
}
mergeIntoMap(rc.Env, newenv)
mergeIntoMap(rc.GlobalEnv, newenv)
}
func (rc *RunContext) setOutput(ctx context.Context, kvPairs map[string]string, arg string) {
logger := common.Logger(ctx)
Expand Down
9 changes: 9 additions & 0 deletions pkg/runner/run_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,15 @@ func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user
func (rc *RunContext) ApplyExtraPath(ctx context.Context, env *map[string]string) {
if rc.ExtraPath != nil && len(rc.ExtraPath) > 0 {
path := rc.JobContainer.GetPathVariableName()
if rc.JobContainer.IsEnvironmentCaseInsensitive() {
// On windows system Path and PATH could also be in the map
for k := range *env {
if strings.EqualFold(path, k) {
path = k
break
}
}
}
if (*env)[path] == "" {
cenv := map[string]string{}
var cpath string
Expand Down
38 changes: 33 additions & 5 deletions pkg/runner/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func setupEnv(ctx context.Context, step step) error {

mergeEnv(ctx, step)
// merge step env last, since it should not be overwritten
mergeIntoMap(step.getEnv(), step.getStepModel().GetEnv())
mergeIntoMap(step, step.getEnv(), step.getStepModel().GetEnv())

exprEval := rc.NewExpressionEvaluator(ctx)
for k, v := range *step.getEnv() {
Expand Down Expand Up @@ -216,9 +216,9 @@ func mergeEnv(ctx context.Context, step step) {

c := job.Container()
if c != nil {
mergeIntoMap(env, rc.GetEnv(), c.Env)
mergeIntoMap(step, env, rc.GetEnv(), c.Env)
} else {
mergeIntoMap(env, rc.GetEnv())
mergeIntoMap(step, env, rc.GetEnv())
}

rc.withGithubEnv(ctx, step.getGithubContext(ctx), *env)
Expand Down Expand Up @@ -258,10 +258,38 @@ func isContinueOnError(ctx context.Context, expr string, step step, stage stepSt
return continueOnError, nil
}

func mergeIntoMap(target *map[string]string, maps ...map[string]string) {
func mergeIntoMap(step step, target *map[string]string, maps ...map[string]string) {
if rc := step.getRunContext(); rc != nil && rc.JobContainer != nil && rc.JobContainer.IsEnvironmentCaseInsensitive() {
mergeIntoMapCaseInsensitive(*target, maps...)
} else {
mergeIntoMapCaseSensitive(*target, maps...)
}
}

func mergeIntoMapCaseSensitive(target map[string]string, maps ...map[string]string) {
for _, m := range maps {
for k, v := range m {
target[k] = v
}
}
}

func mergeIntoMapCaseInsensitive(target map[string]string, maps ...map[string]string) {
foldKeys := make(map[string]string, len(target))
for k := range target {
foldKeys[strings.ToLower(k)] = k
}
toKey := func(s string) string {
foldKey := strings.ToLower(s)
if k, ok := foldKeys[foldKey]; ok {
return k
}
foldKeys[strings.ToLower(foldKey)] = s
return s
}
for _, m := range maps {
for k, v := range m {
(*target)[k] = v
target[toKey(k)] = v
}
}
}
4 changes: 3 additions & 1 deletion pkg/runner/step_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ func TestMergeIntoMap(t *testing.T) {

for _, tt := range table {
t.Run(tt.name, func(t *testing.T) {
mergeIntoMap(&tt.target, tt.maps...)
mergeIntoMapCaseSensitive(tt.target, tt.maps...)
assert.Equal(t, tt.expected, tt.target)
mergeIntoMapCaseInsensitive(tt.target, tt.maps...)
assert.Equal(t, tt.expected, tt.target)
})
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/runner/testdata/windows-add-env/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
runs:
using: composite
steps:
- run: |
echo $env:GITHUB_ENV
echo "kEy=n/a" > $env:GITHUB_ENV
shell: pwsh
17 changes: 17 additions & 0 deletions pkg/runner/testdata/windows-add-env/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,20 @@ jobs:
echo "Unexpected value for `$env:key2: $env:key2"
exit 1
}
- run: |
echo $env:GITHUB_ENV
echo "KEY=test" > $env:GITHUB_ENV
echo "Key=expected" > $env:GITHUB_ENV
- name: Assert GITHUB_ENV is merged case insensitive
run: exit 1
if: env.KEY != 'expected' || env.Key != 'expected' || env.key != 'expected'
- name: Assert step env is merged case insensitive
run: exit 1
if: env.KEY != 'n/a' || env.Key != 'n/a' || env.key != 'n/a'
env:
KeY: 'n/a'
- uses: actions/checkout@v3
- uses: ./windows-add-env
- name: Assert composite env is merged case insensitive
run: exit 1
if: env.KEY != 'n/a' || env.Key != 'n/a' || env.key != 'n/a'
9 changes: 9 additions & 0 deletions pkg/runner/testdata/windows-prepend-path/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ jobs:
mkdir build
echo '@echo off' > build/test.cmd
echo 'echo Hi' >> build/test.cmd
mkdir build2
echo '@echo off' > build2/test2.cmd
echo 'echo test2' >> build2/test2.cmd
- run: |
echo '${{ tojson(runner) }}'
ls
Expand All @@ -23,3 +26,9 @@ jobs:
- run: |
echo $env:PATH
test
- run: |
echo "PATH=$env:PATH;${{ github.workspace }}\build2" > $env:GITHUB_ENV
- run: |
echo $env:PATH
test
test2