Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -to_macro option to update-repos #480

Merged
merged 3 commits into from
Mar 27, 2019
Merged

Add -to_macro option to update-repos #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
97de491
Add -to_macro option to update-repos
blico Mar 13, 2019
21aad1f
PR feedback
blico Mar 15, 2019
e29c199
PR feedback v2
blico Mar 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,8 @@ The following flags are accepted:
``update-repos``
~~~~~~~~~~~~~~~~

The ``update-repos`` command updates repository rules in the WORKSPACE file.
The ``update-repos`` command updates repository rules.
It can write the rules to either the WORKSPACE (by default) or a .bzl file macro function.
It can be used to add new repository rules or update existing rules to the
latest version. It can also import repository rules from a ``go.mod`` file or
a ``Gopkg.lock`` file.
Expand All @@ -384,6 +385,9 @@ a ``Gopkg.lock`` file.
# Import repositories from go.mod
$ gazelle update-repos -from_file=go.mod

# Import repositories from go.mod and update macro
$ gazelle update-repos -from_file=go.mod -to_macro=repositories.bzl%go_repositories

:Note: ``update-repos`` is not directly supported by the ``gazelle`` rule.
You can run it through the ``gazelle`` rule by passing extra arguments after
``--``. For example:
Expand All @@ -409,6 +413,12 @@ The following flags are accepted:
| |
| Gazelle will not process packages outside this directory. |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
| :flag:`-to_macro macroFile%defName` | |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
| Tells Gazelle to write repository rules into a .bzl macro function rather than the WORKSPACE file. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should probably be pointed out here that Gazelle won't see repositories declared in the macro when updating build files. That means that it won't use custom repository names, and it will go out to the network when trying to determine repository roots.

This is a solvable problem of course, but let's leave it for another time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, this was an oversight on my part. We are pretty dependent on the functionality you mentioned, so I would like to get this done sooner than later.

From a quick glance, it looks like one way to get this done would be to add a new flag named something like -go_repositories_macro=path. If the flag is set we call LoadMacroFile from newFixUpdateConfiguration. We then call FixWorkspace and FixLoads for both the macro and workspace files, and also pass both files to repo.ListRepositories.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#132 describes a # gazelle:repository directive that would declare this information in WORKSPACE without requiring command line arguments. That would be a useful escape hatch. I just opened #483, describing a more complete solution. In WORKSPACE, we could have a directive that tells Gazelle to read a macro in a separate .bzl file. Bonus points if we can update existing rules in that macro without needing the -to_macro flag.

Until those are implemented, we should mention this problem here, since the problem won't be obvious to people trying this out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simplify this problem a bit to start with. We can have a default bzl file like //go_repositories.bzl unless it's overridden by some the Gazelle directive proposed in #483. Then --to_macro becomes a boolean flag, and write go_repository rules to //go_repositories.bzl when the flag is set. update and fix will first check if there is a //go_repositories.bzl. If so, read those rules first. Then we should allow people to override rules in //go_repositories.bzl in WORKSPACE files. For example, if they want to apply a patch to some go_repository, they can redefine the rule with the same name in WORKSPACE. Then //go_repositories.bzl contains no manual edits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can imagine large repositories needing multiple macros for different sets of dependencies. When language extensions eventually support update-repos functionality, it should be possible to update macros for non-Go dependencies, too. Basically, I want to make sure we stay flexible here.

| Currently, Gazelle won't see repositories declared in the macro when updating build files. This means that it won't use custom repository names, and it |
| will go out to the network when trying to determine repository roots. |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
| :flag:`-build_file_names file1,file2,...` | |
+----------------------------------------------------------------------------------------------------------+----------------------------------------------+
| Sets the ``build_file_name`` attribute for the generated `go_repository`_ rule(s). |
Expand Down
172 changes: 171 additions & 1 deletion cmd/gazelle/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,7 @@ go_library(
})
}

func TestImportReposFromDep(t *testing.T) {
func TestImportReposFromDepToWorkspace(t *testing.T) {
files := []testtools.FileSpec{
{
Path: "WORKSPACE",
Expand Down Expand Up @@ -1253,6 +1253,176 @@ go_repository(
}})
}

func TestImportReposFromDepToMacro(t *testing.T) {
files := []testtools.FileSpec{
{Path: "WORKSPACE"},
{
Path: "repositories.bzl",
Content: `
load("@bazel_gazelle//:deps.bzl", "go_repository")

def go_repositories():
go_repository(
name = "org_golang_x_net",
importpath = "golang.org/x/net",
tag = "1.2",
)

# keep
go_repository(
name = "org_golang_x_sys",
importpath = "golang.org/x/sys",
remote = "https://github.com/golang/sys",
)

http_archive(
name = "com_github_go_yaml_yaml",
urls = ["https://example.com/yaml.tar.gz"],
sha256 = "1234",
)
`,
}, {
Path: "Gopkg.lock",
Content: `# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'.


[[projects]]
name = "github.com/pkg/errors"
packages = ["."]
revision = "645ef00459ed84a119197bfb8d8205042c6df63d"
version = "v0.8.0"

[[projects]]
branch = "master"
name = "golang.org/x/net"
packages = ["context"]
revision = "66aacef3dd8a676686c7ae3716979581e8b03c47"

[[projects]]
branch = "master"
name = "golang.org/x/sys"
packages = ["unix"]
revision = "bb24a47a89eac6c1227fbcb2ae37a8b9ed323366"

[[projects]]
branch = "v2"
name = "github.com/go-yaml/yaml"
packages = ["."]
revision = "cd8b52f8269e0feb286dfeef29f8fe4d5b397e0b"

[solve-meta]
analyzer-name = "dep"
analyzer-version = 1
inputs-digest = "05c1cd69be2c917c0cc4b32942830c2acfa044d8200fdc94716aae48a8083702"
solver-name = "gps-cdcl"
solver-version = 1
`,
},
}
dir, cleanup := testtools.CreateFiles(t, files)
defer cleanup()

args := []string{"update-repos", "-build_file_generation", "off", "-from_file", "Gopkg.lock", "-to_macro", "repositories.bzl%go_repositories"}
if err := runGazelle(dir, args); err != nil {
t.Fatal(err)
}

testtools.CheckFiles(t, dir, []testtools.FileSpec{
{
Path: "repositories.bzl",
Content: `
load("@bazel_gazelle//:deps.bzl", "go_repository")

def go_repositories():
go_repository(
name = "org_golang_x_net",
build_file_generation = "off",
commit = "66aacef3dd8a676686c7ae3716979581e8b03c47",
importpath = "golang.org/x/net",
)

# keep
go_repository(
name = "org_golang_x_sys",
importpath = "golang.org/x/sys",
remote = "https://github.com/golang/sys",
)

http_archive(
name = "com_github_go_yaml_yaml",
urls = ["https://example.com/yaml.tar.gz"],
sha256 = "1234",
)
go_repository(name = "com_github_pkg_errors", build_file_generation = "off", commit = "645ef00459ed84a119197bfb8d8205042c6df63d", importpath = "github.com/pkg/errors")
`,
}})
}

func TestImportReposFromDepToEmptyMacro(t *testing.T) {
files := []testtools.FileSpec{
{Path: "WORKSPACE"},
{
Path: "Gopkg.lock",
Content: `# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'.


[[projects]]
name = "github.com/pkg/errors"
packages = ["."]
revision = "645ef00459ed84a119197bfb8d8205042c6df63d"
version = "v0.8.0"

[[projects]]
branch = "master"
name = "golang.org/x/net"
packages = ["context"]
revision = "66aacef3dd8a676686c7ae3716979581e8b03c47"

[[projects]]
branch = "master"
name = "golang.org/x/sys"
packages = ["unix"]
revision = "bb24a47a89eac6c1227fbcb2ae37a8b9ed323366"

[[projects]]
branch = "v2"
name = "github.com/go-yaml/yaml"
packages = ["."]
revision = "cd8b52f8269e0feb286dfeef29f8fe4d5b397e0b"

[solve-meta]
analyzer-name = "dep"
analyzer-version = 1
inputs-digest = "05c1cd69be2c917c0cc4b32942830c2acfa044d8200fdc94716aae48a8083702"
solver-name = "gps-cdcl"
solver-version = 1
`,
},
}
dir, cleanup := testtools.CreateFiles(t, files)
defer cleanup()

args := []string{"update-repos", "-build_file_generation", "off", "-from_file", "Gopkg.lock", "-to_macro", "repositories.bzl%go_repositories"}
if err := runGazelle(dir, args); err != nil {
t.Fatal(err)
}

testtools.CheckFiles(t, dir, []testtools.FileSpec{
{
Path: "repositories.bzl",
Content: `
load("@bazel_gazelle//:deps.bzl", "go_repository")

def go_repositories():
go_repository(name = "com_github_go_yaml_yaml", build_file_generation = "off", commit = "cd8b52f8269e0feb286dfeef29f8fe4d5b397e0b", importpath = "github.com/go-yaml/yaml")
go_repository(name = "com_github_pkg_errors", build_file_generation = "off", commit = "645ef00459ed84a119197bfb8d8205042c6df63d", importpath = "github.com/pkg/errors")
go_repository(name = "org_golang_x_net", build_file_generation = "off", commit = "66aacef3dd8a676686c7ae3716979581e8b03c47", importpath = "golang.org/x/net")
go_repository(name = "org_golang_x_sys", build_file_generation = "off", commit = "bb24a47a89eac6c1227fbcb2ae37a8b9ed323366", importpath = "golang.org/x/sys")

`,
}})
}

func TestDeleteRulesInEmptyDir(t *testing.T) {
files := []testtools.FileSpec{
{Path: "WORKSPACE"},
Expand Down
55 changes: 48 additions & 7 deletions cmd/gazelle/update-repos.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"sync"

"github.com/bazelbuild/bazel-gazelle/config"
Expand All @@ -36,6 +37,8 @@ type updateReposConfig struct {
fn updateReposFn
lockFilename string
importPaths []string
macroFileName string
macroDefName string
buildExternalAttr string
buildFileNamesAttr string
buildFileGenerationAttr string
Expand All @@ -56,16 +59,36 @@ func getUpdateReposConfig(c *config.Config) *updateReposConfig {

type updateReposConfigurer struct{}

type macroFlag struct {
macroFileName *string
macroDefName *string
}

func (f macroFlag) Set(value string) error {
args := strings.Split(value, "%")
if len(args) != 2 {
return fmt.Errorf("Failure parsing to_macro: %s, expected format is macroFile%%defName", value)
}
*f.macroFileName = args[0]
*f.macroDefName = args[1]
return nil
}

func (f macroFlag) String() string {
return ""
}

func (_ *updateReposConfigurer) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) {
uc := &updateReposConfig{}
c.Exts[updateReposName] = uc
fs.StringVar(&uc.lockFilename, "from_file", "", "Gazelle will translate repositories listed in this file into repository rules in WORKSPACE. Currently only dep's Gopkg.lock is supported.")
fs.StringVar(&uc.lockFilename, "from_file", "", "Gazelle will translate repositories listed in this file into repository rules in WORKSPACE or a .bzl macro function. Gopkg.lock and go.mod files are supported")
fs.StringVar(&uc.buildFileNamesAttr, "build_file_names", "", "Sets the build_file_name attribute for the generated go_repository rule(s).")
fs.Var(&gzflag.AllowedStringFlag{Value: &uc.buildExternalAttr, Allowed: validBuildExternalAttr}, "build_external", "Sets the build_external attribute for the generated go_repository rule(s).")
fs.Var(&gzflag.AllowedStringFlag{Value: &uc.buildFileGenerationAttr, Allowed: validBuildFileGenerationAttr}, "build_file_generation", "Sets the build_file_generation attribute for the generated go_repository rule(s).")
fs.StringVar(&uc.buildTagsAttr, "build_tags", "", "Sets the build_tags attribute for the generated go_repository rule(s).")
fs.Var(&gzflag.AllowedStringFlag{Value: &uc.buildFileProtoModeAttr, Allowed: validBuildFileProtoModeAttr}, "build_file_proto_mode", "Sets the build_file_proto_mode attribute for the generated go_repository rule(s).")
fs.StringVar(&uc.buildExtraArgsAttr, "build_extra_args", "", "Sets the build_extra_args attribute for the generated go_repository rule(s).")
fs.Var(macroFlag{macroFileName: &uc.macroFileName, macroDefName: &uc.macroDefName}, "to_macro", "Tells Gazelle to write repository rules into a .bzl macro function rather than the WORKSPACE file. . The expected format is: macroFile%defName")
}

func (_ *updateReposConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) error {
Expand Down Expand Up @@ -108,20 +131,38 @@ func updateRepos(args []string) error {
}
uc := getUpdateReposConfig(c)

workspacePath := filepath.Join(c.RepoRoot, "WORKSPACE")
f, err := rule.LoadWorkspaceFile(workspacePath, "")
var f *rule.File
var path string
if uc.macroFileName == "" {
path = filepath.Join(c.RepoRoot, "WORKSPACE")
f, err = rule.LoadWorkspaceFile(path, "")
} else {
path = uc.macroFileName
if _, err := os.Stat(path); os.IsNotExist(err) {
f, err = rule.EmptyMacroFile(path, "", uc.macroDefName)
} else {
f, err = rule.LoadMacroFile(path, "", uc.macroDefName)
}
}
if err != nil {
return fmt.Errorf("error loading %q: %v", workspacePath, err)
return fmt.Errorf("error loading %q: %v", path, err)
}

if uc.macroFileName == "" {
merger.FixWorkspace(f)
}
merger.FixWorkspace(f)

if err := uc.fn(uc, f, kinds); err != nil {
return err
}

merger.FixLoads(f, loads)
if err := merger.CheckGazelleLoaded(f); err != nil {
return err
if uc.macroFileName == "" {
if err := merger.CheckGazelleLoaded(f); err != nil {
return err
}
}

if err := f.Save(f.Path); err != nil {
return fmt.Errorf("error writing %q: %v", f.Path, err)
}
Expand Down
4 changes: 4 additions & 0 deletions cmd/gazelle/update_repos_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestCommandLine(t *testing.T) {
c, _ := testConfig(
t,
"-from_file=Gopkg.lock",
"-to_macro=repositories.bzl%go_repositories",
"-build_file_names=BUILD",
"-build_external=external",
"-build_file_generation=on",
Expand All @@ -47,6 +48,9 @@ func TestCommandLine(t *testing.T) {
if uc.lockFilename != "Gopkg.lock" {
t.Errorf(`got from_file %q; want "Gopkg.lock"`, uc.lockFilename)
}
if uc.macroFileName != "repositories.bzl" || uc.macroDefName != "go_repositories" {
t.Errorf(`got to_macro filename:%q function:%q; want filename:repositories.bzl function:go_repositories`, uc.macroFileName, uc.macroDefName)
}
if uc.buildFileNamesAttr != "BUILD" {
t.Errorf(`got build_file_name %q; want "BUILD"`, uc.buildFileNamesAttr)
}
Expand Down
Loading