bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
390 stars 178 forks source link

Potential conflict between Google3 and external naming conventions for `bzl_library` #284

Open achew22 opened 3 years ago

achew22 commented 3 years ago

Hey @aiuto, @jin, @c-parsons, CC @jayconrod,

I have been going through getting bzl_library targets added to a bunch of repos and I had a realization about it. I think there is someone @ google who is doing the same thing from inside G3. I was trying to figure out why https://github.com/bazelbuild/rules_cc/commit/991eb349bf648b89f7d039a2602d337c85479534 creates a bunch of targets that have //visibility:private. That's a super silly thing to do, but I think I figured out why.

I think that this is stage 1 of a multi part LSC to add bzl_libraries and I'm a little worried that the gazelle generator in this repo is incompatible with it. The LSC seems to be going with names that end in _bzl while I decided to strip the suffix. Is there any way I could have someone look at http://cl/340301767 (referenced in that commit) and tell me if bazel-skylib's automation is incompatible with the automation I suspect is being written.

If it is, what do you think the correct course of action should be here? My inclination is to update the Gazelle generator to match their _bzl suffix and manually fix all the people who've adopted so far and apologize.

What do you think about that? My email is attached to a bunch of commits in this repo if you would like to have this conversation in a less open medium.

Thanks so much!

jin commented 3 years ago

Sorry, I have don't have much context around this, I've pinged the author of the internal bzl_library cleanup.

murali42 commented 3 years ago

As you suspected the private visibility is because this is part of a series of changes. We start with absolute minimal visibility and then start opening up as we find it being used. .bzl files meant to be used widely would be manually flipped to //visibility:public.

Regarding naming convention: We just happened to pick a convention similar to conventions we use for proto_library rules. No deep thinking behind it. Since different code bases may have their own conventions, expecting every one to follow the exact same conventions is going to be hard. For your specific issue, I would defer to gazelle and owners of rules_cc repository. Internal tooling we use doesn't care about any naming conventions, so will be able to handle any name.

achew22 commented 3 years ago

@murali42, thanks for your response.

This repo is where the gazelle plugin that generates bzl_library targets lives. I've had robots fight before, and never really enjoyed it. If you're telling me that the generator that preexists and that you're at least part-way into a multi-part LSC then, since there is relatively little adoption of this Gazelle plugin, I can pretty easily fix things in here and send a PR to fix the small number of repos who already have adopted.

[insert robots fighting meme]

Could you briefly write out the rules (copy/paste from the LSC doc probably) that you're using to determine what targets should be generated so I can reimplement it in the Gazelle plugin and we don't have to worry about this going forward?

Thank you so much!

murali42 commented 3 years ago

If gazelle is going to changed, the rules would be like