Skip to content

Commit d281230

Browse files
ChristopherHXmergify[bot]
ChristopherHX
and
mergify[bot]
authored
refactor: fix add-path / GITHUB_PATH commands (#1472)
* fix: add-path / GITHUB_PATH commands * Disable old code * fix: missing mock * Update tests * fix tests * UpdateExtraPath skip on dryrun * patch test Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 7d99512 commit d281230

12 files changed

+95
-46
lines changed

‎pkg/runner/action.go

+6
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
154154
containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Main)}
155155
logger.Debugf("executing remote job container: %s", containerArgs)
156156

157+
rc.ApplyExtraPath(step.getEnv())
158+
157159
return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
158160
case model.ActionRunsUsingDocker:
159161
location := actionLocation
@@ -486,6 +488,8 @@ func runPreStep(step actionStep) common.Executor {
486488
containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Pre)}
487489
logger.Debugf("executing remote job container: %s", containerArgs)
488490

491+
rc.ApplyExtraPath(step.getEnv())
492+
489493
return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
490494

491495
case model.ActionRunsUsingComposite:
@@ -573,6 +577,8 @@ func runPostStep(step actionStep) common.Executor {
573577
containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Post)}
574578
logger.Debugf("executing remote job container: %s", containerArgs)
575579

580+
rc.ApplyExtraPath(step.getEnv())
581+
576582
return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
577583

578584
case model.ActionRunsUsingComposite:

‎pkg/runner/command.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,13 @@ func (rc *RunContext) setOutput(ctx context.Context, kvPairs map[string]string,
101101
}
102102
func (rc *RunContext) addPath(ctx context.Context, arg string) {
103103
common.Logger(ctx).Infof(" \U00002699 ::add-path:: %s", arg)
104-
rc.ExtraPath = append(rc.ExtraPath, arg)
104+
extraPath := []string{arg}
105+
for _, v := range rc.ExtraPath {
106+
if v != arg {
107+
extraPath = append(extraPath, v)
108+
}
109+
}
110+
rc.ExtraPath = extraPath
105111
}
106112

107113
func parseKeyValuePairs(kvPairs string, separator string) map[string]string {

‎pkg/runner/command_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func TestAddpath(t *testing.T) {
6464
a.Equal("/zoo", rc.ExtraPath[0])
6565

6666
handler("::add-path::/boo\n")
67-
a.Equal("/boo", rc.ExtraPath[1])
67+
a.Equal("/boo", rc.ExtraPath[0])
6868
}
6969

7070
func TestStopCommands(t *testing.T) {
@@ -102,7 +102,7 @@ func TestAddpathADO(t *testing.T) {
102102
a.Equal("/zoo", rc.ExtraPath[0])
103103

104104
handler("##[add-path]/boo\n")
105-
a.Equal("/boo", rc.ExtraPath[1])
105+
a.Equal("/boo", rc.ExtraPath[0])
106106
}
107107

108108
func TestAddmask(t *testing.T) {

‎pkg/runner/container_mock_test.go

+11
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package runner
22

33
import (
44
"context"
5+
"io"
56

67
"github.com/nektos/act/pkg/common"
78
"github.com/nektos/act/pkg/container"
@@ -63,7 +64,17 @@ func (cm *containerMock) CopyDir(destPath string, srcPath string, useGitIgnore b
6364
args := cm.Called(destPath, srcPath, useGitIgnore)
6465
return args.Get(0).(func(context.Context) error)
6566
}
67+
6668
func (cm *containerMock) Exec(command []string, env map[string]string, user, workdir string) common.Executor {
6769
args := cm.Called(command, env, user, workdir)
6870
return args.Get(0).(func(context.Context) error)
6971
}
72+
73+
func (cm *containerMock) GetContainerArchive(ctx context.Context, srcPath string) (io.ReadCloser, error) {
74+
args := cm.Called(ctx, srcPath)
75+
err, hasErr := args.Get(1).(error)
76+
if !hasErr {
77+
err = nil
78+
}
79+
return args.Get(0).(io.ReadCloser), err
80+
}

‎pkg/runner/run_context.go

+38-9
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package runner
22

33
import (
4+
"archive/tar"
5+
"bufio"
46
"context"
57
"crypto/rand"
68
"encoding/hex"
79
"encoding/json"
810
"errors"
911
"fmt"
12+
"io"
1013
"os"
1114
"path/filepath"
1215
"regexp"
@@ -188,10 +191,6 @@ func (rc *RunContext) startHostEnvironment() common.Executor {
188191
Name: "workflow/envs.txt",
189192
Mode: 0666,
190193
Body: "",
191-
}, &container.FileEntry{
192-
Name: "workflow/paths.txt",
193-
Mode: 0666,
194-
Body: "",
195194
}),
196195
)(ctx)
197196
}
@@ -277,10 +276,6 @@ func (rc *RunContext) startJobContainer() common.Executor {
277276
Name: "workflow/envs.txt",
278277
Mode: 0666,
279278
Body: "",
280-
}, &container.FileEntry{
281-
Name: "workflow/paths.txt",
282-
Mode: 0666,
283-
Body: "",
284279
}),
285280
)(ctx)
286281
}
@@ -292,6 +287,41 @@ func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user
292287
}
293288
}
294289

