From e5d4886787185d2c5281a0222aa590f951ebee55 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Thu, 10 Jun 2021 17:28:23 +0200 Subject: [PATCH] Refactor local, composite actions and run steps (#712) Skips docker cp for local actions and use their correct path Defines GITHUB_ACTION_PATH also for nodejs actions Evaluate Env of composite action Evaluate Run of composite action correctly Evaluate Shell of run step Evaluate WorkingDirectory of run step Changed tests for behavior change Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- pkg/runner/step_context.go | 47 +++++++++---------- .../testdata/local-action-docker-url/push.yml | 1 + .../testdata/local-action-dockerfile/push.yml | 1 + pkg/runner/testdata/local-action-js/push.yml | 1 + 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index 9d6eb38..b4854b3 100644 --- a/pkg/runner/step_context.go +++ b/pkg/runner/step_context.go @@ -173,6 +173,7 @@ func (sc *StepContext) setupShellCommand() common.Executor { if step.WorkingDirectory == "" { step.WorkingDirectory = rc.Run.Workflow.Defaults.Run.WorkingDirectory } + step.WorkingDirectory = rc.ExprEval.Interpolate(step.WorkingDirectory) if step.WorkingDirectory != "" { _, err = script.WriteString(fmt.Sprintf("cd %s\n", step.WorkingDirectory)) if err != nil { @@ -181,6 +182,7 @@ func (sc *StepContext) setupShellCommand() common.Executor { } run := rc.ExprEval.Interpolate(step.Run) + step.Shell = rc.ExprEval.Interpolate(step.Shell) if _, err = script.WriteString(run); err != nil { return err @@ -381,15 +383,13 @@ func getOsSafeRelativePath(s, prefix string) string { func (sc *StepContext) getContainerActionPaths(step *model.Step, actionDir string, rc *RunContext) (string, string) { actionName := "" containerActionDir := "." - if !rc.Config.BindWorkdir && step.Type() != model.StepTypeUsesActionRemote { + if step.Type() != model.StepTypeUsesActionRemote { actionName = getOsSafeRelativePath(actionDir, rc.Config.Workdir) - containerActionDir = ActPath + "/actions/" + actionName + containerActionDir = rc.Config.ContainerWorkdir() + "/" + actionName + actionName = "./" + actionName } else if step.Type() == model.StepTypeUsesActionRemote { actionName = getOsSafeRelativePath(actionDir, rc.ActionCacheDir()) containerActionDir = ActPath + "/actions/" + actionName - } else if step.Type() == model.StepTypeUsesActionLocal { - actionName = getOsSafeRelativePath(actionDir, rc.Config.Workdir) - containerActionDir = ActPath + "/actions/" + actionName } if actionName == "" { @@ -422,11 +422,9 @@ func (sc *StepContext) runAction(actionDir string, actionPath string) common.Exe log.Debugf("type=%v actionDir=%s actionPath=%s Workdir=%s ActionCacheDir=%s actionName=%s containerActionDir=%s", step.Type(), actionDir, actionPath, rc.Config.Workdir, rc.ActionCacheDir(), actionName, containerActionDir) maybeCopyToActionDir := func() error { + sc.Env["GITHUB_ACTION_PATH"] = containerActionDir if step.Type() != model.StepTypeUsesActionRemote { - // If the workdir is bound to our repository then we don't need to copy the file - if rc.Config.BindWorkdir { - return nil - } + return nil } err := removeGitIgnore(actionDir) if err != nil { @@ -570,28 +568,29 @@ func (sc *StepContext) execAsComposite(ctx context.Context, step *model.Step, _ } } - if stepClone.Env == nil { - stepClone.Env = make(map[string]string) - } - actionPath := filepath.Join(containerActionDir, actionName) - env := stepClone.Environment() - env["GITHUB_ACTION_PATH"] = actionPath - stepClone.Run = strings.ReplaceAll(stepClone.Run, "${{ github.action_path }}", actionPath) - stepContext := StepContext{ RunContext: rcClone, - Step: &stepClone, + Step: step, Env: mergeMaps(sc.Env, env), } - // Interpolate the outer inputs into the composite step with items - exprEval := sc.NewExpressionEvaluator() - for k, v := range stepContext.Step.With { - if strings.Contains(v, "inputs") { - stepContext.Step.With[k] = exprEval.Interpolate(v) - } + // Required to set github.action_path + if rcClone.Config.Env == nil { + // Workaround to get test working + rcClone.Config.Env = make(map[string]string) } + rcClone.Config.Env["GITHUB_ACTION_PATH"] = sc.Env["GITHUB_ACTION_PATH"] + ev := stepContext.NewExpressionEvaluator() + // Required to interpolate inputs and github.action_path into the env map + stepContext.interpolateEnv(ev) + // Required to interpolate inputs, env and github.action_path into run steps + ev = stepContext.NewExpressionEvaluator() + stepClone.Run = ev.Interpolate(stepClone.Run) + stepClone.Shell = ev.Interpolate(stepClone.Shell) + stepClone.WorkingDirectory = ev.Interpolate(stepClone.WorkingDirectory) + + stepContext.Step = &stepClone executors = append(executors, stepContext.Executor()) } diff --git a/pkg/runner/testdata/local-action-docker-url/push.yml b/pkg/runner/testdata/local-action-docker-url/push.yml index b8a30de..5de1437 100644 --- a/pkg/runner/testdata/local-action-docker-url/push.yml +++ b/pkg/runner/testdata/local-action-docker-url/push.yml @@ -5,4 +5,5 @@ jobs: test: runs-on: ubuntu-latest steps: + - uses: actions/checkout@v2 - uses: ./actions/docker-url diff --git a/pkg/runner/testdata/local-action-dockerfile/push.yml b/pkg/runner/testdata/local-action-dockerfile/push.yml index 2454eed..06f9f4c 100644 --- a/pkg/runner/testdata/local-action-dockerfile/push.yml +++ b/pkg/runner/testdata/local-action-dockerfile/push.yml @@ -5,6 +5,7 @@ jobs: test: runs-on: ubuntu-latest steps: + - uses: actions/checkout@v2 - uses: ./actions/docker-local with: who-to-greet: 'Mona the Octocat' diff --git a/pkg/runner/testdata/local-action-js/push.yml b/pkg/runner/testdata/local-action-js/push.yml index 4095fe5..f23a2f0 100644 --- a/pkg/runner/testdata/local-action-js/push.yml +++ b/pkg/runner/testdata/local-action-js/push.yml @@ -5,6 +5,7 @@ jobs: test: runs-on: ubuntu-latest steps: + - uses: actions/checkout@v2 - uses: ./actions/node12 with: who-to-greet: 'Mona the Octocat'