Skip to content

bazel: resolve benchmark deps for downstream bzlmod consumers - #47553

Closed
phlax with Copilot wants to merge 3 commits into
mainfrom
copilot/update-google-benchmark-dependency
Closed

phlax with Copilot wants to merge 3 commits into
mainfrom
copilot/update-google-benchmark-dependency

Conversation

Copilot AI commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Envoy’s benchmark macros were resolving @benchmark inside Envoy’s repo mapping, which breaks for downstream bzlmod consumers because google_benchmark remains a dev_dependency and is absent when Envoy is not the root module. This change keeps the dependency dev-only in Envoy, adds an external regression target, and shifts benchmark resolution to the caller’s module mapping.

  • Why this change

    • Downstream modules using envoy_cc_benchmark_binary could not build benchmarks unless Envoy promoted google_benchmark to a non-dev dep.
    • We do not want that; downstreams that build benchmarks should declare google_benchmark themselves.
  • External regression coverage

    • Added a minimal benchmark under bazel/tests/external/ that consumes Envoy as an external module.
    • Declared google_benchmark in the external test module and added the new benchmark target to the external CI build path.
    • This provides a concrete yardstick for the downstream bzlmod use case reported in build: remove dev_dependency = true from google_benchmark #47497.
  • Benchmark target split

    • Split //test/benchmark:main into:
      • main_lib: Envoy-owned support deps only
      • main: the in-tree benchmark target that still links @benchmark and @tclap
    • Exported main.cc so the benchmark main can be compiled from the caller context.
  • Macro behavior change

    • Reworked envoy_cc_benchmark_binary and envoy_cc_benchmark_dyn_module_binary to:
      • remove the misleading repository parameter
      • add a srcs parameter and append Label("//test/benchmark:main.cc")
      • depend on Label("//test/benchmark:main_lib")
      • pass @benchmark and @tclap as plain string labels so they resolve in the caller’s repo mapping
    • Added small deduping logic so existing in-tree benchmark targets that already list @benchmark do not pick it up twice.
  • Downstream contract

    • Documented that downstream bzlmod users of envoy_cc_benchmark_binary must declare:
      bazel_dep(name = "google_benchmark", version = "1.9.5", repo_name = "benchmark")
  • Example

    envoy_cc_benchmark_binary(
        name = "benchmark_test",
        srcs = ["benchmark_test.cc"],
    )

    with the downstream module declaring:

    bazel_dep(name = "google_benchmark", version = "1.9.5", repo_name = "benchmark")

@repokitteh-read-only

Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #47553 was opened by Copilot.

see: more, trace.

Copilot AI and others added 2 commits September 20, 2026 11:39
Signed-off-by: GitHub <noreply@github.com>

Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Signed-off-by: GitHub <noreply@github.com>

Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix benchmark dependency resolution for downstream repos bazel: resolve benchmark deps for downstream bzlmod consumers Sep 20, 2026
Copilot AI requested a review from phlax September 20, 2026 11:43
@phlax phlax closed this Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants