From 4d73e4dc6a0bbf5e75ab459bce5483699a7809a9 Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Thu, 27 Jul 2023 17:49:22 -0600 Subject: [PATCH 1/7] Implemented failing test. --- cmd/gazelle/integration_test.go | 54 +++++++++++++++++++++++++++++++++ cmd/gazelle/update-repos.go | 7 ++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index cfc803f47..9e1493e51 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -4364,3 +4364,57 @@ go_library( t.Fatalf("got %s ; want %s; diff %s", string(got), want, cmp.Diff(string(got), want)) } } + +func TestUpdateReposWithBzlmodWithToMacro(t *testing.T) { + t.Error("IMPLEMENT ME!") +} + +func TestUpdateReposWithBzlmodWithoutToMacro(t *testing.T) { + dir, cleanup := testtools.CreateFiles(t, []testtools.FileSpec{ + {Path: "WORKSPACE"}, + { + Path: "MODULE.bazel", + Content: `bazel_dep(name = "rules_go", version = "0.39.1", repo_name = "bazel_gazelle")`, + }, + { + Path: "go.mod", + Content: ` +module example.com/foo/v2 + +go 1.19 + +require ( + github.com/stretchr/testify v1.8.4 +) +`, + }, + }) + + defer cleanup() + + // DEBUG BEGIN + log.Printf("*** CHUCK: TestUpdateReposWithBzlmodWithoutToMacro START ======================") + // DEBUG END + + 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 := ioutil.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)) + } + + // DEBUG BEGIN + log.Printf("*** CHUCK: TestUpdateReposWithBzlmodWithoutToMacro STOP ======================") + // DEBUG END + +} diff --git a/cmd/gazelle/update-repos.go b/cmd/gazelle/update-repos.go index 27c47c4b2..980f693e0 100644 --- a/cmd/gazelle/update-repos.go +++ b/cmd/gazelle/update-repos.go @@ -20,6 +20,7 @@ import ( "errors" "flag" "fmt" + "log" "os" "path/filepath" "sort" @@ -196,6 +197,10 @@ func updateRepos(wd string, args []string) (err error) { return err } + // DEBUG BEGIN + log.Printf("*** CHUCK: updateRepos c.Bzlmod: %+#v", c.Bzlmod) + // DEBUG END + // Organize generated and empty rules by file. A rule should go into the file // it came from (by name). New rules should go into WORKSPACE or the file // specified with -to_macro. @@ -300,7 +305,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 } From a7890870e54c9bf6aed6598a272efa3398289717 Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Fri, 28 Jul 2023 10:33:58 -0600 Subject: [PATCH 2/7] First pass at logic --- cmd/gazelle/integration_test.go | 13 --------- cmd/gazelle/update-repos.go | 50 +++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 34 deletions(-) diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index 9e1493e51..e9e1240a1 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -4372,10 +4372,6 @@ func TestUpdateReposWithBzlmodWithToMacro(t *testing.T) { func TestUpdateReposWithBzlmodWithoutToMacro(t *testing.T) { dir, cleanup := testtools.CreateFiles(t, []testtools.FileSpec{ {Path: "WORKSPACE"}, - { - Path: "MODULE.bazel", - Content: `bazel_dep(name = "rules_go", version = "0.39.1", repo_name = "bazel_gazelle")`, - }, { Path: "go.mod", Content: ` @@ -4392,10 +4388,6 @@ require ( defer cleanup() - // DEBUG BEGIN - log.Printf("*** CHUCK: TestUpdateReposWithBzlmodWithoutToMacro START ======================") - // DEBUG END - args := []string{ "update-repos", "-from_file=go.mod", @@ -4412,9 +4404,4 @@ require ( } else if string(got) != want { t.Fatalf("got %s ; want %s; diff %s", string(got), want, cmp.Diff(string(got), want)) } - - // DEBUG BEGIN - log.Printf("*** CHUCK: TestUpdateReposWithBzlmodWithoutToMacro STOP ======================") - // DEBUG END - } diff --git a/cmd/gazelle/update-repos.go b/cmd/gazelle/update-repos.go index 980f693e0..eadde992f 100644 --- a/cmd/gazelle/update-repos.go +++ b/cmd/gazelle/update-repos.go @@ -238,35 +238,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] { @@ -288,7 +293,10 @@ 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. 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 && ensureMacroInWorkspace(uc, workspaceInsertIndex) { if !seenFile[uc.workspace] { seenFile[uc.workspace] = true sortedFiles = append(sortedFiles, uc.workspace) From 0d68491c8c70b83c87c3f9be13e20bd846d17b7a Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Fri, 28 Jul 2023 10:48:11 -0600 Subject: [PATCH 3/7] Finished tests --- cmd/gazelle/integration_test.go | 56 ++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index e9e1240a1..59d943594 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -4366,7 +4366,61 @@ go_library( } func TestUpdateReposWithBzlmodWithToMacro(t *testing.T) { - t.Error("IMPLEMENT ME!") + 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 +) +`, + }, + }) + + defer 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 := ioutil.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 := ioutil.ReadFile(filepath.Join(dir, "go_deps.bzl")); err != nil { + t.Fatal(err) + } else if string(got) != want { + // DEBUG BEGIN + log.Printf("*** CHUCK: string(got):\n%s", string(got)) + // DEBUG END + t.Fatalf("got %s ; want %s; diff %s", string(got), want, cmp.Diff(string(got), want)) + } } func TestUpdateReposWithBzlmodWithoutToMacro(t *testing.T) { From 24f95bfefe1e1d32e1f4fa57901a5ddf837bdb31 Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Fri, 28 Jul 2023 10:53:09 -0600 Subject: [PATCH 4/7] Remove debug code --- cmd/gazelle/integration_test.go | 3 --- cmd/gazelle/update-repos.go | 5 ----- 2 files changed, 8 deletions(-) diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index 59d943594..4ecba589a 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -4416,9 +4416,6 @@ def my_go_deps(): if got, err := ioutil.ReadFile(filepath.Join(dir, "go_deps.bzl")); err != nil { t.Fatal(err) } else if string(got) != want { - // DEBUG BEGIN - log.Printf("*** CHUCK: string(got):\n%s", string(got)) - // DEBUG END 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 index eadde992f..f285b56d3 100644 --- a/cmd/gazelle/update-repos.go +++ b/cmd/gazelle/update-repos.go @@ -20,7 +20,6 @@ import ( "errors" "flag" "fmt" - "log" "os" "path/filepath" "sort" @@ -197,10 +196,6 @@ func updateRepos(wd string, args []string) (err error) { return err } - // DEBUG BEGIN - log.Printf("*** CHUCK: updateRepos c.Bzlmod: %+#v", c.Bzlmod) - // DEBUG END - // Organize generated and empty rules by file. A rule should go into the file // it came from (by name). New rules should go into WORKSPACE or the file // specified with -to_macro. From c792de883d0e5c12471d9b79cd775039a8ebfc51 Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Fri, 28 Jul 2023 10:55:38 -0600 Subject: [PATCH 5/7] Fix comment --- cmd/gazelle/update-repos.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmd/gazelle/update-repos.go b/cmd/gazelle/update-repos.go index f285b56d3..3de295ea3 100644 --- a/cmd/gazelle/update-repos.go +++ b/cmd/gazelle/update-repos.go @@ -288,9 +288,7 @@ func updateRepos(wd string, args []string) (err error) { sortedFiles = append(sortedFiles, f) } } - // 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 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 From 2869ce688eb3eedae0c975db3623cfb01f9be5e5 Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Mon, 7 Aug 2023 16:33:51 -0600 Subject: [PATCH 6/7] Use t.Cleanup to register the cleanup funciton --- cmd/gazelle/integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index 4ecba589a..3a85a5e0f 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -4382,7 +4382,7 @@ require ( }, }) - defer cleanup() + t.Cleanup(cleanup) args := []string{ "update-repos", From 0ba5c2959293c9e8ca7d635b98dc5e36b23f3252 Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Mon, 7 Aug 2023 16:38:05 -0600 Subject: [PATCH 7/7] Missed a cleanup. Switch to os.ReadFile --- cmd/gazelle/integration_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index 3a85a5e0f..5b7672dae 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -4396,7 +4396,7 @@ require ( // Confirm that the WORKSPACE is still empty want := "" - if got, err := ioutil.ReadFile(filepath.Join(dir, "WORKSPACE")); err != nil { + 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)) @@ -4413,7 +4413,7 @@ def my_go_deps(): version = "v1.8.4", ) ` - if got, err := ioutil.ReadFile(filepath.Join(dir, "go_deps.bzl")); err != nil { + 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)) @@ -4437,7 +4437,7 @@ require ( }, }) - defer cleanup() + t.Cleanup(cleanup) args := []string{ "update-repos", @@ -4450,7 +4450,7 @@ require ( // Confirm that the WORKSPACE is still empty want := "" - if got, err := ioutil.ReadFile(filepath.Join(dir, "WORKSPACE")); err != nil { + 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))