290+
func (rc *RunContext) ApplyExtraPath(env *map[string]string) {
291+
if rc.ExtraPath != nil && len(rc.ExtraPath) > 0 {
292+
path := rc.JobContainer.GetPathVariableName()
293+
if (*env)[path] == "" {
294+
(*env)[path] = rc.JobContainer.DefaultPathVariable()
295+
}
296+
(*env)[path] = rc.JobContainer.JoinPathVariable(append(rc.ExtraPath, (*env)[path])...)
297+
}
298+
}
299+
300+
func (rc *RunContext) UpdateExtraPath(ctx context.Context, githubEnvPath string) error {
301+
if common.Dryrun(ctx) {
302+
return nil
303+
}
304+
pathTar, err := rc.JobContainer.GetContainerArchive(ctx, githubEnvPath)
305+
if err != nil {
306+
return err
307+
}
308+
defer pathTar.Close()
309+
310+
reader := tar.NewReader(pathTar)
311+
_, err = reader.Next()
312+
if err != nil && err != io.EOF {
313+
return err
314+
}
315+
s := bufio.NewScanner(reader)
316+
for s.Scan() {
317+
line := s.Text()
318+
if len(line) > 0 {
319+
rc.addPath(ctx, line)
320+
}
321+
}
322+
return nil
323+
}
324+
295325
// stopJobContainer removes the job container (if it exists) and its volume (if it exists) if !rc.Config.ReuseContainers
296326
func (rc *RunContext) stopJobContainer() common.Executor {
297327
return func(ctx context.Context) error {
@@ -639,7 +669,6 @@ func nestedMapLookup(m map[string]interface{}, ks ...string) (rval interface{})
639669
func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubContext, env map[string]string) map[string]string {
640670
env["CI"] = "true"
641671
env["GITHUB_ENV"] = rc.JobContainer.GetActPath() + "/workflow/envs.txt"
642-
env["GITHUB_PATH"] = rc.JobContainer.GetActPath() + "/workflow/paths.txt"
643672
env["GITHUB_WORKFLOW"] = github.Workflow
644673
env["GITHUB_RUN_ID"] = github.RunID
645674
env["GITHUB_RUN_NUMBER"] = github.RunNumber

‎pkg/runner/step.go

+9-12
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,19 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
100100
actPath := rc.JobContainer.GetActPath()
101101
outputFileCommand := path.Join("workflow", "outputcmd.txt")
102102
stateFileCommand := path.Join("workflow", "statecmd.txt")
103+
pathFileCommand := path.Join("workflow", "pathcmd.txt")
103104
(*step.getEnv())["GITHUB_OUTPUT"] = path.Join(actPath, outputFileCommand)
104105
(*step.getEnv())["GITHUB_STATE"] = path.Join(actPath, stateFileCommand)
106+
(*step.getEnv())["GITHUB_PATH"] = path.Join(actPath, pathFileCommand)
105107
_ = rc.JobContainer.Copy(actPath, &container.FileEntry{
106108
Name: outputFileCommand,
107109
Mode: 0666,
108110
}, &container.FileEntry{
109111
Name: stateFileCommand,
110112
Mode: 0666,
113+
}, &container.FileEntry{
114+
Name: pathFileCommand,
115+
Mode: 0666,
111116
})(ctx)
112117

113118
err = executor(ctx)
@@ -151,6 +156,10 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
151156
for k, v := range output {
152157
rc.setOutput(ctx, map[string]string{"name": k}, v)
153158
}
159+
err = rc.UpdateExtraPath(ctx, path.Join(actPath, pathFileCommand))
160+
if err != nil {
161+
return err
162+
}
154163
if orgerr != nil {
155164
return orgerr
156165
}
@@ -170,10 +179,6 @@ func setupEnv(ctx context.Context, step step) error {
170179
if err != nil {
171180
return err
172181
}
173-
err = rc.JobContainer.UpdateFromPath(step.getEnv())(ctx)
174-
if err != nil {
175-
return err
176-
}
177182
// merge step env last, since it should not be overwritten
178183
mergeIntoMap(step.getEnv(), step.getStepModel().GetEnv())
179184

@@ -209,14 +214,6 @@ func mergeEnv(ctx context.Context, step step) {
209214
mergeIntoMap(env, rc.GetEnv())
210215
}
211216

212-
path := rc.JobContainer.GetPathVariableName()
213-
if (*env)[path] == "" {
214-
(*env)[path] = rc.JobContainer.DefaultPathVariable()
215-
}
216-
if rc.ExtraPath != nil && len(rc.ExtraPath) > 0 {
217-
(*env)[path] = rc.JobContainer.JoinPathVariable(append(rc.ExtraPath, (*env)[path])...)
218-
}
219-
220217
rc.withGithubEnv(ctx, step.getGithubContext(ctx), *env)
221218
}
222219

‎pkg/runner/step_action_local_test.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package runner
22

33
import (
4+
"bytes"
45
"context"
6+
"io"
57
"path/filepath"
68
"strings"
79
"testing"
@@ -75,10 +77,6 @@ func TestStepActionLocalTest(t *testing.T) {
7577
return nil
7678
})
7779

78-
cm.On("UpdateFromPath", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
79-
return nil
80-
})
81-
8280
cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
8381
return nil
8482
})
@@ -91,6 +89,8 @@ func TestStepActionLocalTest(t *testing.T) {
9189
return nil
9290
})
9391

92+
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
93+
9494
salm.On("runAction", sal, filepath.Clean("/tmp/path/to/action"), (*remoteAction)(nil)).Return(func(ctx context.Context) error {
9595
return nil
9696
})
@@ -280,7 +280,6 @@ func TestStepActionLocalPost(t *testing.T) {
280280
if tt.mocks.env {
281281
cm.On("UpdateFromImageEnv", &sal.env).Return(func(ctx context.Context) error { return nil })
282282
cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sal.env).Return(func(ctx context.Context) error { return nil })
283-
cm.On("UpdateFromPath", &sal.env).Return(func(ctx context.Context) error { return nil })
284283
}
285284
if tt.mocks.exec {
286285
suffixMatcher := func(suffix string) interface{} {
@@ -301,6 +300,8 @@ func TestStepActionLocalPost(t *testing.T) {
301300
cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
302301
return nil
303302
})
303+
304+
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
304305
}
305306

306307
err := sal.post()(ctx)

‎pkg/runner/step_action_remote_test.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package runner
22

33
import (
4+
"bytes"
45
"context"
56
"errors"
7+
"io"
68
"strings"
79
"testing"
810

@@ -166,7 +168,6 @@ func TestStepActionRemote(t *testing.T) {
166168
if tt.mocks.env {
167169
cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil })
168170
cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil })
169-
cm.On("UpdateFromPath", &sar.env).Return(func(ctx context.Context) error { return nil })
170171
}
171172
if tt.mocks.read {
172173
sarm.On("readAction", sar.Step, suffixMatcher("act/remote-action@v1"), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil)
@@ -185,6 +186,8 @@ func TestStepActionRemote(t *testing.T) {
185186
cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
186187
return nil
187188
})
189+
190+
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
188191
}
189192

190193
err := sar.pre()(ctx)
@@ -592,7 +595,6 @@ func TestStepActionRemotePost(t *testing.T) {
592595
if tt.mocks.env {
593596
cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil })
594597
cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil })
595-
cm.On("UpdateFromPath", &sar.env).Return(func(ctx context.Context) error { return nil })
596598
}
597599
if tt.mocks.exec {
598600
cm.On("Exec", []string{"node", "/var/run/act/actions/remote-action@v1/post.js"}, sar.env, "", "").Return(func(ctx context.Context) error { return tt.err })
@@ -608,6 +610,8 @@ func TestStepActionRemotePost(t *testing.T) {
608610
cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
609611
return nil
610612
})
613+
614+
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
611615
}
612616

613617
err := sar.post()(ctx)

‎pkg/runner/step_docker_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package runner
22

33
import (
4+
"bytes"
45
"context"
6+
"io"
57
"testing"
68

79
"github.com/nektos/act/pkg/container"
@@ -63,10 +65,6 @@ func TestStepDockerMain(t *testing.T) {
6365
return nil
6466
})
6567

66-
cm.On("UpdateFromPath", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
67-
return nil
68-
})
69-
7068
cm.On("Pull", false).Return(func(ctx context.Context) error {
7169
return nil
7270
})
@@ -99,6 +97,8 @@ func TestStepDockerMain(t *testing.T) {
9997
return nil
10098
})
10199

100+
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
101+
102102
err := sd.main()(ctx)
103103
assert.Nil(t, err)
104104

‎pkg/runner/step_run.go

+1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ func (sr *stepRun) main() common.Executor {
3030
return runStepExecutor(sr, stepStageMain, common.NewPipelineExecutor(
3131
sr.setupShellCommandExecutor(),
3232
func(ctx context.Context) error {
33+
sr.getRunContext().ApplyExtraPath(&sr.env)
3334
return sr.getRunContext().JobContainer.Exec(sr.cmd, sr.env, "", sr.Step.WorkingDirectory)(ctx)
3435
},
3536
))

‎pkg/runner/step_run_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package runner
22

33
import (
4+
"bytes"
45
"context"
6+
"io"
57
"testing"
68

79
"github.com/nektos/act/pkg/container"
@@ -61,10 +63,6 @@ func TestStepRun(t *testing.T) {
6163
return nil
6264
})
6365

64-
cm.On("UpdateFromPath", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
65-
return nil
66-
})
67-
6866
cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
6967
return nil
7068
})
@@ -79,6 +77,8 @@ func TestStepRun(t *testing.T) {
7977

8078
ctx := context.Background()
8179

80+
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
81+
8282
err := sr.main()(ctx)
8383
assert.Nil(t, err)
8484

0 commit comments

Comments
 (0)