search

bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
22.95k stars 4.02k forks source link

Starlarkified proto_library doesn't support target names containing slashes #19458

Open illicitonion opened 12 months ago

illicitonion commented 12 months ago

Description of the bug:

This check: https://github.com/bazelbuild/bazel/blob/80235380833c61bc47dd14efb8fe1c395e17316a/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L78-L79 fails if a target name contains a / character.

This was allowed in Bazel 6, but will be broken in Bazel 7.

Which category does this issue belong to?

No response

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "rules_proto",
    sha256 = "dc3fb206a2cb3441b485eb1e423165b231235a1ea9b031b4433cf7bc1fa460dd",
    strip_prefix = "rules_proto-5.3.0-21.7",
    urls = [
        "https://artifacts.apple.com/github.com/bazelbuild/rules_proto/archive/5.3.0-21.7.tar.gz",
    ],
)

load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")

rules_proto_dependencies()
rules_proto_toolchains()

dir/BUILD.bazel:

dir/BUILD.bazel 
load("@rules_proto//proto:defs.bzl", "proto_library")

proto_library(
    name = "child/person_proto",
    srcs = ["child/person.proto"],
    strip_import_prefix = "/dir",
)

java_proto_library(
    name = "java_child",
    deps = [":child/person_proto"],
)

The following succeeds:

USE_BAZEL_VERSION=6.3.2 bazel build dir/...

The following fails:

% USE_BAZEL_VERSION=7.0.0-pre.20230823.4 bazel build dir/...
Starting local Bazel server and connecting to it...
INFO: Invocation ID: 93bff424-c310-472b-b71e-9ab196655a3f
ERROR: [redacted]/BUILD.bazel:3:14: in proto_library rule //dir:child/person_proto: 
Traceback (most recent call last):
    File "/virtual_builtins_bzl/common/proto/proto_library.bzl", line 80, column 27, in _proto_library_impl
    File "/virtual_builtins_bzl/common/proto/proto_info.bzl", line 144, column 24, in ProtoInfo
    File "/virtual_builtins_bzl/common/proto/proto_info.bzl", line 79, column 17, in _create_proto_info
Error in fail: proto_path needs to be formed like '_virtual_imports/target_name'
ERROR: [redacted]/BUILD.bazel:3:14: Analysis of target '//dir:child/person_proto' failed
ERROR: Analysis of target '//dir:child/person_proto' failed; build aborted
INFO: Elapsed time: 3.127s
INFO: 0 processes.
ERROR: Build did NOT complete successfully
Loading: 36 packages loaded
    Fetching repository @local_jdk; starting

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 7.0.0-pre.20230823.4

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

711f1b0e2496a189ed4d61ce321a91fa0f5f09a8

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

illicitonion commented 12 months ago

Ideally we'd allow these targets, but if not it would be great for proto_library to give a clear error about illegal names, rather than code failing later on in a harder to understand way.

/cc @comius