mirror of
https://gitea.com/gitea/act_runner.git
synced 2026-05-07 15:53:24 +02:00
fix: re-fetch cached reusable workflow on every run (#930)
`cloneIfRequired` only ran the underlying clone executor when the target directory was missing, so a reusable workflow referenced by a moving ref (`uses: org/repo/.gitea/workflows/wf.yml@master`) was cached forever after the first invocation — edits to the source file never propagated. Always invoke `git.NewGitCloneExecutor`. It handles existing repositories via fetch + pull + hard-reset, so branch and tag refs are brought up to date on each run, matching GitHub Actions semantics. Drops the global `executorLock` too: `NewGitCloneExecutor` already takes a per-directory lock via `acquireCloneLock`, so the outer mutex only added unnecessary serialization across unrelated reusable-workflow clones — worse now that every invocation runs the full fetch. Includes a regression test that drives the wrapper against a local bare repo, pushes a new commit on `master` between two invocations, and asserts the cached workflow file reflects the new tip. Fixes: https://github.com/go-gitea/gitea/issues/37483 Fixes: https://gitea.com/gitea/runner/issues/726 Related: https://github.com/go-gitea/gitea/issues/30543 Would be subsumed by https://gitea.com/gitea/runner/pulls/814 ("WIP: Introduce new action cache") once that lands. --- This PR was written with the help of Claude Opus 4.7 Reviewed-on: https://gitea.com/gitea/runner/pulls/930 Reviewed-by: Zettat123 <39446+zettat123@noreply.gitea.com> Co-authored-by: silverwind <me@silverwind.io> Co-committed-by: silverwind <me@silverwind.io>
This commit is contained in:
@@ -7,15 +7,11 @@ package runner
|
|||||||
import (
|
import (
|
||||||
"archive/tar"
|
"archive/tar"
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
|
||||||
"fmt"
|
"fmt"
|
||||||
"io/fs"
|
|
||||||
"net/url"
|
"net/url"
|
||||||
"os"
|
|
||||||
"path"
|
"path"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
|
||||||
|
|
||||||
"gitea.com/gitea/runner/act/common"
|
"gitea.com/gitea/runner/act/common"
|
||||||
"gitea.com/gitea/runner/act/common/git"
|
"gitea.com/gitea/runner/act/common/git"
|
||||||
@@ -51,7 +47,7 @@ func newLocalReusableWorkflowExecutor(rc *RunContext) common.Executor {
|
|||||||
token := rc.Config.GetToken()
|
token := rc.Config.GetToken()
|
||||||
|
|
||||||
return common.NewPipelineExecutor(
|
return common.NewPipelineExecutor(
|
||||||
newMutexExecutor(cloneIfRequired(rc, *remoteReusableWorkflow, workflowDir, token)),
|
cloneRemoteReusableWorkflow(rc, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, workflowDir, token),
|
||||||
newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.FilePath()),
|
newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.FilePath()),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -85,7 +81,7 @@ func newRemoteReusableWorkflowExecutor(rc *RunContext) common.Executor {
|
|||||||
token := getGitCloneToken(rc.Config, remoteReusableWorkflow.CloneURL())
|
token := getGitCloneToken(rc.Config, remoteReusableWorkflow.CloneURL())
|
||||||
|
|
||||||
return common.NewPipelineExecutor(
|
return common.NewPipelineExecutor(
|
||||||
newMutexExecutor(cloneIfRequired(rc, *remoteReusableWorkflow, workflowDir, token)),
|
cloneRemoteReusableWorkflow(rc, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, workflowDir, token),
|
||||||
newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.FilePath()),
|
newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.FilePath()),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -125,41 +121,25 @@ func newActionCacheReusableWorkflowExecutor(rc *RunContext, filename string, rem
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
var executorLock sync.Mutex
|
// cloneRemoteReusableWorkflow always invokes the clone executor — moving refs
|
||||||
|
// (branches, tags) must be re-resolved each run, matching GitHub Actions.
|
||||||
func newMutexExecutor(executor common.Executor) common.Executor {
|
//
|
||||||
return func(ctx context.Context) error {
|
// Callers must not change remoteReusableWorkflow.URL, because:
|
||||||
executorLock.Lock()
|
|
||||||
defer executorLock.Unlock()
|
|
||||||
|
|
||||||
return executor(ctx)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func cloneIfRequired(rc *RunContext, remoteReusableWorkflow remoteReusableWorkflow, targetDirectory, token string) common.Executor {
|
|
||||||
return common.NewConditionalExecutor(
|
|
||||||
func(ctx context.Context) bool {
|
|
||||||
_, err := os.Stat(targetDirectory)
|
|
||||||
notExists := errors.Is(err, fs.ErrNotExist)
|
|
||||||
return notExists
|
|
||||||
},
|
|
||||||
func(ctx context.Context) error {
|
|
||||||
// interpolate the cloneURL
|
|
||||||
cloneURL := rc.NewExpressionEvaluator(ctx).Interpolate(ctx, remoteReusableWorkflow.CloneURL())
|
|
||||||
// Do not change the remoteReusableWorkflow.URL, because:
|
|
||||||
// 1. Gitea doesn't support specifying GithubContext.ServerURL by the GITHUB_SERVER_URL env
|
// 1. Gitea doesn't support specifying GithubContext.ServerURL by the GITHUB_SERVER_URL env
|
||||||
// 2. Gitea has already full URL with rc.Config.GitHubInstance when calling newRemoteReusableWorkflowWithPlat
|
// 2. Gitea has already full URL with rc.Config.GitHubInstance when calling newRemoteReusableWorkflowWithPlat
|
||||||
|
//
|
||||||
// remoteReusableWorkflow.URL = rc.getGithubContext(ctx).ServerURL
|
// remoteReusableWorkflow.URL = rc.getGithubContext(ctx).ServerURL
|
||||||
|
func cloneRemoteReusableWorkflow(rc *RunContext, cloneURL, ref, targetDirectory, token string) common.Executor {
|
||||||
|
return func(ctx context.Context) error {
|
||||||
|
cloneURL = rc.NewExpressionEvaluator(ctx).Interpolate(ctx, cloneURL)
|
||||||
return git.NewGitCloneExecutor(git.NewGitCloneExecutorInput{
|
return git.NewGitCloneExecutor(git.NewGitCloneExecutorInput{
|
||||||
URL: cloneURL,
|
URL: cloneURL,
|
||||||
Ref: remoteReusableWorkflow.Ref,
|
Ref: ref,
|
||||||
Dir: targetDirectory,
|
Dir: targetDirectory,
|
||||||
Token: token,
|
Token: token,
|
||||||
OfflineMode: rc.Config.ActionOfflineMode,
|
OfflineMode: rc.Config.ActionOfflineMode,
|
||||||
})(ctx)
|
})(ctx)
|
||||||
},
|
}
|
||||||
nil,
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func newReusableWorkflowExecutor(rc *RunContext, directory, workflow string) common.Executor {
|
func newReusableWorkflowExecutor(rc *RunContext, directory, workflow string) common.Executor {
|
||||||
|
|||||||
82
act/runner/reusable_workflow_test.go
Normal file
82
act/runner/reusable_workflow_test.go
Normal file
@@ -0,0 +1,82 @@
|
|||||||
|
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||||
|
// SPDX-License-Identifier: MIT
|
||||||
|
|
||||||
|
package runner
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"os"
|
||||||
|
"os/exec"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"gitea.com/gitea/runner/act/model"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Regression test for go-gitea/gitea#37483: a remote reusable workflow at a moving
|
||||||
|
// ref (branch/tag) must reflect the new tip on every invocation, not stay pinned
|
||||||
|
// to the cache populated on the first run.
|
||||||
|
func TestReusableWorkflowCachedBranchRefRefreshes(t *testing.T) {
|
||||||
|
if _, err := exec.LookPath("git"); err != nil {
|
||||||
|
t.Skip("git not available in PATH")
|
||||||
|
}
|
||||||
|
|
||||||
|
remoteDir := t.TempDir()
|
||||||
|
gitMust(t, "", "init", "--bare", "--initial-branch=master", remoteDir)
|
||||||
|
|
||||||
|
workDir := t.TempDir()
|
||||||
|
gitMust(t, "", "clone", remoteDir, workDir)
|
||||||
|
gitMust(t, workDir, "config", "user.email", "test@test")
|
||||||
|
gitMust(t, workDir, "config", "user.name", "test")
|
||||||
|
gitMust(t, workDir, "checkout", "-b", "master")
|
||||||
|
|
||||||
|
const workflowPath = ".gitea/workflows/reusable.yml"
|
||||||
|
tmpl := func(tag string) string {
|
||||||
|
return "name: reusable\non:\n workflow_call:\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo " + tag + "\n"
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, os.MkdirAll(filepath.Join(workDir, ".gitea/workflows"), 0o755))
|
||||||
|
require.NoError(t, os.WriteFile(filepath.Join(workDir, workflowPath), []byte(tmpl("v1")), 0o644))
|
||||||
|
gitMust(t, workDir, "add", workflowPath)
|
||||||
|
gitMust(t, workDir, "commit", "-m", "v1")
|
||||||
|
gitMust(t, workDir, "push", "-u", "origin", "master")
|
||||||
|
|
||||||
|
rc := &RunContext{
|
||||||
|
Config: &Config{},
|
||||||
|
Run: &model.Run{
|
||||||
|
JobID: "j1",
|
||||||
|
Workflow: &model.Workflow{
|
||||||
|
Name: "wf",
|
||||||
|
Jobs: map[string]*model.Job{"j1": {}},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
cacheDir := t.TempDir()
|
||||||
|
|
||||||
|
require.NoError(t, cloneRemoteReusableWorkflow(rc, remoteDir, "master", cacheDir, "")(context.Background()))
|
||||||
|
got, err := os.ReadFile(filepath.Join(cacheDir, workflowPath))
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, tmpl("v1"), string(got))
|
||||||
|
|
||||||
|
// Branch tip moves; cache key (cacheDir) does not.
|
||||||
|
require.NoError(t, os.WriteFile(filepath.Join(workDir, workflowPath), []byte(tmpl("v2")), 0o644))
|
||||||
|
gitMust(t, workDir, "commit", "-am", "v2")
|
||||||
|
gitMust(t, workDir, "push", "origin", "master")
|
||||||
|
|
||||||
|
require.NoError(t, cloneRemoteReusableWorkflow(rc, remoteDir, "master", cacheDir, "")(context.Background()))
|
||||||
|
got, err = os.ReadFile(filepath.Join(cacheDir, workflowPath))
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, tmpl("v2"), string(got), "cached workflow file must reflect the updated branch tip")
|
||||||
|
}
|
||||||
|
|
||||||
|
func gitMust(t *testing.T, dir string, args ...string) {
|
||||||
|
t.Helper()
|
||||||
|
cmd := exec.Command("git", args...)
|
||||||
|
if dir != "" {
|
||||||
|
cmd.Dir = dir
|
||||||
|
}
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
require.NoError(t, err, "git %v: %s", args, string(out))
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user