Skip to content

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