-
Notifications
You must be signed in to change notification settings - Fork 654
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
Use param files with go-protoc #3190
Conversation
@linzhp Could you try this at Uber? |
Always using the param file can cause the build to slow down because of the file IO overhead. May not be visible in small projects. However, with 100's of dependencies it can get amplified. In any case, I verified with this fix and it still fails with "argument list too long" |
@sateeshpnv Could you share logs of the failed command line, ideally with |
Firstly, thanks a lot for looking into it so quickly. It fails with
Content of the params file:
Though param file is used by go-protoc-bin, I guess the problem is - the native protoc binary is still executed with a very long argument list. Please see the #3191, which is working; not sure if that is the most elegant way, though. |
@sateeshpnv Could you give this another try? I updated it based on your PR. Instead of a hardcoded limit, it uses a param file for protoc if Bazel uses one for go-protoc. |
@fmeum , It works now. Thank you. |
@sateeshpnv Thanks for confirming. Could you close your other PR? Thanks again for the report and the fix! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because @sateeshpnv is from Uber, I believe he has tested this PR in Go monorepo :)
If Bazel uses a param file for the args to go-protoc, go-protoc will in turn use a param file for protoc.
[![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.33.0` -> `v0.34.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_go</summary> ### [`v0.34.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.34.0) [Compare Source](https://togithub.com/bazelbuild/rules_go/compare/v0.33.0...v0.34.0) #### What's Changed - releaser: fix scrubbing timestamp from patch files by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3180](https://togithub.com/bazelbuild/rules_go/pull/3180) - Replace Starlark JSON parser with json.decode by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3184](https://togithub.com/bazelbuild/rules_go/pull/3184) - gopackagesdriver: separates "s" files in pkg info by [@​iamricard](https://togithub.com/iamricard) in [https://github.com/bazelbuild/rules_go/pull/3165](https://togithub.com/bazelbuild/rules_go/pull/3165) - Refactor away references to @​io_bazel_rules_go by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3185](https://togithub.com/bazelbuild/rules_go/pull/3185) - Do not print to stderr if cgo linking succeeds after retry by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3187](https://togithub.com/bazelbuild/rules_go/pull/3187) - Use param files with go-protoc by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3190](https://togithub.com/bazelbuild/rules_go/pull/3190) - Don't include non-executable go_binary in dependent's runfiles by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3151](https://togithub.com/bazelbuild/rules_go/pull/3151) - Link in native libraries of transitive dependencies in archive mode by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3186](https://togithub.com/bazelbuild/rules_go/pull/3186) - runfiles: remove deprecated api by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3198](https://togithub.com/bazelbuild/rules_go/pull/3198) - Fix failing open hermeticity test by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3206](https://togithub.com/bazelbuild/rules_go/pull/3206) - Fix go_googleapis Gazelle patch by [@​nickgooding](https://togithub.com/nickgooding) in [https://github.com/bazelbuild/rules_go/pull/3193](https://togithub.com/bazelbuild/rules_go/pull/3193) - Exclude unsupported C/C++ features by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3189](https://togithub.com/bazelbuild/rules_go/pull/3189) - Allow gomock to take Bazel common attributes by [@​linzhp](https://togithub.com/linzhp) in [https://github.com/bazelbuild/rules_go/pull/3207](https://togithub.com/bazelbuild/rules_go/pull/3207) - Transition on edges not self by [@​illicitonion](https://togithub.com/illicitonion) in [https://github.com/bazelbuild/rules_go/pull/3116](https://togithub.com/bazelbuild/rules_go/pull/3116) - Include go_transition_test in bazel aspect by [@​ian-h-chamberlain](https://togithub.com/ian-h-chamberlain) in [https://github.com/bazelbuild/rules_go/pull/3160](https://togithub.com/bazelbuild/rules_go/pull/3160) - Add an example for go_download_sdk.sdks by [@​fishy](https://togithub.com/fishy) in [https://github.com/bazelbuild/rules_go/pull/3139](https://togithub.com/bazelbuild/rules_go/pull/3139) - tests: nogo over generated code by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3214](https://togithub.com/bazelbuild/rules_go/pull/3214) - test nogo/coverage: test generated code by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3213](https://togithub.com/bazelbuild/rules_go/pull/3213) - Remove references to go_transition_test by [@​linzhp](https://togithub.com/linzhp) in [https://github.com/bazelbuild/rules_go/pull/3215](https://togithub.com/bazelbuild/rules_go/pull/3215) - Basic bzlmod setup by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3047](https://togithub.com/bazelbuild/rules_go/pull/3047) - Run BCR tests against Bazel 6.0.0-pre.20220608.2 by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3223](https://togithub.com/bazelbuild/rules_go/pull/3223) - Use repo-relative labels in MODULE.bazel by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3226](https://togithub.com/bazelbuild/rules_go/pull/3226) - upkeep: upgrade to go 1.18.3 and gazelle v0.26.0 by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3220](https://togithub.com/bazelbuild/rules_go/pull/3220) - nogo: ignore generated source files by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3216](https://togithub.com/bazelbuild/rules_go/pull/3216) - asm: Pass package path with -p by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3231](https://togithub.com/bazelbuild/rules_go/pull/3231) - bzlmod: Add support for gomock by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3232](https://togithub.com/bazelbuild/rules_go/pull/3232) - test cgo: ensure helper script works by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3236](https://togithub.com/bazelbuild/rules_go/pull/3236) - Fix //tests/legacy/examples/cgo:cgo_lib_test on M1 Macs by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3237](https://togithub.com/bazelbuild/rules_go/pull/3237) - gopackagesdriver: Descend into go_proto_compiler's deps by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3240](https://togithub.com/bazelbuild/rules_go/pull/3240) - new_library: remove unused resolver by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3219](https://togithub.com/bazelbuild/rules_go/pull/3219) - nogo: instantiate type info for generic types when running under Go >=1.18 by [@​farhaven](https://togithub.com/farhaven) in [https://github.com/bazelbuild/rules_go/pull/3212](https://togithub.com/bazelbuild/rules_go/pull/3212) #### New Contributors - [@​iamricard](https://togithub.com/iamricard) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3165](https://togithub.com/bazelbuild/rules_go/pull/3165) - [@​ian-h-chamberlain](https://togithub.com/ian-h-chamberlain) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3160](https://togithub.com/bazelbuild/rules_go/pull/3160) - [@​fishy](https://togithub.com/fishy) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3139](https://togithub.com/bazelbuild/rules_go/pull/3139) - [@​farhaven](https://togithub.com/farhaven) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3212](https://togithub.com/bazelbuild/rules_go/pull/3212) **Full Changelog**: bazelbuild/rules_go@v0.33.0...v0.34.0 #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "16e9fca53ed6bd4ff4ad76facc9b7b651a89db1689a2877d6fd7b82aa824e366", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.34.0/rules_go-v0.34.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.34.0/rules_go-v0.34.0.zip", ], ) load("@​io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.18.4") </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 is behind base branch, 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, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-generator-go). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xMTcuNCIsInVwZGF0ZWRJblZlciI6IjMyLjExNy40In0=-->
[![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.33.0` -> `v0.34.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_go</summary> ### [`v0.34.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.34.0) [Compare Source](https://togithub.com/bazelbuild/rules_go/compare/v0.33.0...v0.34.0) #### What's Changed - releaser: fix scrubbing timestamp from patch files by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3180](https://togithub.com/bazelbuild/rules_go/pull/3180) - Replace Starlark JSON parser with json.decode by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3184](https://togithub.com/bazelbuild/rules_go/pull/3184) - gopackagesdriver: separates "s" files in pkg info by [@​iamricard](https://togithub.com/iamricard) in [https://github.com/bazelbuild/rules_go/pull/3165](https://togithub.com/bazelbuild/rules_go/pull/3165) - Refactor away references to @​io_bazel_rules_go by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3185](https://togithub.com/bazelbuild/rules_go/pull/3185) - Do not print to stderr if cgo linking succeeds after retry by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3187](https://togithub.com/bazelbuild/rules_go/pull/3187) - Use param files with go-protoc by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3190](https://togithub.com/bazelbuild/rules_go/pull/3190) - Don't include non-executable go_binary in dependent's runfiles by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3151](https://togithub.com/bazelbuild/rules_go/pull/3151) - Link in native libraries of transitive dependencies in archive mode by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3186](https://togithub.com/bazelbuild/rules_go/pull/3186) - runfiles: remove deprecated api by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3198](https://togithub.com/bazelbuild/rules_go/pull/3198) - Fix failing open hermeticity test by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3206](https://togithub.com/bazelbuild/rules_go/pull/3206) - Fix go_googleapis Gazelle patch by [@​nickgooding](https://togithub.com/nickgooding) in [https://github.com/bazelbuild/rules_go/pull/3193](https://togithub.com/bazelbuild/rules_go/pull/3193) - Exclude unsupported C/C++ features by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3189](https://togithub.com/bazelbuild/rules_go/pull/3189) - Allow gomock to take Bazel common attributes by [@​linzhp](https://togithub.com/linzhp) in [https://github.com/bazelbuild/rules_go/pull/3207](https://togithub.com/bazelbuild/rules_go/pull/3207) - Transition on edges not self by [@​illicitonion](https://togithub.com/illicitonion) in [https://github.com/bazelbuild/rules_go/pull/3116](https://togithub.com/bazelbuild/rules_go/pull/3116) - Include go_transition_test in bazel aspect by [@​ian-h-chamberlain](https://togithub.com/ian-h-chamberlain) in [https://github.com/bazelbuild/rules_go/pull/3160](https://togithub.com/bazelbuild/rules_go/pull/3160) - Add an example for go_download_sdk.sdks by [@​fishy](https://togithub.com/fishy) in [https://github.com/bazelbuild/rules_go/pull/3139](https://togithub.com/bazelbuild/rules_go/pull/3139) - tests: nogo over generated code by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3214](https://togithub.com/bazelbuild/rules_go/pull/3214) - test nogo/coverage: test generated code by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3213](https://togithub.com/bazelbuild/rules_go/pull/3213) - Remove references to go_transition_test by [@​linzhp](https://togithub.com/linzhp) in [https://github.com/bazelbuild/rules_go/pull/3215](https://togithub.com/bazelbuild/rules_go/pull/3215) - Basic bzlmod setup by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3047](https://togithub.com/bazelbuild/rules_go/pull/3047) - Run BCR tests against Bazel 6.0.0-pre.20220608.2 by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3223](https://togithub.com/bazelbuild/rules_go/pull/3223) - Use repo-relative labels in MODULE.bazel by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3226](https://togithub.com/bazelbuild/rules_go/pull/3226) - upkeep: upgrade to go 1.18.3 and gazelle v0.26.0 by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3220](https://togithub.com/bazelbuild/rules_go/pull/3220) - nogo: ignore generated source files by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3216](https://togithub.com/bazelbuild/rules_go/pull/3216) - asm: Pass package path with -p by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3231](https://togithub.com/bazelbuild/rules_go/pull/3231) - bzlmod: Add support for gomock by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3232](https://togithub.com/bazelbuild/rules_go/pull/3232) - test cgo: ensure helper script works by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3236](https://togithub.com/bazelbuild/rules_go/pull/3236) - Fix //tests/legacy/examples/cgo:cgo_lib_test on M1 Macs by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3237](https://togithub.com/bazelbuild/rules_go/pull/3237) - gopackagesdriver: Descend into go_proto_compiler's deps by [@​fmeum](https://togithub.com/fmeum) in [https://github.com/bazelbuild/rules_go/pull/3240](https://togithub.com/bazelbuild/rules_go/pull/3240) - new_library: remove unused resolver by [@​sluongng](https://togithub.com/sluongng) in [https://github.com/bazelbuild/rules_go/pull/3219](https://togithub.com/bazelbuild/rules_go/pull/3219) - nogo: instantiate type info for generic types when running under Go >=1.18 by [@​farhaven](https://togithub.com/farhaven) in [https://github.com/bazelbuild/rules_go/pull/3212](https://togithub.com/bazelbuild/rules_go/pull/3212) #### New Contributors - [@​iamricard](https://togithub.com/iamricard) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3165](https://togithub.com/bazelbuild/rules_go/pull/3165) - [@​ian-h-chamberlain](https://togithub.com/ian-h-chamberlain) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3160](https://togithub.com/bazelbuild/rules_go/pull/3160) - [@​fishy](https://togithub.com/fishy) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3139](https://togithub.com/bazelbuild/rules_go/pull/3139) - [@​farhaven](https://togithub.com/farhaven) made their first contribution in [https://github.com/bazelbuild/rules_go/pull/3212](https://togithub.com/bazelbuild/rules_go/pull/3212) **Full Changelog**: bazelbuild/rules_go@v0.33.0...v0.34.0 #### `WORKSPACE` code load("@​bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "16e9fca53ed6bd4ff4ad76facc9b7b651a89db1689a2877d6fd7b82aa824e366", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.34.0/rules_go-v0.34.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.34.0/rules_go-v0.34.0.zip", ], ) load("@​io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.18.4") </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 is behind base branch, 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, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-config-validator). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xMTcuNCIsInVwZGF0ZWRJblZlciI6IjMyLjExNy40In0=-->
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
Makes the
GoProtocGen
action use parameter files if the argument list gets too long.Which issues(s) does this PR fix?
Fixes #3188
Other notes for review
Tested manually by temporarily setting
use_always = True
and running the tests.