Skip to content

Conversation

@mmorel-35
Copy link
Collaborator

@mmorel-35 mmorel-35 commented Dec 18, 2025

Fix arch_alias compatibility with bzlmod

clang_platform is not compatible with bzlmod: https://github.com/envoyproxy/envoy/actions/runs/20279190616/job/58236274568

Analysis

The arch_alias repository rule in bazel/repository/utils.bzl was created to enable architecture-specific platform aliases for Envoy. It works by:

  • Creating an external repository with a BUILD file containing an alias
  • Allowing .bazelrc to reference platform aliases like @clang_platform
  • Resolving to different platforms based on host CPU architecture (amd64/aarch64)

Problem

The current implementation is WORKSPACE-only and doesn't follow bzlmod best practices:

  • Repository rules in bzlmod need module extensions
  • Should use proper tag classes for configuration
  • Current approach doesn't work when --enable_bzlmod is set

Solution Implemented

  • Analyze the arch_alias implementation and usage
  • Create a module extension wrapper for arch_alias
  • Add bzlmod-compatible extension definition with tag_class
  • Fix alias naming in bzlmod to extract apparent name from canonical name
  • Update documentation with bzlmod usage examples
  • Create BCR-compatible test modules following best practices
  • Organize tests properly: group under arch_alias/ and remove redundant _test suffix
  • Remove unused alias_name parameter - the logic automatically handles bzlmod canonical names
  • Add tests to GitHub workflow for both bzlmod and workspace modes
  • Add test module to BCR presubmit configuration
  • Remove module() declaration from test module (not intended for publication)
  • Test both WORKSPACE and bzlmod modes
  • Verify backward compatibility
  • Address all code review feedback

Changes Made

  1. Enhanced utils.bzl - Added arch_alias_ext module extension with proper tag_class
  2. Automatic Name Extraction - The _get_alias_name() helper automatically extracts apparent names from bzlmod's canonical format (module++ext+namename)
  3. Removed Redundant Parameter - Removed alias_name parameter as it was unnecessary
  4. Improved Documentation - Added inline documentation explaining bzlmod canonical vs apparent names
  5. Well-Organized Test Structure:
    • bazel/test/arch_alias/bzlmod/ - BCR-compatible bzlmod test module
    • bazel/test/arch_alias/workspace/ - WORKSPACE mode backward compatibility tests
    • Tests grouped under common directory, no redundant _test suffix
    • Comprehensive README files documenting structure and usage
  6. CI Integration:
    • Added arch_alias tests to GitHub workflow (.github/workflows/_bazel.yml) for both bzlmod and workspace modes
    • Added test_arch_alias task to BCR presubmit configuration (.bcr/bazel/presubmit.yml)
    • Removed module() declaration from test MODULE.bazel as it's not intended for publication

Testing

All tests pass in both bzlmod and WORKSPACE modes, verifying multiple aliases work correctly. Tests now run automatically in CI/CD pipelines on every PR and in BCR presubmit checks.

@netlify
Copy link

netlify bot commented Dec 18, 2025

Deploy Preview for nifty-bassi-e26446 ready!

Name Link
🔨 Latest commit 7529bd1
🔍 Latest deploy log https://app.netlify.com/projects/nifty-bassi-e26446/deploys/6943ab64ace6990008b79ed4
😎 Deploy Preview https://deploy-preview-3131--nifty-bassi-e26446.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for working on this @mmorel-35

can you separate all the test changes out to a different pr please - lets make the change and then consider if/where to test as an external repo

(and remove all the bot docs - not against adding docs but they need to be minimal, focused and targeted and easy to maintain)

@@ -0,0 +1,11 @@
# Bazel configuration for arch_alias extension test module
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test files here seem misplaced - altho i understand these tests need a separate workspace to use toolshed externally, which i think is not a bad idea - but then it should be testing non-bzlmod also

"amd64": "@platforms//host",
"aarch64": "@platforms//host",
"x86_64": "@platforms//host",
"arm64": "@platforms//host",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure iiuc but what is the point of the test if they all return the same thing

@@ -0,0 +1,32 @@
# arch_alias Tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please dont keep submitting voluminous bot docs - it overwhelms reviewers and detracts from the actual changes

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