refactor(logging): use the logging package for the remaining act log calls (#1301)

Move the global logrus calls left in `act` onto `logging`, and add `SetOutput` and `Output` for tests that capture the process logger. Behavior is unchanged.

Stacked on https://gitea.com/gitea/runner/pulls/1300.

Assisted-by: Codet
Co-authored-by: bircni <bircni@icloud.com>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1301
Reviewed-by: bircni <bircni@icloud.com>

act/common/executor.go

@@ -9,7 +9,7 @@ import (
 	"fmt"
 	"runtime/debug"
 
-	log "github.com/sirupsen/logrus"
+	"gitea.com/gitea/runner/act/common/logging"
 )
 
 // Executor define contract for the steps of a workflow
@@ -74,33 +74,33 @@ func NewParallelExecutor(parallel int, executors ...Executor) Executor {
 		errs := make(chan error, len(executors))
 
 		if 1 > parallel {
-			log.Debugf("Parallel tasks (%d) below minimum, setting to 1", parallel)
+			logging.Debugf("Parallel tasks (%d) below minimum, setting to 1", parallel)
 			parallel = 1
 		}
 
-		log.Infof("NewParallelExecutor: Creating %d workers for %d executors", parallel, len(executors))
+		logging.Infof("NewParallelExecutor: Creating %d workers for %d executors", parallel, len(executors))
 
 		for i := 0; i < parallel; i++ {
 			go func(workerID int, work <-chan Executor, errs chan<- error) {
-				log.Debugf("Worker %d started", workerID)
+				logging.Debugf("Worker %d started", workerID)
 				taskCount := 0
 				for executor := range work {
 					taskCount++
-					log.Debugf("Worker %d executing task %d", workerID, taskCount)
+					logging.Debugf("Worker %d executing task %d", workerID, taskCount)
 					// Recover from panics in executors to avoid crashing the worker
 					// goroutine which would leave the runner process hung.
 					// https://gitea.com/gitea/runner/issues/371
 					errs <- func() (err error) {
 						defer func() {
 							if r := recover(); r != nil {
-								log.Errorf("panic in executor: %v\n%s", r, debug.Stack())
+								logging.Errorf("panic in executor: %v\n%s", r, debug.Stack())
 								err = fmt.Errorf("panic: %v", r)
 							}
 						}()
 						return executor(ctx)
 					}()
 				}
-				log.Debugf("Worker %d finished (%d tasks executed)", workerID, taskCount)
+				logging.Debugf("Worker %d finished (%d tasks executed)", workerID, taskCount)
 			}(i, work, errs)
 		}
 

act/common/logging/global.go

@@ -3,7 +3,11 @@
 
 package logging
 
-import "github.com/sirupsen/logrus"
+import (
+	"io"
+
+	"github.com/sirupsen/logrus"
+)
 
 var std = New(NewLogrusHandler(logrus.StandardLogger()))
 
@@ -16,6 +20,12 @@ func SetLevel(level Level) { logrus.SetLevel(toLogrusLevel(level)) }
 // GetLevel returns the minimum level of the process-wide Logger.
 func GetLevel() Level { return fromLogrusLevel(logrus.GetLevel()) }
 
+// SetOutput sets where the process-wide Logger writes.
+func SetOutput(w io.Writer) { logrus.SetOutput(w) }
+
+// Output returns where the process-wide Logger writes.
+func Output() io.Writer { return logrus.StandardLogger().Out }
+
 func Enabled(level Level) bool { return std.Enabled(level) }
 
 func With(args ...any) *Logger          { return std.With(args...) }

act/common/logging/logging_test.go

@@ -100,13 +100,13 @@ func TestCallerPC(t *testing.T) {
 	}
 
 	std := logrus.StandardLogger()
-	prevOut, prevCaller, prevHooks := std.Out, std.ReportCaller, std.ReplaceHooks(logrus.LevelHooks{})
+	prevOut, prevCaller, prevHooks := logging.Output(), std.ReportCaller, std.ReplaceHooks(logrus.LevelHooks{})
 	t.Cleanup(func() {
-		std.SetOutput(prevOut)
+		logging.SetOutput(prevOut)
 		std.SetReportCaller(prevCaller)
 		std.ReplaceHooks(prevHooks)
 	})
-	std.SetOutput(io.Discard)
+	logging.SetOutput(io.Discard)
 	std.SetReportCaller(true)
 	hook := logrustest.NewGlobal()
 	logging.Infof("global")

act/container/docker_images_test.go

@@ -12,13 +12,14 @@ import (
 	"strings"
 	"testing"
 
-	log "github.com/sirupsen/logrus"
+	"gitea.com/gitea/runner/act/common/logging"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func init() {
-	log.SetLevel(log.DebugLevel)
+	logging.SetLevel(logging.LevelDebug)
 }
 
 // buildScratchImage builds a tiny empty image for the given platform locally (FROM scratch, no

act/container/linux_container_environment_extensions.go

@@ -11,7 +11,7 @@ import (
 	"runtime"
 	"strings"
 
-	log "github.com/sirupsen/logrus"
+	"gitea.com/gitea/runner/act/common/logging"
 )
 
 type LinuxContainerEnvironmentExtensions struct{}
@@ -21,13 +21,13 @@ type LinuxContainerEnvironmentExtensions struct{}
 // For use in docker volumes and binds
 func (*LinuxContainerEnvironmentExtensions) ToContainerPath(path string) string {
 	if runtime.GOOS == "windows" && strings.Contains(path, "/") {
-		log.Error("You cannot specify linux style local paths (/mnt/etc) on Windows as it does not understand them.")
+		logging.Error("You cannot specify linux style local paths (/mnt/etc) on Windows as it does not understand them.")
 		return ""
 	}
 
 	abspath, err := filepath.Abs(path)
 	if err != nil {
-		log.Error(err)
+		logging.Error(err)
 		return ""
 	}
 

act/container/linux_container_environment_extensions_test.go

@@ -11,7 +11,8 @@ import (
 	"strings"
 	"testing"
 
-	log "github.com/sirupsen/logrus"
+	"gitea.com/gitea/runner/act/common/logging"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -27,7 +28,7 @@ func TestContainerPath(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		cwd, err := os.Getwd()
 		if err != nil {
-			log.Error(err)
+			logging.Error(err)
 		}
 
 		rootDrive := os.Getenv("SystemDrive")
@@ -49,7 +50,7 @@ func TestContainerPath(t *testing.T) {
 	} else {
 		cwd, err := os.Getwd()
 		if err != nil {
-			log.Error(err)
+			logging.Error(err)
 		}
 		for _, v := range []containerPathJob{
 			{"/home/act/go/src/github.com/nektos/act", "/home/act/go/src/github.com/nektos/act", ""},

act/ghcontext/github_context_test.go

@@ -9,13 +9,14 @@ import (
 	"errors"
 	"testing"
 
+	"gitea.com/gitea/runner/act/common/logging"
+
 	"gitea.dev/actionslib/pkg/model"
-	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestSetRef(t *testing.T) {
-	log.SetLevel(log.DebugLevel)
+	logging.SetLevel(logging.LevelDebug)
 
 	oldFindGitRef := findGitRef
 	oldFindGitRevision := findGitRevision
@@ -129,7 +130,7 @@ func TestSetRef(t *testing.T) {
 }
 
 func TestSetSha(t *testing.T) {
-	log.SetLevel(log.DebugLevel)
+	logging.SetLevel(logging.LevelDebug)
 
 	oldFindGitRef := findGitRef
 	oldFindGitRevision := findGitRevision

act/runner/runner.go

@@ -22,7 +22,6 @@ import (
 
 	"gitea.dev/actionslib/pkg/model"
 	docker_container "github.com/moby/moby/api/types/container"
-	log "github.com/sirupsen/logrus"
 	"go.yaml.in/yaml/v4"
 )
 
@@ -150,7 +149,7 @@ func (runner *runnerImpl) configure() (*runnerImpl, error) {
 	if runner.config.EventJSON != "" {
 		runner.eventJSON = runner.config.EventJSON
 	} else if runner.config.EventPath != "" {
-		log.Debugf("Reading event.json from %s", runner.config.EventPath)
+		logging.Debugf("Reading event.json from %s", runner.config.EventPath)
 		eventJSONBytes, err := os.ReadFile(runner.config.EventPath)
 		if err != nil {
 			return nil, err
@@ -164,7 +163,7 @@ func maxParallelFor(strategy *model.Strategy, combinations int) int {
 	maxParallel := 4 // actionslib has no default for an undeclared max-parallel
 	if strategy != nil {
 		if limit, err := strategy.MaxParallel(); err != nil {
-			log.Errorf("Ignoring invalid max-parallel: %v", err)
+			logging.Errorf("Ignoring invalid max-parallel: %v", err)
 		} else if limit > 0 {
 			maxParallel = limit
 		}
@@ -186,36 +185,36 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
 	maxJobNameLen := 0
 
 	stagePipeline := make([]common.Executor, 0)
-	log.Debugf("Plan Stages: %v", plan.Stages)
+	logging.Debugf("Plan Stages: %v", plan.Stages)
 
 	for i := range plan.Stages {
 		stage := plan.Stages[i]
 		stagePipeline = append(stagePipeline, func(ctx context.Context) error {
 			pipeline := make([]common.Executor, 0)
 			for _, run := range stage.Runs {
-				log.Debugf("Stages Runs: %v", stage.Runs)
+				logging.Debugf("Stages Runs: %v", stage.Runs)
 				stageExecutor := make([]common.Executor, 0)
 				job := run.Job()
-				log.Debugf("Job.Name: %v", job.Name)
-				log.Debugf("Job.RawNeeds: %v", job.RawNeeds)
-				log.Debugf("Job.RawRunsOn: %v", job.RawRunsOn)
-				log.Debugf("Job.Env: %v", job.Env)
-				log.Debugf("Job.If: %v", job.If)
+				logging.Debugf("Job.Name: %v", job.Name)
+				logging.Debugf("Job.RawNeeds: %v", job.RawNeeds)
+				logging.Debugf("Job.RawRunsOn: %v", job.RawRunsOn)
+				logging.Debugf("Job.Env: %v", job.Env)
+				logging.Debugf("Job.If: %v", job.If)
 				for step := range job.Steps {
 					if nil != job.Steps[step] {
-						log.Debugf("Job.Steps: %v", job.Steps[step].String())
+						logging.Debugf("Job.Steps: %v", job.Steps[step].String())
 					}
 				}
-				log.Debugf("Job.TimeoutMinutes: %v", job.TimeoutMinutes)
-				log.Debugf("Job.Services: %v", job.Services)
-				log.Debugf("Job.Strategy: %v", job.Strategy)
-				log.Debugf("Job.RawContainer: %v", job.RawContainer)
-				log.Debugf("Job.Defaults.Run.Shell: %v", job.Defaults.Run.Shell)
-				log.Debugf("Job.Defaults.Run.WorkingDirectory: %v", job.Defaults.Run.WorkingDirectory)
-				log.Debugf("Job.Outputs: %v", job.Outputs)
-				log.Debugf("Job.Uses: %v", job.Uses)
-				log.Debugf("Job.With: %v", job.With)
-				log.Debugf("Job.Result: %v", job.Result)
+				logging.Debugf("Job.TimeoutMinutes: %v", job.TimeoutMinutes)
+				logging.Debugf("Job.Services: %v", job.Services)
+				logging.Debugf("Job.Strategy: %v", job.Strategy)
+				logging.Debugf("Job.RawContainer: %v", job.RawContainer)
+				logging.Debugf("Job.Defaults.Run.Shell: %v", job.Defaults.Run.Shell)
+				logging.Debugf("Job.Defaults.Run.WorkingDirectory: %v", job.Defaults.Run.WorkingDirectory)
+				logging.Debugf("Job.Outputs: %v", job.Outputs)
+				logging.Debugf("Job.Uses: %v", job.Uses)
+				logging.Debugf("Job.With: %v", job.With)
+				logging.Debugf("Job.Result: %v", job.Result)
 
 				if job.Strategy != nil || job.RawStrategy.Kind == yaml.ScalarNode {
 					strategyRc, err := runner.newRunContext(ctx, run, nil)
@@ -227,13 +226,13 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
 							return err
 						}
 					} else {
-						log.Debugf("Job.Strategy.FailFast: %v", job.Strategy.GetFailFast())
-						log.Debugf("Job.Strategy.FailFastString: %v", job.Strategy.FailFastString)
-						log.Debugf("Job.Strategy.MaxParallelString: %v", job.Strategy.MaxParallelString)
-						log.Debugf("Job.Strategy.RawMatrix: %v", job.Strategy.RawMatrix)
+						logging.Debugf("Job.Strategy.FailFast: %v", job.Strategy.GetFailFast())
+						logging.Debugf("Job.Strategy.FailFastString: %v", job.Strategy.FailFastString)
+						logging.Debugf("Job.Strategy.MaxParallelString: %v", job.Strategy.MaxParallelString)
+						logging.Debugf("Job.Strategy.RawMatrix: %v", job.Strategy.RawMatrix)
 						// An unevaluated expression is left in place, which GetMatrixes below rejects.
 						if err := strategyRc.NewExpressionEvaluator(ctx).EvaluateYamlNode(ctx, &job.Strategy.RawMatrix); err != nil {
-							log.Errorf("Error while evaluating matrix: %v", err)
+							logging.Errorf("Error while evaluating matrix: %v", err)
 						}
 						for _, value := range []*string{&job.Strategy.FailFastString, &job.Strategy.MaxParallelString} {
 							if evaluated, err := strategyRc.ExprEval.Interpolate(ctx, *value); err == nil {
@@ -247,11 +246,11 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
 				if err != nil {
 					return fmt.Errorf("could not get job matrix: %w", err)
 				}
-				log.Debugf("Job Matrices: %v", matrixes)
+				logging.Debugf("Job Matrices: %v", matrixes)
 
 				maxParallel := maxParallelFor(job.Strategy, len(matrixes))
 
-				log.Infof("Running job with maxParallel=%d for %d matrix combinations", maxParallel, len(matrixes))
+				logging.Infof("Running job with maxParallel=%d for %d matrix combinations", maxParallel, len(matrixes))
 
 				for i, matrix := range matrixes {
 					index, total := i, len(matrixes)
@@ -310,7 +309,7 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
 			if len(pipeline) == 1 {
 				// Single run/job: execute directly without additional parallelization wrapper
 				// This ensures max-parallel is the only limiting factor
-				log.Debugf("Single pipeline element, executing directly")
+				logging.Debugf("Single pipeline element, executing directly")
 				return pipeline[0](ctx)
 			}
 
@@ -320,9 +319,9 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
 			// If MaxParallel is set in config, use it
 			if runner.config.MaxParallel > 0 {
 				parallelism = runner.config.MaxParallel
-				log.Debugf("Using configured max-parallel: %d", parallelism)
+				logging.Debugf("Using configured max-parallel: %d", parallelism)
 			} else {
-				log.Debugf("Using CPU count for parallelism: %d", parallelism)
+				logging.Debugf("Using CPU count for parallelism: %d", parallelism)
 			}
 
 			// Don't exceed the number of pipeline elements
@@ -330,7 +329,7 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
 				parallelism = len(pipeline)
 			}
 
-			log.Infof("Executing %d pipeline elements with parallelism %d", len(pipeline), parallelism)
+			logging.Infof("Executing %d pipeline elements with parallelism %d", len(pipeline), parallelism)
 			return common.NewParallelExecutor(parallelism, pipeline...)(ctx)
 		})
 	}

act/runner/runner_test.go

@@ -17,10 +17,10 @@ import (
 	"time"
 
 	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/common/logging"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/joho/godotenv"
-	log "github.com/sirupsen/logrus"
 	assert "github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -28,7 +28,7 @@ import (
 var (
 	baseImage = "node:24-bookworm-slim"
 	platforms map[string]string
-	logLevel  = log.DebugLevel
+	logLevel  = logging.LevelDebug
 	workdir   = "testdata"
 	secrets   map[string]string
 	testRunID = strings.ToLower(rand.Text()[:8]) // keeps docker names apart from concurrent test processes on one daemon
@@ -55,7 +55,7 @@ func init() {
 	}
 
 	if l := os.Getenv("ACT_TEST_LOG_LEVEL"); l != "" {
-		if lvl, err := log.ParseLevel(l); err == nil {
+		if lvl, err := logging.ParseLevel(l); err == nil {
 			logLevel = lvl
 		}
 	}
@@ -71,10 +71,11 @@ func TestNoWorkflowsFoundByPlanner(t *testing.T) {
 	planner, err := model.NewWorkflowPlanner("res", true)
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
 
-	out := log.StandardLogger().Out
+	out := logging.Output()
+	t.Cleanup(func() { logging.SetOutput(out) })
 	var buf bytes.Buffer
-	log.SetOutput(&buf)
-	log.SetLevel(log.DebugLevel)
+	logging.SetOutput(&buf)
+	logging.SetLevel(logging.LevelDebug)
 	plan, err := planner.PlanEvent("pull_request")
 	assert.NotNil(t, plan)
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
@@ -84,17 +85,17 @@ func TestNoWorkflowsFoundByPlanner(t *testing.T) {
 	assert.NotNil(t, plan)
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
 	assert.Contains(t, buf.String(), "no workflows found by planner")
-	log.SetOutput(out)
 }
 
 func TestGraphMissingEvent(t *testing.T) {
 	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/no-event.yml", true)
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
 
-	out := log.StandardLogger().Out
+	out := logging.Output()
+	t.Cleanup(func() { logging.SetOutput(out) })
 	var buf bytes.Buffer
-	log.SetOutput(&buf)
-	log.SetLevel(log.DebugLevel)
+	logging.SetOutput(&buf)
+	logging.SetLevel(logging.LevelDebug)
 
 	plan, err := planner.PlanEvent("push")
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
@@ -102,7 +103,6 @@ func TestGraphMissingEvent(t *testing.T) {
 	assert.Empty(t, plan.Stages)
 
 	assert.Contains(t, buf.String(), "no events found for workflow: no-event.yml")
-	log.SetOutput(out)
 }
 
 func TestGraphMissingFirst(t *testing.T) {
@@ -119,29 +119,30 @@ func TestGraphWithMissing(t *testing.T) {
 	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/missing.yml", true)
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
 
-	out := log.StandardLogger().Out
+	out := logging.Output()
+	t.Cleanup(func() { logging.SetOutput(out) })
 	var buf bytes.Buffer
-	log.SetOutput(&buf)
-	log.SetLevel(log.DebugLevel)
+	logging.SetOutput(&buf)
+	logging.SetLevel(logging.LevelDebug)
 
 	plan, err := planner.PlanEvent("push")
 	assert.NotNil(t, plan)
 	assert.Empty(t, plan.Stages)
 	assert.EqualError(t, err, "unable to build dependency graph for missing (missing.yml)") //nolint:testifylint // pre-existing issue from nektos/act
 	assert.Contains(t, buf.String(), "unable to build dependency graph for missing (missing.yml)")
-	log.SetOutput(out)
 }
 
 func TestGraphWithSomeMissing(t *testing.T) {
-	log.SetLevel(log.DebugLevel)
+	logging.SetLevel(logging.LevelDebug)
 
 	planner, err := model.NewWorkflowPlanner("testdata/issue-1595/", true)
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
 
-	out := log.StandardLogger().Out
+	out := logging.Output()
+	t.Cleanup(func() { logging.SetOutput(out) })
 	var buf bytes.Buffer
-	log.SetOutput(&buf)
-	log.SetLevel(log.DebugLevel)
+	logging.SetOutput(&buf)
+	logging.SetLevel(logging.LevelDebug)
 
 	plan, err := planner.PlanAll()
 	assert.Error(t, err, "unable to build dependency graph for no first (no-first.yml)") //nolint:testifylint // pre-existing issue from nektos/act
@@ -149,7 +150,6 @@ func TestGraphWithSomeMissing(t *testing.T) {
 	assert.Len(t, plan.Stages, 1)
 	assert.Contains(t, buf.String(), "unable to build dependency graph for missing (missing.yml)")
 	assert.Contains(t, buf.String(), "unable to build dependency graph for no first (no-first.yml)")
-	log.SetOutput(out)
 }
 
 func TestNewPlanExecutorRequiresRunsOnBeforeRunningAnyJob(t *testing.T) {
@@ -221,7 +221,7 @@ type TestJobFileInfo struct {
 func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config) {
 	fmt.Printf("::group::%s\n", j.workflowPath) //nolint:forbidigo // pre-existing issue from nektos/act
 
-	log.SetLevel(logLevel)
+	logging.SetLevel(logLevel)
 
 	workdir, err := filepath.Abs(j.workdir)
 	assert.NoError(t, err, workdir) //nolint:testifylint // pre-existing issue from nektos/act