refactor(logging): replace log.Printf with injected zap logger and remove dead code

core/git/BUILD.bazel

@@ -5,7 +5,10 @@ go_library(
     srcs = ["git.go"],
     importpath = "github.com/uber/tango/core/git",
     visibility = ["//visibility:public"],
-    deps = ["//core/execcmd"],
+    deps = [
+        "//core/execcmd",
+        "@org_uber_go_zap//:zap",
+    ],
 )
 
 go_test(
@@ -15,5 +18,6 @@ go_test(
     deps = [
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
+        "@org_uber_go_zap//:zap",
     ],
 )

core/git/git.go

@@ -21,14 +21,14 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"log"
 	"os"
 	"os/exec"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/uber/tango/core/execcmd"
+	"go.uber.org/zap"
 )
 
 const (
@@ -63,13 +63,19 @@ type Interface interface {
 type impl struct {
 	directory string
 	runner    commandRunner
+	logger    *zap.SugaredLogger
 }
 
-// New creates new Git interface implementation
-func New(directory string) Interface {
+// New creates new Git interface implementation. A nil logger is tolerated and
+// discards log output.
+func New(directory string, logger *zap.SugaredLogger) Interface {
+	if logger == nil {
+		logger = zap.NewNop().Sugar()
+	}
 	return &impl{
 		directory: directory,
 		runner:    &osExecRunner{},
+		logger:    logger,
 	}
 }
 
@@ -205,12 +211,12 @@ func (c *impl) FileHashes(ctx context.Context, ref string) (map[string][]byte, e
 		x := strings.Split(line, "\t")
 		parts := strings.Fields(x[0]) // strings.Split is guaranteed to return an array of length 1
 		if len(parts) < 3 || len(x) < 2 {
-			log.Printf("skipping %q due to unexpected format\n", line)
+			c.logger.Warnw("skipping ls-tree line due to unexpected format", "line", line)
 			continue
 		}
 		hash, err := hex.DecodeString(parts[2])
 		if err != nil {
-			log.Printf("skipping %q due to parsing error %v\n", line, err)
+			c.logger.Warnw("skipping ls-tree line due to parsing error", "line", line, zap.Error(err))
 			continue
 		}
 		fileHashes[x[1]] = hash

core/git/git_test.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
 )
 
 type runnerCall struct {
@@ -253,6 +254,7 @@ func TestDefaultGit_FileHashes(t *testing.T) {
 			g := &impl{
 				directory: "/repo",
 				runner:    m,
+				logger:    zap.NewNop().Sugar(),
 			}
 			m.out = tt.giveOutput
 			m.err = tt.wantError

core/itg/graph/graph.go

@@ -15,7 +15,6 @@
 package graph
 
 import (
-	"log"
 	"maps"
 	"slices"
 
@@ -237,25 +236,6 @@ func (g *OptimizedGraph) AddTarget(target *targethasher.Target) {
 	}
 }
 
-// GetReverseDepsAsTargets returns the reverse dependencies of the given targets.
-func (g *OptimizedGraph) GetReverseDepsAsTargets(targetNames []string) []targethasher.Target {
-	reverseDeps := make([]targethasher.Target, 0, 2*len(targetNames))
-	visited := NewIntSet()
-	reverseDepIDs := make([]int, 0, len(targetNames))
-	for _, targetName := range targetNames {
-		targetID, ok := g.TargetNameToID[targetName]
-		if !ok {
-			log.Printf("starting target %s is not in the graph", targetName)
-			continue
-		}
-		reverseDepIDs = g.getNewReverseDeps(targetID, reverseDepIDs, visited)
-	}
-	for _, reverseDepID := range reverseDepIDs {
-		reverseDeps = append(reverseDeps, g.OptimizedTargetToTarget(reverseDepID))
-	}
-	return reverseDeps
-}
-
 // OptimizedTargetToTarget converts an OptimizedTarget back to a Target.
 func (g *OptimizedGraph) OptimizedTargetToTarget(targetID int) targethasher.Target {
 	name, ok := g.TargetIDToString[targetID]
@@ -300,19 +280,6 @@ func (g *OptimizedGraph) OptimizedTargetToTarget(targetID int) targethasher.Targ
 	return target
 }
 
-func (g *OptimizedGraph) getNewReverseDeps(targetID int, reverseDeps []int, visited IntSet) []int {
-	if visited.Contains(targetID) {
-		return reverseDeps
-	}
-
-	visited.Insert(targetID)
-	reverseDeps = append(reverseDeps, targetID)
-	for reverseDep := range g.OptimizedTargets[targetID].ReverseDeps {
-		reverseDeps = g.getNewReverseDeps(reverseDep, reverseDeps, visited)
-	}
-	return reverseDeps
-}
-
 func (g *OptimizedGraph) getOrGenerateTargetID(targetName string) int {
 	if id, ok := g.TargetNameToID[targetName]; ok {
 		return id

core/itg/graph/graph_test.go

@@ -247,66 +247,6 @@ func TestOptimizedGraphCopy(t *testing.T) {
 	assert.NotEqual(t, []byte{0xFF}, g.OptimizedTargets[bID].Hash)
 }
 
-// --- GetReverseDepsAsTargets ---
-
-func TestGetReverseDepsAsTargets(t *testing.T) {
-	t.Parallel()
-
-	t.Run("starting target included in results", func(t *testing.T) {
-		t.Parallel()
-		g := OptimizeGraph(map[string]*targethasher.Target{
-			"//pkg:a": {Name: "//pkg:a", RuleType: "go_library"},
-		})
-		result := g.GetReverseDepsAsTargets([]string{"//pkg:a"})
-		require.Len(t, result, 1)
-		assert.Equal(t, "//pkg:a", result[0].Name)
-	})
-
-	t.Run("transitive reverse deps all returned", func(t *testing.T) {
-		t.Parallel()
-		// a ← b ← c; starting from a should yield a, b, c
-		targets := map[string]*targethasher.Target{
-			"//pkg:a": {Name: "//pkg:a", RuleType: "go_library"},
-			"//pkg:b": {Name: "//pkg:b", RuleType: "go_library", Deps: []string{"//pkg:a"}},
-			"//pkg:c": {Name: "//pkg:c", RuleType: "go_library", Deps: []string{"//pkg:b"}},
-		}
-		g := OptimizeGraph(targets)
-		result := g.GetReverseDepsAsTargets([]string{"//pkg:a"})
-
-		names := make(map[string]struct{}, len(result))
-		for _, t := range result {
-			names[t.Name] = struct{}{}
-		}
-		assert.Contains(t, names, "//pkg:a")
-		assert.Contains(t, names, "//pkg:b")
-		assert.Contains(t, names, "//pkg:c")
-	})
-
-	t.Run("unrelated targets not returned", func(t *testing.T) {
-		t.Parallel()
-		targets := map[string]*targethasher.Target{
-			"//pkg:a":         {Name: "//pkg:a", RuleType: "go_library"},
-			"//pkg:b":         {Name: "//pkg:b", RuleType: "go_library", Deps: []string{"//pkg:a"}},
-			"//pkg:unrelated": {Name: "//pkg:unrelated", RuleType: "go_library"},
-		}
-		g := OptimizeGraph(targets)
-		result := g.GetReverseDepsAsTargets([]string{"//pkg:a"})
-
-		names := make(map[string]struct{}, len(result))
-		for _, t := range result {
-			names[t.Name] = struct{}{}
-		}
-		assert.NotContains(t, names, "//pkg:unrelated")
-	})
-
-	t.Run("unknown target is skipped", func(t *testing.T) {
-		t.Parallel()
-		g := OptimizeGraph(nil)
-		result := g.GetReverseDepsAsTargets([]string{"//pkg:missing"})
-		assert.Empty(t, result)
-	})
-}
-
 // --- OptimizedTargetToTarget ---
 
 func TestOptimizedTargetToTarget(t *testing.T) {

core/repomanager/repo_manager.go

@@ -157,7 +157,7 @@ func (r *repoManager) Lease(ctx context.Context, desc tangopb.BuildDescription)
 		slot.created = true
 	}
 
-	repoGit := git.New(slot.dir)
+	repoGit := git.New(slot.dir, r.logger)
 	return workspace.NewWorkspace(workspace.WorkspaceParams{
 		Path:   slot.dir,
 		Git:    repoGit,

example/main.go

@@ -83,7 +83,7 @@ func run() error {
 	defer os.RemoveAll(workerRootPath)
 
 	rm, err := repomanager.NewRepoManager(appCtx, repomanager.Params{
-		Git:                  git.New(repoManagerClonePath),
+		Git:                  git.New(repoManagerClonePath, logger),
 		Logger:               logger,
 		RepoManagerClonePath: repoManagerClonePath,
 		WorkerRootPath:       workerRootPath,
@@ -96,7 +96,7 @@ func run() error {
 		Storage:     store,
 		RepoManager: rm,
 		Logger:      logger,
-		GitFactory:  git.New,
+		GitFactory:  func(dir string) git.Interface { return git.New(dir, logger) },
 		Config:      cfg,
 	})
 	if err != nil {

orchestrator/native_orchestrator.go

@@ -142,12 +142,12 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, param GetTarget
 	logger.Infow("GetTargetGraph: Checked out base revision")
 
 	requests := make([]workspace.Request, 0, len(param.Req.BuildDescription.Requests))
-	factory := b.gitFactory
-	if factory == nil {
-		factory = git.New
+	gitFactory := b.gitFactory
+	if gitFactory == nil {
+		gitFactory = func(dir string) git.Interface { return git.New(dir, b.logger) }
 	}
 
-	gitModule := factory(ws.Path())
+	gitModule := gitFactory(ws.Path())
 	for _, req := range param.Req.BuildDescription.Requests {
 		request, err := workspace.NewRequest(req.GetUrl(), gitModule, param.Req.BuildDescription.BaseSha, req.GetCommit(), logger)
 		if err != nil {