Skip to content
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

Add support for --incompatible_enable_proto_toolchain_resolution #3919

Merged
merged 12 commits into from
May 13, 2024

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Apr 12, 2024

What type of PR is this?

Feature

What does this PR do? Why is it needed?

With --incompatible_enable_proto_toolchain_resolution, users of protoc are expected to obtain it through @rules_proto//proto:toolchain_type instead of the Bazel command line flag --proto_compiler. This change adds support for this in a backwards compatible way.

Note that the above is actually a lie: The proper way to use protoc would be to go through a Go-specific proto_lang_toolchain and use the methods on proto_common to interact with protoc. Since rules_go has a very bespoke setup with customizable compilers and the need to apply reset transitions in case protoc is built from source, this would require major changes. We should revisit this after Proto toolchainization has been enabled by default.

Which issues(s) does this PR fix?

Fixes #3895

Other notes for review

@fmeum
Copy link
Collaborator Author

fmeum commented Apr 12, 2024

@alexeagle None of us have worked with proto toolchains so far, so it would be great if you could also take a look.

@fmeum fmeum force-pushed the 3895-proto branch 7 times, most recently from 57bee2d to 69df93d Compare April 12, 2024 11:04
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 12, 2024

FYI @sluongng You might find this interesting :-)

Copy link
Contributor

@sluongng sluongng left a comment

Choose a reason for hiding this comment

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

A few questions, mostly because I am not that familiar with the new flag implication.

proto/compiler.bzl Show resolved Hide resolved
WORKSPACE Outdated Show resolved Hide resolved
tests/bcr/MODULE.bazel Show resolved Hide resolved
proto/compiler.bzl Show resolved Hide resolved
WORKSPACE Show resolved Hide resolved
@fmeum
Copy link
Collaborator Author

fmeum commented Apr 12, 2024

I'll take another stab at fixing the integration tests next week. Everything else is stable for review.

@fmeum
Copy link
Collaborator Author

fmeum commented May 7, 2024

I updated rules_proto to 6.0.0 and fixed the remaining tests, PTAL @linzhp @tyler-french

@alexeagle
Copy link
Contributor

@sluongng FYI looks like this PR will merge as soon as you click "Resolve" on the two open threads; are they answered to your satisfaction?

@sluongng
Copy link
Contributor

Hmm i cannot resolve the conversations. Perhaps @fmeum can.

The PR LGTM. I hope the legacy toolchain still provide an alternative option to use protoc from source.

Cheers!

@fmeum fmeum merged commit a54fd56 into master May 13, 2024
5 checks passed
@fmeum fmeum deleted the 3895-proto branch May 13, 2024 20:04
tyler-french added a commit that referenced this pull request May 21, 2024
This PR prepares a minor release to allow downstream users to begin integrating with #3919
cgrindel-self-hosted-renovate bot added a commit to cgrindel/bazel-starlib that referenced this pull request May 22, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) |
http_archive | minor | `v0.47.1` -> `v0.48.0` |

---

### Release Notes

<details>
<summary>bazelbuild/rules_go (io_bazel_rules_go)</summary>

###
[`v0.48.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.48.0)

[Compare
Source](https://togithub.com/bazelbuild/rules_go/compare/v0.47.1...v0.48.0)

#### Important Changes!

`--incompatible_enable_proto_toolchain_resolution` is now supported in
`rules_go`. This means that protoc should now be supplied as a
toolchain. `protoc` can be registered using
https://github.com/aspect-build/toolchains_protoc, or a local proto
toolchain can be added.

#### `WORKSPACE` code

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

    http_archive(
        name = "io_bazel_rules_go",
sha256 =
"33acc4ae0f70502db4b893c9fc1dd7a9bf998c23e7ff2c4517741d4049a976f8",
        urls = [

"https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.48.0/rules_go-v0.48.0.zip",

"https://github.com/bazelbuild/rules_go/releases/download/v0.48.0/rules_go-v0.48.0.zip",
        ],
    )

load("@&#8203;io_bazel_rules_go//go:deps.bzl", "go_register_toolchains",
"go_rules_dependencies")

    go_rules_dependencies()

    go_register_toolchains(version = "1.22.3")

#### What's Changed

- Update docs to cover new FilePath ReplaceDirective Support by
[@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[bazelbuild/rules_go#3931
- go_test: use different mnemonic for compilation by
[@&#8203;sluongng](https://togithub.com/sluongng) in
[bazelbuild/rules_go#3936
- Add go.work support to the documentation by
[@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[bazelbuild/rules_go#3932
- feat(mode): add `purego` tag when `pure` by
[@&#8203;mattyclarkson](https://togithub.com/mattyclarkson) in
[bazelbuild/rules_go#3901
- generate_test_main: Move timeout handling back to bzltestutil by
[@&#8203;DolceTriade](https://togithub.com/DolceTriade) in
[bazelbuild/rules_go#3939
- Add support for `--incompatible_enable_proto_toolchain_resolution` by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/rules_go#3919
- Add exec_compatible_with to @&#8203;go_sdk//:builder by
[@&#8203;EdSchouten](https://togithub.com/EdSchouten) in
[bazelbuild/rules_go#3943
- prepare v0.48.0 release by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/rules_go#3946

#### New Contributors

- [@&#8203;stefanpenner](https://togithub.com/stefanpenner) made their
first contribution in
[bazelbuild/rules_go#3931

**Full Changelog**:
bazelbuild/rules_go@v0.47.1...v0.48.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDkuNCIsInVwZGF0ZWRJblZlciI6IjM2LjEwOS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
renovate bot added a commit to kreempuff/rules_unreal_engine that referenced this pull request May 22, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) |
http_archive | minor | `v0.47.1` -> `v0.48.0` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>bazelbuild/rules_go (io_bazel_rules_go)</summary>

###
[`v0.48.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.48.0)

