Skip to content

Implement bzl_mod compatible workaround for local_includes [IO-416] #161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

martin4861
Copy link
Contributor

The expression Label(native.repository_name() + "//:WORKSPACE") is malformed when using bzl_mod. Replacing it by Label(target_name) is compatible with both WORKSPACE and the bzl_mod.

The backwards compatibility to WORKSPACE (Bazel 6) is kept and tested as follows (see also linked PRs):

  • starling-core as bzl_mod (Bazel 7)
  • starling-core as WORKSPACE
  • starling as WORKSPACE
  • orion-engine as WORKSPACE
  • orion as WORKSPACE

@martin4861 martin4861 requested a review from Copilot July 7, 2025 11:16
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Implements a bzl_mod-compatible workaround for local include paths by passing the rule’s name into the helper instead of relying on native.repository_name().

  • Updated construct_local_include to take target_name and calculate workspace root from it.
  • Changed _construct_local_includes and all calling rules to supply kwargs.get("name") as the first argument.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
cc/utils.bzl Changed construct_local_include signature and replaced workspace root logic to use Label(target_name).
cc/defs.bzl Updated _construct_local_includes signature and modified all rule implementations to pass the rule name.
Comments suppressed due to low confidence (3)

cc/utils.bzl:22

  • The docstring parameter name does not match the function argument target_name; update the doc to refer to target_name for consistency.
        name: The target, which calls this macro

cc/utils.bzl:11

  • Add unit tests for construct_local_include to verify correct include paths are generated under both WORKSPACE and bzl_mod consumption scenarios.
def construct_local_include(target_name, path):

cc/utils.bzl:11

  • Consider validating that target_name is non-empty before constructing a Label; an empty string may lead to an invalid label and build-time errors.
def construct_local_include(target_name, path):

@martin4861 martin4861 force-pushed the martin4861/implement-bzlmod-compatible-local_include-macro branch from 648904c to 532501f Compare July 7, 2025 11:38
@martin4861 martin4861 closed this Jul 7, 2025
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.

1 participant