Skip to content

[bazel] Add rules for clang-fuzzer protobuf-related libraries #123126

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

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

slackito
Copy link
Collaborator

Also bumped up bazel_skylib to the latest version because the proto rules were complaining about a missing feature.

@llvmbot llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Jan 15, 2025
@slackito slackito merged commit 0e417a7 into llvm:main Jan 16, 2025
8 checks passed
@slackito slackito deleted the bazel-clang-proto-fuzzer branch January 16, 2025 18:32
@chapuni
Copy link
Contributor

chapuni commented Jan 17, 2025

Could we split out clang-proto-fuzzer stuff into the directory (for me to exclude them easily)?

For now, I have:

    "-@llvm-project//clang:clang-fuzzer-initialize",
    "-@llvm-project//clang:clang-proto-to-cxx",
    "-@llvm-project//clang:cxx_cc_proto",
    "-@llvm-project//clang:cxx-proto",
    "-@llvm-project//clang:proto-to-cxx-lib",

@slackito
Copy link
Collaborator Author

Could we split out clang-proto-fuzzer stuff into the directory (for me to exclude them easily)?

I'm not opposed to this, but if these targets are causing you problems I'd like to know more details. I tested this on a machine without protoc installed so I assumed it'd be okay.

Does it need to be a separate directory or would something like a config_setting with a custom flag work for you? I think that would be similar in spirit to the CLANG_ENABLE_PROTO_FUZZER cmake flag. Then we could have a setting with a custom --@llvm-project//clang:enable_proto_fuzzer (or something like this, I'm not super familiar with this part of bazel yet) flag, that people could add to their blazerc.

For now, I have:

    "-@llvm-project//clang:clang-fuzzer-initialize",
    "-@llvm-project//clang:clang-proto-to-cxx",
    "-@llvm-project//clang:cxx_cc_proto",
    "-@llvm-project//clang:cxx-proto",
    "-@llvm-project//clang:proto-to-cxx-lib",

Do you mean only the proto-related stuff, or everything clang-fuzzer? If it's the former, we don't need to exclude clang-fuzzer-initialize because it doesn't depend on protobufs. And if it's the latter, we'd also need to exclude/move some other recently-added targets like clang-fuzzer-dictionary and handle-cxx.

@chapuni
Copy link
Contributor

chapuni commented Jan 17, 2025

Do you mean only the proto-related stuff, or everything clang-fuzzer? If it's the former, we don't need to exclude clang-fuzzer-initialize because it doesn't depend on protobufs. And if it's the latter, we'd also need to exclude/move some other recently-added targets like clang-fuzzer-dictionary and handle-cxx.

Thanks for your considerations.
Practically the former but it'd be better able to exclude stuff controlled by CLANG_ENABLE_PROTO_FUZZER.

I am using @llvm-project for bootstrapping clang toolchain. clang-fuzzer is not required for now.
My builder has the stage for comparison of each relocatable-object.o file CMake tree and Bazel tree but I want to build much target as possible. (Note, almost all files can be made identical with my toolchain and toolchain's clang can be made identical to the final artifact clang)

I don't use the in-tree WORKSPACE. Ultimately I will have to configure protobuf but for now I would like to keep minimal configuration.

@slackito
Copy link
Collaborator Author

I think something like #123833 should do the trick.

github-merge-queue bot pushed a commit to carbon-language/carbon-lang that referenced this pull request Feb 4, 2025
- The actual reason I started this: minor lowering updates in the golden
LLVM IR
- Process.inc changed enough to need a patch context update.
- llvm/llvm-project#123126 added `proto_library`
uses without a `load`, which is broken in bazel 8
- Just commenting these out because we don't use them. I'll follow up
separately about a possible fix, but continuing to use `WORKSPACE` is a
bigger issue LLVM probably should address.
- Note this update is also triggering removal of `migrate_cpp`, in #4887
github-merge-queue bot pushed a commit to carbon-language/carbon-lang that referenced this pull request Feb 4, 2025
- The actual reason I started this: minor lowering updates in the golden
LLVM IR
- Process.inc changed enough to need a patch context update.
- llvm/llvm-project#123126 added `proto_library`
uses without a `load`, which is broken in bazel 8
- Just commenting these out because we don't use them. I'll follow up
separately about a possible fix, but continuing to use `WORKSPACE` is a
bigger issue LLVM probably should address.
- Note this update is also triggering removal of `migrate_cpp`, in #4887
github-merge-queue bot pushed a commit to carbon-language/carbon-lang that referenced this pull request Feb 4, 2025
- The actual reason I started this: minor lowering updates in the golden
LLVM IR
- Process.inc changed enough to need a patch context update.
- llvm/llvm-project#123126 added `proto_library`
uses without a `load`, which is broken in bazel 8
- Just commenting these out because we don't use them. I'll follow up
separately about a possible fix, but continuing to use `WORKSPACE` is a
bigger issue LLVM probably should address.
- Note this update is also triggering removal of `migrate_cpp`, in #4887
github-merge-queue bot pushed a commit to carbon-language/carbon-lang that referenced this pull request Feb 4, 2025
- The actual reason I started this: minor lowering updates in the golden
LLVM IR
- Process.inc changed enough to need a patch context update.
- llvm/llvm-project#123126 added `proto_library`
uses without a `load`, which is broken in bazel 8
- Just commenting these out because we don't use them. I'll follow up
separately about a possible fix, but continuing to use `WORKSPACE` is a
bigger issue LLVM probably should address.
- Note this update is also triggering removal of `migrate_cpp`, in #4887
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants