bazel-skylib icon indicating copy to clipboard operation
bazel-skylib copied to clipboard

Move Gazelle extension to //gazelle/bzl and change package name

Open jayconrod opened this issue 5 years ago • 5 comments

This fixes an issue with importing bazel-skylib into google3. Currently, Glaze (internal Go build file generator) attempts to generate a target (//gazelle:gazelle) that conflicts with one that's already declared here.

I think the right solution is actually to move the package into a subdirectory. In the future (bazelbuild/bazel-gazelle#5), Gazelle's Go extension will generate target names similar to what Glaze does, so the same conflict will happen in open source. I think it's also logical to have a directory of packages in case more need to be added in the future, and for the extension to have a package name matching the language it works with.

This is an incompatible change, but the //gazelle directory isn't part of a tagged release yet, so hopefully it won't break anyone.

jayconrod avatar Aug 04 '20 21:08 jayconrod

cc @juliexxia

If moving the package isn't an option, renaming //gazelle:gazelle would be a narrower fix.

jayconrod avatar Aug 04 '20 21:08 jayconrod

@achew22 Good idea. I've started bazelbuild/bazel-gazelle#857 for that, but I can't finish it until this one is merged.

jayconrod avatar Aug 05 '20 21:08 jayconrod

@jayconrod, feel free to merge this if you're confident that it'll work. You might try copybara importing the PR before merging though, just in case TAP hate you 😉 .

You've got all the permissions you need to merge.

achew22 avatar Aug 06 '20 04:08 achew22

I don't have write access in this repo. Someone else will need to hit the button.

@juliexxia Could you confirm this will work with the Copybara import? I'm not sure if it's possible to import a PR rather than the main branch; if not, we can merge, see what goes wrong, and fix it in a follow-up.

jayconrod avatar Aug 06 '20 13:08 jayconrod

I believe there's a way to import a single PR and runs tests. I'll do that now.

juliexxia avatar Aug 06 '20 15:08 juliexxia