bazel: transition oci_image to opt/release mode for Rust#61740
Conversation
jamesmcnamara
left a comment
There was a problem hiding this comment.
Is there any way we could only add this flag if it's rust image? Obviously we could create a seperate macro like rust_oci_image but is there anything we could detect in there to only add this when needed?
unfortunately it doesnt seem like |
|
Follow up from a DM with James, potential idea for having this apply only to Rust targets transitions are able to read config_settings as inputs. So the idea could be that oci_image transitions on some internal-to-us sentinel config_setting. https://bazel.build/extending/config#attaching-transitions Its unclear to me whether this is necessary for us, but it seems like compilation_mode=opt does affect Go builds as well. See the following which depends on a config_setting based on compilation_mode. See this section for more info (tldr it toggles the equivalent of |
There was a problem hiding this comment.
nit: I couldn't easily find a single place that explains how that thing works (not the config target we're seeing here, but the whole flag), would be nice to add some context.
There was a problem hiding this comment.
Can we create a ticket to not forget to doc this once we switch to notion?
There was a problem hiding this comment.
nit: I couldn't easily find a single place that explains how that thing works (not the config target we're seeing here, but the whole flag), would be nice to add some context.
for reference: https://bazel.build/docs/user-manual#compilation-mode
|
Thanks for fixing this! We should also check Tagging @valerybugakov and @dominiccooney as you've done a lot of work in that repo. |
|
Thanks @jtibshirani . cody-engine uses zig, not bazel, to get the cross-compiling dependency management we need, and we are using |
|
@Strum355 can we merge this or is this blocked on something? |
I believe we probably can. We will have Go binaries that are unstripped with debug info (see opened issue here, no reply yet though: bazel-contrib/rules_go#3917), unlike previously fully stripped binaries. Additional work to make this only apply to Rust would be an extra time investment Im not sure is worth or not Im not sure if thats a concern at all, presumably this isnt something we've ever thought about but Id still be interested to get some thoughts from @sourcegraph/security-code-review whether this is anything to think about |
How were we getting symbolicated stack traces previously? |
Thats a separate symbol table from the ones that get stripped ( |
|
@Strum355 I don't see any high risk security concerns using unstripped binary with debug info. |
cd8cf63 to
ed8d47e
Compare
Our rust binaries (e.g. scip-ctags/syntect_server) were being built in some mix of opt & fastbuild mode[1]. Unlike with Go where there is no release/debug mode flag, Rust requires opting into optimized release builds. We can do that automagically when building any oci_image target with the power of ✨ transitions ✨ This has the side-effect of our Go binaries no longer being stripped & containing debug symbols, see bazel-contrib/rules_go#3917 Also to note, [in Cargo.toml we opt into debug symbols in release mode](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@nsc/bazel-release-mode-rust/-/blob/docker-images/syntax-highlighter/Cargo.toml?L67%3A11-70%3A9). Is this preserved by this PR for bazel[2]? [1] `strings` on the binaries showed the 3rd-party crates having `k8-opt` filepath names e.g. ``` $ / # strings syntect_server | grep k8- /tmp/bazel-working-directory/__main__/bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/crate_index__onig_sys-69.8.1/onig_sys_build_script_.runfiles/crate_index__onig_sys-69.8.1 ``` but the final binaries (and the 1st-party crates) themselves were being built in fastbuild mode. See https://github.com/sourcegraph/devx-support/issues/790 for original point of contact [2] It seems like it may be preserved, but I dont know how reliable this is for Rust binaries ``` $ file bazel-bin/docker-images/syntax-highlighter/scip-ctags bazel-bin/docker-images/syntax-highlighter/scip-ctags: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.0.0, with debug_info, not stripped ``` ## Test plan Tested for sourcegraph/scip-ctags image: ``` / $ strings scip-ctags | grep "Could not parse file" / $ echo $? 1 ``` (cherry picked from commit ce6a366)
…or Rust (#62049) bazel: transition oci_image to opt/release mode for Rust (#61740) Our rust binaries (e.g. scip-ctags/syntect_server) were being built in some mix of opt & fastbuild mode[1]. Unlike with Go where there is no release/debug mode flag, Rust requires opting into optimized release builds. We can do that automagically when building any oci_image target with the power of ✨ transitions ✨ This has the side-effect of our Go binaries no longer being stripped & containing debug symbols, see bazel-contrib/rules_go#3917 Also to note, [in Cargo.toml we opt into debug symbols in release mode](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@nsc/bazel-release-mode-rust/-/blob/docker-images/syntax-highlighter/Cargo.toml?L67%3A11-70%3A9). Is this preserved by this PR for bazel[2]? [1] `strings` on the binaries showed the 3rd-party crates having `k8-opt` filepath names e.g. ``` $ / # strings syntect_server | grep k8- /tmp/bazel-working-directory/__main__/bazel-out/k8-opt-exec-ST-13d3ddad9198/bin/external/crate_index__onig_sys-69.8.1/onig_sys_build_script_.runfiles/crate_index__onig_sys-69.8.1 ``` but the final binaries (and the 1st-party crates) themselves were being built in fastbuild mode. See sourcegraph/devx-support#790 for original point of contact [2] It seems like it may be preserved, but I dont know how reliable this is for Rust binaries ``` $ file bazel-bin/docker-images/syntax-highlighter/scip-ctags bazel-bin/docker-images/syntax-highlighter/scip-ctags: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.0.0, with debug_info, not stripped ``` ## Test plan Tested for sourcegraph/scip-ctags image: ``` / $ strings scip-ctags | grep "Could not parse file" / $ echo $? 1 ``` (cherry picked from commit ce6a366) Co-authored-by: Noah S-C <noah@sourcegraph.com>
Our rust binaries (e.g. scip-ctags/syntect_server) were being built in some mix of opt & fastbuild mode[1]. Unlike with Go where there is no release/debug mode flag, Rust requires opting into optimized release builds. We can do that automagically when building any oci_image target with the power of ✨ transitions ✨
This has the side-effect of our Go binaries no longer being stripped & containing debug symbols, see bazel-contrib/rules_go#3917
Also to note, in Cargo.toml we opt into debug symbols in release mode. Is this preserved by this PR for bazel[2]?
[1]
stringson the binaries showed the 3rd-party crates havingk8-optfilepath names e.g.but the final binaries (and the 1st-party crates) themselves were being built in fastbuild mode. See https://github.com/sourcegraph/devx-support/issues/790 for original point of contact
[2] It seems like it may be preserved, but I dont know how reliable this is for Rust binaries
Test plan
Tested for sourcegraph/scip-ctags image: