fix: avoid updating WORKSPACE file when running update-repos when…

… bzlmod is enabled (#1589) * Implemented failing test. * First pass at logic * Finished tests * Remove debug code * Fix comment * Use t.Cleanup to register the cleanup funciton * Missed a cleanup. Switch to os.ReadFile
bazelbuild · Aug 8, 2023 · 18e0fd3 · 18e0fd3
1 parent ded0811
commit 18e0fd3
Show file tree

Hide file tree

Showing 2 changed files with 120 additions and 22 deletions.
diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go
@@ -4364,3 +4364,95 @@ go_library(
 		t.Fatalf("got %s ; want %s; diff %s", string(got), want, cmp.Diff(string(got), want))
 	}
 }
+
+func TestUpdateReposWithBzlmodWithToMacro(t *testing.T) {
+	dir, cleanup := testtools.CreateFiles(t, []testtools.FileSpec{
+		{Path: "WORKSPACE"},
+		{
+			Path: "go.mod",
+			Content: `
+module example.com/foo/v2
+
+go 1.19
+
+require (
+	github.com/stretchr/testify v1.8.4
+)
+`,
+		},
+	})
+
+	t.Cleanup(cleanup)
+
+	args := []string{
+		"update-repos",
+		"-from_file=go.mod",
+		"-to_macro=go_deps.bzl%my_go_deps",
+		"-bzlmod",
+	}
+	if err := runGazelle(dir, args); err != nil {
+		t.Fatal(err)
+	}
+
+	// Confirm that the WORKSPACE is still empty
+	want := ""
+	if got, err := os.ReadFile(filepath.Join(dir, "WORKSPACE")); err != nil {
+		t.Fatal(err)
+	} else if string(got) != want {
+		t.Fatalf("got %s ; want %s; diff %s", string(got), want, cmp.Diff(string(got), want))
+	}
+
+	// Confirm that the macro file was written
+	want = `load("@bazel_gazelle//:deps.bzl", "go_repository")
+
+def my_go_deps():
+    go_repository(
+        name = "com_github_stretchr_testify",
+        importpath = "github.com/stretchr/testify",
+        sum = "h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=",
+        version = "v1.8.4",
+    )
+`
+	if got, err := os.ReadFile(filepath.Join(dir, "go_deps.bzl")); err != nil {
+		t.Fatal(err)
+	} else if string(got) != want {
+		t.Fatalf("got %s ; want %s; diff %s", string(got), want, cmp.Diff(string(got), want))
+	}
+}
+
+func TestUpdateReposWithBzlmodWithoutToMacro(t *testing.T) {
+	dir, cleanup := testtools.CreateFiles(t, []testtools.FileSpec{
+		{Path: "WORKSPACE"},
+		{
+			Path: "go.mod",
+			Content: `
+module example.com/foo/v2
+
+go 1.19
+
+require (
+	github.com/stretchr/testify v1.8.4
+)
+`,
+		},
+	})
+
+	t.Cleanup(cleanup)
+
+	args := []string{
+		"update-repos",
+		"-from_file=go.mod",
+		"-bzlmod",
+	}
+	if err := runGazelle(dir, args); err != nil {
+		t.Fatal(err)
+	}
+
+	// Confirm that the WORKSPACE is still empty
+	want := ""
+	if got, err := os.ReadFile(filepath.Join(dir, "WORKSPACE")); err != nil {
+		t.Fatal(err)
+	} else if string(got) != want {
+		t.Fatalf("got %s ; want %s; diff %s", string(got), want, cmp.Diff(string(got), want))
+	}
+}
diff --git a/cmd/gazelle/update-repos.go b/cmd/gazelle/update-repos.go
@@ -233,35 +233,40 @@ func updateRepos(wd string, args []string) (err error) {
 		emptyForFiles[f] = append(emptyForFiles[f], r)
 	}
 
-	var newGenFile *rule.File
 	var macroPath string
 	if uc.macroFileName != "" {
 		macroPath = filepath.Join(c.RepoRoot, filepath.Clean(uc.macroFileName))
 	}
-	for f := range genForFiles {
-		if macroPath == "" && wspace.IsWORKSPACE(f.Path) ||
-			macroPath != "" && f.Path == macroPath && f.DefName == uc.macroDefName {
-			newGenFile = f
-			break
+	// If we are in bzlmod mode, then do not update the workspace. However, if a macro file was
+	// specified, proceed with generating the macro file. This is useful for rule repositories that
+	// build with bzlmod enabled, but support clients that use legacy WORKSPACE dependency loading.
+	if !c.Bzlmod || macroPath != "" {
+		var newGenFile *rule.File
+		for f := range genForFiles {
+			if macroPath == "" && wspace.IsWORKSPACE(f.Path) ||
+				macroPath != "" && f.Path == macroPath && f.DefName == uc.macroDefName {
+				newGenFile = f
+				break
+			}
 		}
-	}
-	if newGenFile == nil {
-		if uc.macroFileName == "" {
-			newGenFile = uc.workspace
-		} else {
-			var err error
-			newGenFile, err = rule.LoadMacroFile(macroPath, "", uc.macroDefName)
-			if os.IsNotExist(err) {
-				newGenFile, err = rule.EmptyMacroFile(macroPath, "", uc.macroDefName)
-				if err != nil {
-					return fmt.Errorf("error creating %q: %v", macroPath, err)
+		if newGenFile == nil {
+			if uc.macroFileName == "" {
+				newGenFile = uc.workspace
+			} else {
+				var err error
+				newGenFile, err = rule.LoadMacroFile(macroPath, "", uc.macroDefName)
+				if os.IsNotExist(err) {
+					newGenFile, err = rule.EmptyMacroFile(macroPath, "", uc.macroDefName)
+					if err != nil {
+						return fmt.Errorf("error creating %q: %v", macroPath, err)
+					}
+				} else if err != nil {
+					return fmt.Errorf("error loading %q: %v", macroPath, err)
 				}
-			} else if err != nil {
-				return fmt.Errorf("error loading %q: %v", macroPath, err)
 			}
 		}
+		genForFiles[newGenFile] = append(genForFiles[newGenFile], newGen...)
 	}
-	genForFiles[newGenFile] = append(genForFiles[newGenFile], newGen...)
 
 	workspaceInsertIndex := findWorkspaceInsertIndex(uc.workspace, kinds, loads)
 	for _, r := range genForFiles[uc.workspace] {
@@ -283,7 +288,8 @@ func updateRepos(wd string, args []string) (err error) {
 			sortedFiles = append(sortedFiles, f)
 		}
 	}
-	if ensureMacroInWorkspace(uc, workspaceInsertIndex) {
+	// If we are in bzlmod mode, then do not update the workspace.
+	if !c.Bzlmod && ensureMacroInWorkspace(uc, workspaceInsertIndex) {
 		if !seenFile[uc.workspace] {
 			seenFile[uc.workspace] = true
 			sortedFiles = append(sortedFiles, uc.workspace)
@@ -300,7 +306,7 @@ func updateRepos(wd string, args []string) (err error) {
 	for _, f := range sortedFiles {
 		merger.MergeFile(f, emptyForFiles[f], genForFiles[f], merger.PreResolve, kinds)
 		merger.FixLoads(f, loads)
-		if f == uc.workspace {
+		if f == uc.workspace && !c.Bzlmod {
 			if err := merger.CheckGazelleLoaded(f); err != nil {
 				return err
 			}