enhance: download each action repository once per job (#1178)

A job downloads each action repository once, keyed on the clone URL and ref, so repeated `uses:` and different paths of one repository share a checkout. The download is reported once as `{org}/{repo}@{ref}`, the way actions/runner reports it.

The action itself is still read per step, because a repository without an action file gets a synthetic action built from that step's `with.args`.

Fixes https://gitea.com/gitea/runner/issues/1159

---------

Co-authored-by: silverwind <me@silverwind.io>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1178
Reviewed-by: bircni <bircni@icloud.com>
Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

.gitea/workflows/test.yml

@@ -24,7 +24,7 @@ jobs:
           check-latest: true
       - name: prepare anonymous docker config
         run: mkdir -p "$DOCKER_CONFIG" && echo '{}' > "$DOCKER_CONFIG/config.json"
-      # Pre-pull act/runner's two largest base images so a slow pull can't dominate `make test`;
+      # Pre-pull internal/act/runner's two largest base images so a slow pull can't dominate `make test`;
       # the rest (alpine/ubuntu) pull on demand, absorbed by the make-test -timeout. The host
       # daemon retains them between runs, so this is usually a fast manifest re-check.
       - name: pre-pull test images
@@ -48,4 +48,4 @@ jobs:
       - name: coverage report
         run: |
           make coverage-report
-          cat .tmp/coverage.md >> "$GITHUB_STEP_SUMMARY"
+          cat .tmp/coverage.md >> "$GITHUB_STEP_SUMMARY"

.gitignore

@@ -1,6 +1,6 @@
 /gitea-runner
 .env
-!/act/runner/testdata/secrets/.env
+!/internal/act/runner/testdata/secrets/.env
 .runner
 .runner.lock
 coverage.txt
@@ -14,4 +14,4 @@ coverage.txt
 __debug_bin
 # gorelease binary folder
 /dist
-.DS_Store
+.DS_Store

internal/act/artifactcache/handler.go

@@ -26,7 +26,7 @@ import (
 	"sync/atomic"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/julienschmidt/httprouter"
 	"github.com/sirupsen/logrus"

internal/act/artifacts/server.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/julienschmidt/httprouter"
 )

internal/act/common/git/git.go

@@ -15,7 +15,7 @@ import (
 	"strings"
 	"sync"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 	"gitea.com/gitea/runner/internal/pkg/lock"
 
 	"github.com/go-git/go-git/v5"
@@ -458,10 +458,14 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor {
 				logger.Debugf("Unable to pull %s: %v", refName, err)
 			}
 		case isOfflineMode && reused:
-			reusedMsg = " (reused in offline mode)"
+			reusedMsg = " (offline mode)"
 		}
 
-		logger.Debugf("Cloned %s to %s%s", input.URL, input.Dir, reusedMsg)
+		if reused {
+			logger.Debugf("Reused %s at %s%s", input.URL, input.Dir, reusedMsg)
+		} else {
+			logger.Debugf("Cloned %s to %s", input.URL, input.Dir)
+		}
 
 		if hash.String() != input.Ref && refType == "branch" {
 			logger.Debugf("Provided ref is not a sha. Updating branch ref after pull")

internal/act/common/git/git_test.go

@@ -16,7 +16,7 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	log "github.com/sirupsen/logrus"
 	logrustest "github.com/sirupsen/logrus/hooks/test"
@@ -373,22 +373,28 @@ func TestGitCloneExecutorOfflineMode(t *testing.T) {
 
 	// Prime the cache with an online clone of main.
 	cacheDir := t.TempDir()
+	logger, hook := logrustest.NewNullLogger()
+	logger.SetLevel(log.DebugLevel)
+	ctx := common.WithLogger(context.Background(), logger.WithField("job", "j1"))
 	require.NoError(t, NewGitCloneExecutor(NewGitCloneExecutorInput{
 		URL: remoteDir,
 		Ref: "main",
 		Dir: cacheDir,
-	})(context.Background()))
+	})(ctx))
+	assert.Contains(t, logMessages(hook), "Cloned "+remoteDir+" to "+cacheDir)
 
 	t.Run("cached branch resolves without fetching", func(t *testing.T) {
 		// Offline reuse of a cached branch must succeed even though ResolveRevision(input.Ref)
 		// finds no local refs/heads/<ref>.
+		hook.Reset()
 		err := NewGitCloneExecutor(NewGitCloneExecutorInput{
 			URL:         remoteDir,
 			Ref:         "main",
 			Dir:         cacheDir,
 			OfflineMode: true,
-		})(context.Background())
+		})(ctx)
 		require.NoError(t, err)
+		assert.Contains(t, logMessages(hook), "Reused "+remoteDir+" at "+cacheDir+" (offline mode)")
 
 		out, err := exec.Command("git", "-C", cacheDir, "log", "--oneline", "-1", "--format=%s").Output()
 		require.NoError(t, err)
@@ -445,6 +451,14 @@ func TestGitCloneExecutorQuietDemotesCloneLine(t *testing.T) {
 	}
 }
 
+func logMessages(hook *logrustest.Hook) []string {
+	messages := []string{}
+	for _, entry := range hook.AllEntries() {
+		messages = append(messages, entry.Message)
+	}
+	return messages
+}
+
 func TestGitCloneExecutorShallow(t *testing.T) {
 	// Build a local "remote" with several commits on main plus a tag, so a full clone would pull noticeably more history than a shallow one.
 	remoteDir := t.TempDir()

internal/act/container/container_types.go

@@ -10,7 +10,7 @@ import (
 	"fmt"
 	"io"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/docker/go-connections/nat"
 	"github.com/moby/moby/api/types/container"

internal/act/container/docker_auth.go

@@ -9,7 +9,7 @@ package container
 import (
 	"context"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/distribution/reference"
 	"github.com/docker/cli/cli/config"

internal/act/container/docker_build.go

@@ -12,7 +12,7 @@ import (
 	"os"
 	"path/filepath"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/moby/go-archive"
 	"github.com/moby/go-archive/compression"

internal/act/container/docker_network.go

@@ -13,7 +13,7 @@ import (
 	"strings"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/moby/moby/client"
 )

internal/act/container/docker_pull.go

@@ -11,7 +11,7 @@ import (
 	"fmt"
 	"strings"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/distribution/reference"
 	"github.com/moby/moby/api/pkg/authconfig"

internal/act/container/docker_run.go

@@ -22,8 +22,8 @@ import (
 	"strings"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/filecollector"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/filecollector"
 
 	"dario.cat/mergo"
 	cerrdefs "github.com/containerd/errdefs"

internal/act/container/docker_run_test.go

@@ -19,7 +19,7 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/pkg/stdcopy"

internal/act/container/docker_stub.go

@@ -12,7 +12,7 @@ import (
 	"runtime"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/moby/moby/api/types/system"
 )

internal/act/container/docker_volume.go

@@ -9,7 +9,7 @@ package container
 import (
 	"context"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/moby/moby/client"
 )

internal/act/container/host_environment.go

@@ -21,9 +21,9 @@ import (
 	"sync/atomic"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/filecollector"
-	"gitea.com/gitea/runner/act/lookpath"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/filecollector"
+	"gitea.com/gitea/runner/internal/act/lookpath"
 	"gitea.com/gitea/runner/internal/pkg/process"
 
 	"github.com/go-git/go-billy/v5/helper/polyfill"

internal/act/container/host_environment_test.go

@@ -17,7 +17,7 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"

internal/act/container/parse_env_file.go

@@ -12,7 +12,7 @@ import (
 	"io"
 	"strings"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"golang.org/x/text/encoding/unicode"
 	"golang.org/x/text/transform"

internal/act/ghcontext/github_context.go

@@ -13,8 +13,8 @@ import (
 	"fmt"
 	"strings"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/common/git"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/common/git"
 
 	"gitea.dev/actionslib/pkg/model"
 )

internal/act/runner/action.go

@@ -20,9 +20,9 @@ import (
 	"runtime"
 	"strings"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/common/git"
-	"gitea.com/gitea/runner/act/container"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/common/git"
+	"gitea.com/gitea/runner/internal/act/container"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/kballard/go-shellquote"
@@ -293,7 +293,9 @@ func removeGitIgnore(ctx context.Context, directory string) error {
 // same `act-dockeraction:latest` image on a shared docker daemon. A subsequent
 // repository would then silently run the image built for an earlier one.
 // Including the repository keeps the tag stable for caching within a repository
-// while preventing cross-repository collisions.
+// while preventing cross-repository collisions. A remote action needs the same
+// treatment, because its actionName is the shared checkout of its repository and
+// ref plus the action's path inside it.
 // See https://gitea.com/gitea/runner/issues/1039.
 func dockerActionImageTag(repository, actionName string, localAction bool) string {
 	name := actionName
@@ -302,11 +304,10 @@ func dockerActionImageTag(repository, actionName string, localAction bool) strin
 	}
 	// The human-readable name is sanitized by collapsing every non-alphanumeric character to "-".
 	sanitized := regexp.MustCompile("[^a-zA-Z0-9]").ReplaceAllString(name, "-")
-	if localAction {
-		// For local actions a short hash of the raw repository and action path is appended so the tag stays unique per repository.
-		sum := sha256.Sum256([]byte(repository + "\x00" + actionName))
-		sanitized += "-" + hex.EncodeToString(sum[:])[:12]
-	}
+	// Sanitizing is lossy, so a short hash of the raw repository and action path is appended, keeping
+	// the tag unique per repository and per action inside it.
+	sum := sha256.Sum256([]byte(repository + "\x00" + actionName))
+	sanitized += "-" + hex.EncodeToString(sum[:])[:12]
 	// "-dockeraction" ensures that "./", "./test " won't get converted to "act-:latest", "act-test-:latest" which are invalid docker image names
 	image := fmt.Sprintf("%s-dockeraction:%s", sanitized, "latest")
 	image = "act-" + strings.TrimLeft(image, "-")

internal/act/runner/action_cache_test.go

@@ -17,7 +17,7 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/stretchr/testify/assert"
@@ -31,7 +31,7 @@ func runGit(t *testing.T, dir string, args ...string) {
 	}
 	cmd := exec.Command("git", args...)
 	// Fixed identity and host-config isolation so commits succeed offline regardless of the
-	// host's git config (mirrors gitCmd in act/common/git).
+	// host's git config (mirrors gitCmd in internal/act/common/git).
 	cmd.Env = append(os.Environ(),
 		"GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@example.com",
 		"GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@example.com",

internal/act/runner/action_composite.go

@@ -12,7 +12,7 @@ import (
 	"strconv"
 	"strings"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"gitea.dev/actionslib/pkg/model"
 )

internal/act/runner/action_test.go

@@ -13,9 +13,9 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/common/git"
-	"gitea.com/gitea/runner/act/container"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/common/git"
+	"gitea.com/gitea/runner/internal/act/container"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/stretchr/testify/assert"
@@ -496,11 +496,11 @@ func TestExecAsDockerHoldsCloneLockForRemoteUncached(t *testing.T) {
 }
 
 func TestDockerActionImageTag(t *testing.T) {
-	// Remote actions already carry a unique, ref-scoped actionName (the uses
-	// hash), so the tag must be left untouched for backwards compatibility.
-	assert.Equal(t,
-		"act-abc123-dockeraction:latest",
-		dockerActionImageTag("owner/repo", "abc123", false),
+	// A remote action's actionName is the checkout of its repository and ref plus its path inside it,
+	// and paths that sanitize alike share the readable prefix, so siblings must stay apart.
+	assert.NotEqual(t,
+		dockerActionImageTag("owner/repo", "abc123/a-b", false),
+		dockerActionImageTag("owner/repo", "abc123/a_b", false),
 	)
 
 	// Local actions keep a human-readable, repository-namespaced prefix and gain a short hash suffix that makes the tag unique per (repository, actionName).

internal/act/runner/cancellation_test.go

@@ -8,7 +8,7 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"gitea.dev/actionslib/pkg/exprparser"
 	"gitea.dev/actionslib/pkg/model"

internal/act/runner/command.go

@@ -10,7 +10,7 @@ import (
 	"regexp"
 	"strings"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 )
 
 var commandPatternGA *regexp.Regexp

internal/act/runner/command_test.go

@@ -11,7 +11,7 @@ import (
 	"os"
 	"testing"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/sirupsen/logrus/hooks/test"

internal/act/runner/container_mock_test.go

@@ -8,8 +8,8 @@ import (
 	"context"
 	"io"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/container"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/container"
 
 	"github.com/stretchr/testify/mock"
 )

internal/act/runner/expression.go

@@ -15,8 +15,8 @@ import (
 	"strings"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/container"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/container"
 
 	_ "embed"
 

internal/act/runner/helpers_test.go

@@ -9,7 +9,7 @@ import (
 	"runtime"
 	"testing"
 
-	"gitea.com/gitea/runner/act/container"
+	"gitea.com/gitea/runner/internal/act/container"
 
 	mobyclient "github.com/moby/moby/client"
 )

internal/act/runner/job_executor.go

@@ -21,8 +21,8 @@ import (
 	"time"
 	"unicode"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/container"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/container"
 
 	"gitea.dev/actionslib/pkg/exprparser"
 	"gitea.dev/actionslib/pkg/model"

internal/act/runner/job_executor_test.go

@@ -20,8 +20,8 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/container"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/container"
 
 	"gitea.dev/actionslib/pkg/model"
 	log "github.com/sirupsen/logrus"
@@ -146,7 +146,8 @@ func TestPrintPrepareActionsGolden(t *testing.T) {
 		&actionPreparerMock{reference: "actions/checkout@v7", sha: "9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0", ok: true},
 		// A resolved commit is best effort; the ref alone is reported when it is unknown.
 		&actionPreparerMock{reference: "actions/setup-go@v6", ok: true},
-		// A step that downloads nothing, such as the checkout of the workflow's own repository.
+		// A step that downloads nothing, such as the checkout of the workflow's own repository or a
+		// second step on an action the job already downloaded.
 		&actionPreparerMock{ok: false},
 	}
 	require.NoError(t, printPrepareActions(&RunContext{}, preparers)(ctx))

internal/act/runner/job_hooks.go

@@ -11,8 +11,8 @@ import (
 	"path"
 	"strings"
 
-	"gitea.com/gitea/runner/act/common"
-	"gitea.com/gitea/runner/act/container"
+	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/internal/act/container"
 )
 
 // GitHub's job-hook variables, read as a fallback when the settings are unset.

internal/act/runner/job_hooks_test.go

@@ -11,7 +11,7 @@ import (
 	"maps"
 	"testing"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/sirupsen/logrus/hooks/test"

internal/act/runner/local_repository_cache.go

@@ -16,7 +16,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	"gitea.com/gitea/runner/act/filecollector"
+	"gitea.com/gitea/runner/internal/act/filecollector"
 )
 
 type LocalRepositoryCache struct {

internal/act/runner/logger.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 	"sync"
 
-	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/act/common"
 
 	"github.com/sirupsen/logrus"
 	"golang.org/x/term"