googleapis icon indicating copy to clipboard operation
googleapis copied to clipboard

feat(bazel): make googleapis a Bzlmod module

Open tyler-french opened this issue 1 year ago • 11 comments

This PR adds the googleapis repo to the Bazel central registry as a Bazel Module.

tyler-french avatar Sep 27 '23 13:09 tyler-french

@fmeum Could you take a look? It's a bit difficult because this package doesn't have versioned releases

tyler-french avatar Nov 07 '23 15:11 tyler-french

Hey everyone,

I'm interested in helping with the transition of googleapis to bzlmod support. While reviewing the dependency graph, I noticed a key decision point: should we opt for pure bzlmod support, or consider a partial approach using use_repo?

Adopting a purely bzlmod-based approach means that all gapic dependencies would also need to support bzlmod. This, in turn, hinges on grpc being a bzlmod project. However, it appears that grpc requires a substantial patch file to support bzlmod, which could complicate matters.

What are the team's thoughts on this? Are there any plans already in place for tackling these dependencies?

silicon-ninja avatar Dec 05 '23 05:12 silicon-ninja

Hi @tyler-french, @fmeum, and @silicon-ninja,

Thank you for your suggestions! We indeed cannot immediately switch to bzlmod: gRPC dependency makes it somewhat complicated, but that's not the only issue. For now, we will keep using Bazel 6 (via Bazelisk) and WORKSPACE for some more time, while we think internally how we can switch. We definitely should figure out how to switch soon because of Bazel's plans to deprecate / remove WORKSPACE support.

Please note that we are unable to merge pull requests here, because all Bazel configuration (and protos) in this repository is auto-synced from the internal code storage. I will leave this PR open for discussions, but the code change will come from the regular sync process, so this PR will not get merged.

Thank you!

alexander-fenster avatar Dec 13 '23 20:12 alexander-fenster

Hi @alexander-fenster, isn't it still possible to support both in a soft migration? See https://bazel.build/external/migration#workspace.bzlmod - a WORKSPACE.bzlmod file that makes Bazel with bzlmod ignore the WORKSPACE file while Bazel without bzlmod will ignore WORKSPACE.bzlmod and MODULE.bazel

arnehormann avatar Dec 14 '23 13:12 arnehormann

See https://github.com/bazelbuild/bazel-central-registry/issues/1113

Note that the migration of core ecosystem infrastructure such as this blocks downstream migration. I appreciate in this case that it's in fact upstream dependencies that may be blocking this one, e.g. protobuf and grpc.

My project just uses googleapis for proto definitions in the google.type and google.api packages, so I was able to patch in just enough support in my own Bazel registry.

SanjayVas avatar Dec 14 '23 18:12 SanjayVas

Perhaps the global common component types should be split out from googleapis.

SanjayVas avatar Dec 14 '23 20:12 SanjayVas

@SanjayVas We actually have https://github.com/googleapis/api-common-protos for this specific purpose, I'll see if we can actually update that one first and see if it helps.

In general, folks, please give us some time and we'll figure this out - I just want to set some expectations that it likely won't happen until the new year. There are a lot of moving parts involved (much more than I would like to have :) ), but we are on it.

alexander-fenster avatar Dec 15 '23 00:12 alexander-fenster

@SanjayVas We actually have https://github.com/googleapis/api-common-protos for this specific purpose, I'll see if we can actually update that one first and see if it helps.

I am aware, but that often lags way behind the main googleapis repo. It's not automatically kept up-to-date. I imagine that's why the AIPs point to googleapis as the canonical location and why lots of core Bazel infra (e.g. rules_go) depends on it.

SanjayVas avatar Dec 15 '23 02:12 SanjayVas

@SanjayVas We actually have https://github.com/googleapis/api-common-protos for this specific purpose, I'll see if we can actually update that one first and see if it helps.

In general, folks, please give us some time and we'll figure this out - I just want to set some expectations that it likely won't happen until the new year. There are a lot of moving parts involved (much more than I would like to have :) ), but we are on it.

Hi @alexander-fenster, I hope it's okay to ping you after 2.5 months. As it's the new year now, I'd love to get an update if you could please provide one. Even if it's just a rough plan. This issue (here and in grpc / grpc-java) blocks migrations to Bazel 7 and locks us into legacy software and into some maintenance heavy workarounds. I'd really love to get rid of them without additional manual patches.

ah-quant avatar Mar 04 '24 12:03 ah-quant

Explain. Ihave no clue but definitely interested

On Tue, Mar 26, 2024, 3:48 PM Keith Smiley @.***> wrote:

@.**** commented on this pull request.

t doesn't have a name, so I assume you meant module here

In extensions.bzl https://github.com/googleapis/googleapis/pull/855#discussion_r1540090068 :

  •    "ruby": attr.bool(default = False),
    
  • }, +)

+def _switched_rules_impl(ctx):

  • attrs = {}
  • for module in ctx.modules:
  •    if not module.is_root:
    
  •        continue
    
  •    is_tag_set = False
    
  •    set_tag_name = ""
    
  •    for t in module.tags.use_languages:
    
  •        if is_tag_set:
    
  •            fail("Multiple use_language tags are set in the root module: '{}' and '{}'. Only one is allowed.".format(set_tag_name, t.name))
    

⬇️ Suggested change

  •            fail("Multiple use_language tags are set in the root module: '{}' and '{}'. Only one is allowed.".format(set_tag_name, t.name))
    
  •            fail("Multiple use_language tags are set in the root module: '{}' and '{}'. Only one is allowed.".format(set_tag_name, module.name))
    

In extensions.bzl https://github.com/googleapis/googleapis/pull/855#discussion_r1540090278 :

+def _switched_rules_impl(ctx):

  • attrs = {}
  • for module in ctx.modules:
  •    if not module.is_root:
    
  •        continue
    
  •    is_tag_set = False
    
  •    set_tag_name = ""
    
  •    for t in module.tags.use_languages:
    
  •        if is_tag_set:
    
  •            fail("Multiple use_language tags are set in the root module: '{}' and '{}'. Only one is allowed.".format(set_tag_name, t.name))
    
  •        is_tag_set = True
    
  •        set_tag_name = t.name
    

⬇️ Suggested change

  •        set_tag_name = t.name
    
  •        set_tag_name = module.name
    

— Reply to this email directly, view it on GitHub https://github.com/googleapis/googleapis/pull/855#pullrequestreview-1961700501, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7YUO6N7YVTI6C72DDHXLELY2HNJTAVCNFSM6AAAAAA5JM2EPWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSNRRG4YDANJQGE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

SeeYaaG avatar Mar 26 '24 23:03 SeeYaaG

i stacked some commits on this over here https://github.com/googleapis/googleapis/pull/892

keith avatar Mar 27 '24 21:03 keith