[Compare
Source](https://togithub.com/bazelbuild/rules_go/compare/v0.47.1...v0.48.0)

#### Important Changes!

`--incompatible_enable_proto_toolchain_resolution` is now supported in
`rules_go`. This means that protoc should now be supplied as a
toolchain. `protoc` can be registered using
https://github.com/aspect-build/toolchains_protoc, or a local proto
toolchain can be added.

#### `WORKSPACE` code

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

    http_archive(
        name = "io_bazel_rules_go",
sha256 =
"33acc4ae0f70502db4b893c9fc1dd7a9bf998c23e7ff2c4517741d4049a976f8",
        urls = [

"https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.48.0/rules_go-v0.48.0.zip",

"https://github.com/bazelbuild/rules_go/releases/download/v0.48.0/rules_go-v0.48.0.zip",
        ],
    )

load("@&#8203;io_bazel_rules_go//go:deps.bzl", "go_register_toolchains",
"go_rules_dependencies")

    go_rules_dependencies()

    go_register_toolchains(version = "1.22.3")

#### What's Changed

- Update docs to cover new FilePath ReplaceDirective Support by
[@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[bazelbuild/rules_go#3931
- go_test: use different mnemonic for compilation by
[@&#8203;sluongng](https://togithub.com/sluongng) in
[bazelbuild/rules_go#3936
- Add go.work support to the documentation by
[@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[bazelbuild/rules_go#3932
- feat(mode): add `purego` tag when `pure` by
[@&#8203;mattyclarkson](https://togithub.com/mattyclarkson) in
[bazelbuild/rules_go#3901
- generate_test_main: Move timeout handling back to bzltestutil by
[@&#8203;DolceTriade](https://togithub.com/DolceTriade) in
[bazelbuild/rules_go#3939
- Add support for `--incompatible_enable_proto_toolchain_resolution` by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/rules_go#3919
- Add exec_compatible_with to @&#8203;go_sdk//:builder by
[@&#8203;EdSchouten](https://togithub.com/EdSchouten) in
[bazelbuild/rules_go#3943
- prepare v0.48.0 release by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/rules_go#3946

#### New Contributors

- [@&#8203;stefanpenner](https://togithub.com/stefanpenner) made their
first contribution in
[bazelbuild/rules_go#3931

**Full Changelog**:
bazelbuild/rules_go@v0.47.1...v0.48.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/kreempuff/rules_unreal_engine).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbXX0=-->
fmeum pushed a commit to bazel-contrib/toolchains_llvm that referenced this pull request May 22, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) |
http_archive | minor | `v0.47.1` -> `v0.48.0` |

---

### Release Notes

<details>
<summary>bazelbuild/rules_go (io_bazel_rules_go)</summary>

###
[`v0.48.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.48.0)

[Compare
Source](https://togithub.com/bazelbuild/rules_go/compare/v0.47.1...v0.48.0)

#### Important Changes!

`--incompatible_enable_proto_toolchain_resolution` is now supported in
`rules_go`. This means that protoc should now be supplied as a
toolchain. `protoc` can be registered using
https://github.com/aspect-build/toolchains_protoc, or a local proto
toolchain can be added.

#### `WORKSPACE` code

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

    http_archive(
        name = "io_bazel_rules_go",
sha256 =
"33acc4ae0f70502db4b893c9fc1dd7a9bf998c23e7ff2c4517741d4049a976f8",
        urls = [

"https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.48.0/rules_go-v0.48.0.zip",

"https://github.com/bazelbuild/rules_go/releases/download/v0.48.0/rules_go-v0.48.0.zip",
        ],
    )

load("@&#8203;io_bazel_rules_go//go:deps.bzl", "go_register_toolchains",
"go_rules_dependencies")

    go_rules_dependencies()

    go_register_toolchains(version = "1.22.3")

#### What's Changed

- Update docs to cover new FilePath ReplaceDirective Support by
[@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[bazelbuild/rules_go#3931
- go_test: use different mnemonic for compilation by
[@&#8203;sluongng](https://togithub.com/sluongng) in
[bazelbuild/rules_go#3936
- Add go.work support to the documentation by
[@&#8203;stefanpenner](https://togithub.com/stefanpenner) in
[bazelbuild/rules_go#3932
- feat(mode): add `purego` tag when `pure` by
[@&#8203;mattyclarkson](https://togithub.com/mattyclarkson) in
[bazelbuild/rules_go#3901
- generate_test_main: Move timeout handling back to bzltestutil by
[@&#8203;DolceTriade](https://togithub.com/DolceTriade) in
[bazelbuild/rules_go#3939
- Add support for `--incompatible_enable_proto_toolchain_resolution` by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/rules_go#3919
- Add exec_compatible_with to @&#8203;go_sdk//:builder by
[@&#8203;EdSchouten](https://togithub.com/EdSchouten) in
[bazelbuild/rules_go#3943
- prepare v0.48.0 release by
[@&#8203;tyler-french](https://togithub.com/tyler-french) in
[bazelbuild/rules_go#3946

#### New Contributors

- [@&#8203;stefanpenner](https://togithub.com/stefanpenner) made their
first contribution in
[bazelbuild/rules_go#3931

**Full Changelog**:
bazelbuild/rules_go@v0.47.1...v0.48.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/bazel-contrib/toolchains_llvm).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ0YXJnZXRCcmFuY2giOiJtYXN0ZXIiLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

Support --incompatible_enable_proto_toolchain_resolution
4 participants