Closed jayconrod closed 4 years ago
at
//foo/bar
should be namedfoo
I think you meant to say: at //foo/bar
should be named bar
.
And cannot wait for this change to finally land.
Correct. I've edited the original issue.
Just wonder if there's any ETA when this would be fixed. Thanks!
No ETA, sorry.
This would need a major migration plan to become effective, right?
@riking Yes
Is there an active design document for this, or should one be drafted? Is there a consensus? I've read a few different assertions on how things should work that all seem to slightly contradict each other.
I'm rolling out gazelle to our monorepo, and the "go_default_library" everywhere can be a little grating on the eye.
Sorry, this is annoying. I'm afraid there's no active plan to make this happen at the moment though. Other functional issues (editor support, coverage, proto support) still seem more important.
The problem is migration. A change to all rules will be really disruptive for large projects (and people have objected loudly to disruptions of this scale in the past). So this definitely needs to be opt-in for existing projects. At the same time, the new names should be used by default for new projects.
This is also difficult for repositories that are used as dependencies in other repositories. If they want to switch to the new names, they'd need to have aliases with the old names to avoid breaking their dependents. Gazelle should be able to generate those names.
Would it be possible to draft the design without executing on it? I can create a design doc if you want. That way we know where we are headed. What I'd need is an initial summary of what you would like to see, plus a list of considerations similar to the above message.
We have our own little monorepo over here, and I am already half inclined to hack gazelle (which I tried yesterday for verification purposes) to produce better names before I start enforcing it via a glaze-style presubmit. It would be nice to know that the solution is compatible with the desired end state, and if desired I could contribute it behind said opt-in flag.
Here's a sketch of how I think this should work. I'd rather not review a more detailed design doc right now though.
//language/go
extension should accept a directive, # gazelle:go_naming_convention
, that supports the values:
go_default_library
- old style, what Gazelle currently does.import
- new style.foo
, the go_library
would be named foo
. The go_test
would be named foo_test
.go_library
would be named foo_lib
, the go_test
would be named foo_test
, and the go_binary
would be named foo
.example.com/foo/v2
would be named foo
, not v2
.import_alias
- same as import
but additionally generates alias
rules mapping go_default_library
targets to import
targets.//language/go
should attempt to automatically determine the naming convention currently in use by scanning build files one or two directories deep from the root. If it doesn't find any go_library
rules conforming to one style or the other (as will be the case for a repository without any build files), it should use the import
style.Fix
method should rename rules as needed between the two styles. That should probably happen whether or not c.ShouldFix
is set, since there's explicit user intent here.go_repository
should have a build_naming_convention
attribute, used to set this directive. It should default to import_alias
.build_naming_convention
attribute for known repositories. If the repository is unknown or if the attribute isn't set explicitly, it should follow the convention currently in use.EDIT: These questions were pretty much answered during implementation. Ignore the below.
For import
mode, what happens when the bazel package name and importpath differ?
Eg. package '//foo' with importpath 'bar'.
Should the go_library
be called foo
or bar
? (Similar question for the go_binary
)
@linzhp @blico Just wanted to give you a heads up that @tomlu is working on a fix for this issue (#808).
The plan we're following is basically this comment above. Please let me know if there's something we're not anticipating though.
Originally bazelbuild/rules_go#265
We used
go_default_library
as the name forgo_library
rules generated by Gazelle to simplify dependency resolution. Both the Go rules and Gazelle relied on this, along withgo_prefix
.Now,
go_prefix
is deprecated and Gazelle addsimportpath
attributes to all Go rules. It also uses an index for dependency resolution instead of transforming import path strings. There's no longer a technical need forgo_default_library
, so we'd like to migrate away from that name.In general, we'd like the library name to match the last component of the Bazel package. For example, a library stored at
//foo/bar
should be namedbar
, so it can be compiled withbazel build //foo/bar
instead ofbazel build //foo/bar:go_default_library
. Tests would be namedbar_test
orbar_xtest
.We may want this to work differently in packages with a
go_binary
(sources havepackage main
). For these packages, thego_binary
name should match the package (again, so it can be compiled with//foo/bar
). The library will need a different name. We may want to put sources and dependencies ingo_binary
instead of embedding a library if there are no tests.