carverauto/serviceradar

Fork 0

wip linker changes #2835

Merged

mfreeman451 merged 2 commits from refs/pull/2835/head into staging

2026-02-03 01:28:41 +00:00

mfreeman451 commented

2026-02-03 01:26:11 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2674
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2674
Original created: 2026-02-03T01:26:11Z
Original updated: 2026-02-03T01:28:44Z
Original head: carverauto/serviceradar:2669-chore-change-rust-linker
Original base: staging
Original merged: 2026-02-03T01:28:41Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Enhancement

Description

Add LLVM toolchain (clang/lld) support for cross-platform builds
Enable platform-specific Bazel configuration sections
Register LLVM toolchain with version mappings for macOS and Linux
Clean up whitespace in Bazel configuration

Diagram Walkthrough

flowchart LR
  A["Bazel Configuration"] -->|"Enable platform-specific config"| B["Platform Config Support"]
  C["MODULE.bazel"] -->|"Add toolchains_llvm dependency"| D["LLVM Toolchain Extension"]
  D -->|"Register with version mappings"| E["Clang/LLD Linker Available"]
  B -->|"Support"| E

File Walkthrough

Relevant files

Configuration changes

.bazelrc

Enable platform-specific Bazel configuration

.bazelrc

Enable platform-specific build configuration sections via
--enable_platform_specific_config flag
Remove extra blank line for cleaner formatting

+2/-1

Enhancement

MODULE.bazel

Add LLVM toolchain configuration and registration

MODULE.bazel

Add toolchains_llvm version 1.5.0 as a Bazel dependency
Configure LLVM toolchain extension with clang/lld linker
Define LLVM version mappings for darwin-aarch64, darwin-x86_64, and
ubuntu-22.04-x86_64
Register LLVM toolchain globally for all build targets

+17/-0

Imported from GitHub pull request. Original GitHub pull request: #2674 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2674 Original created: 2026-02-03T01:26:11Z Original updated: 2026-02-03T01:28:44Z Original head: carverauto/serviceradar:2669-chore-change-rust-linker Original base: staging Original merged: 2026-02-03T01:28:41Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Enhancement ___ ### **Description** - Add LLVM toolchain (clang/lld) support for cross-platform builds - Enable platform-specific Bazel configuration sections - Register LLVM toolchain with version mappings for macOS and Linux - Clean up whitespace in Bazel configuration ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Bazel Configuration"] -->|"Enable platform-specific config"| B["Platform Config Support"] C["MODULE.bazel"] -->|"Add toolchains_llvm dependency"| D["LLVM Toolchain Extension"] D -->|"Register with version mappings"| E["Clang/LLD Linker Available"] B -->|"Support"| E ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>.bazelrc</strong><dd><code>Enable platform-specific Bazel configuration</code>                          </dd></summary> <hr> .bazelrc <ul><li>Enable platform-specific build configuration sections via <br><code>--enable_platform_specific_config</code> flag<br> <li> Remove extra blank line for cleaner formatting</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2674/files#diff-544556920c45b42cbfe40159b082ce8af6bd929e492d076769226265f215832f">+2/-1</a>      </td> </tr> </table></td></tr><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>MODULE.bazel</strong><dd><code>Add LLVM toolchain configuration and registration</code>                </dd></summary> <hr> MODULE.bazel <ul><li>Add <code>toolchains_llvm</code> version 1.5.0 as a Bazel dependency<br> <li> Configure LLVM toolchain extension with clang/lld linker<br> <li> Define LLVM version mappings for darwin-aarch64, darwin-x86_64, and <br>ubuntu-22.04-x86_64<br> <li> Register LLVM toolchain globally for all build targets</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2674/files#diff-6136fc12446089c3db7360e923203dd114b6a1466252e71667c6791c20fe6bdc">+17/-0</a>    </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-02-03 01:26:30 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2674#issuecomment-3838281499
Original created: 2026-02-03T01:26:30Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance

🟢

No security concerns identified

No security vulnerabilities detected by AI analysis. Human verification advised for critical code.

Ticket Compliance

🟡

🎫 #2669

🟢	Replace usage of the deprecated `gold` linker for Rust builds with a supported alternative (e.g., `lld` or GNU `ld`) to eliminate the warning about `gold` being deprecated and buggy with Rust.
⚪	Confirm via CI/local build logs that the Rust build no longer invokes `gold` and that the warning is gone on all relevant platforms/targets.

Codebase Duplication Compliance

⚪

Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance

🟢

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2674#issuecomment-3838281499 Original created: 2026-02-03T01:26:30Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2669>#2669</a></summary> <table width='100%'><tbody> <tr><td rowspan=1>🟢</td> <td>Replace usage of the deprecated <code>gold</code> linker for Rust builds with a supported alternative <br>(e.g., <code>lld</code> or GNU <code>ld</code>) to eliminate the warning about <code>gold</code> being deprecated and buggy with <br>Rust.<br></td></tr> <tr><td rowspan=1>⚪</td> <td>Confirm via CI/local build logs that the Rust build no longer invokes <code>gold</code> and that the <br>warning is gone on all relevant platforms/targets.<br></td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=6>🟢</td><td> <details><summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2">   </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2026-02-03 01:27:35 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2674#issuecomment-3838285581
Original created: 2026-02-03T01:27:35Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Scope LLVM toolchain to Rust builds The LLVM toolchain is currently registered globally, affecting all native code. It is recommended to scope this change to only Rust builds to mitigate risk and precisely address the linker warning. Examples: MODULE.bazel [231] `register_toolchains("@llvm_toolchain//:all")` Solution Walkthrough: Before: `# MODULE.bazel # ... # Define LLVM toolchain versions llvm_ext.toolchain( name = "llvm_toolchain", llvm_versions = { ... }, ) use_repo(llvm_ext, "llvm_toolchain") # Register the toolchain globally, making it the default for C/C++ register_toolchains("@llvm_toolchain//:all")` After: `# MODULE.bazel # ... # Define LLVM toolchain versions llvm_ext.toolchain( name = "llvm_toolchain", llvm_versions = { ... }, ) use_repo(llvm_ext, "llvm_toolchain") # DO NOT register toolchain globally. # Instead, configure Rust to use the LLD linker, for example in .bazelrc: # build:rust --linkopt=-fuse-ld=lld # build:rust --@rules_rust//rust/settings:linker_binary=@llvm_toolchain//:lld` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant risk in globally registering the LLVM toolchain, which could unintentionally break non-Rust builds, and proposes a much safer, targeted fix that aligns with the original issue.	High
General	Improve build portability across Linux distributions To improve build portability, replace the specific platform key `ubuntu-22.04-x86_64` with the more generic `linux-x86_64` in the `llvm_versions` dictionary. MODULE.bazel [222-229] `llvm_ext.toolchain( name = "llvm_toolchain", llvm_versions = { "darwin-aarch64": "17.0.4", "darwin-x86_64": "15.0.7", - "ubuntu-22.04-x86_64": "17.0.4", + "linux-x86_64": "17.0.4", }, )` Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that using a specific OS version like `ubuntu-22.04-x86_64` harms build portability and suggests using the more generic `linux-x86_64`, which is a significant improvement for maintainability.	Medium
General	Move flag to build section In `.bazelrc`, move the `--enable_platform_specific_config` flag from the `common` section to the `build` section for more precise scoping. .bazelrc [15] `-common --enable_platform_specific_config +build --enable_platform_specific_config` Apply / Chat Suggestion importance[1-10]: 2 __ Why: While scoping the flag to `build` is slightly more precise, using `common` is also correct and more flexible for future platform-specific configurations for other commands (e.g., `test`). This is a minor stylistic preference with low impact.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2674#issuecomment-3838285581 Original created: 2026-02-03T01:27:35Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Scope LLVM toolchain to Rust builds</summary> ___ **The LLVM toolchain is currently registered globally, affecting all native code. <br>It is recommended to scope this change to only Rust builds to mitigate risk and <br>precisely address the linker warning.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/2674/files#diff-6136fc12446089c3db7360e923203dd114b6a1466252e71667c6791c20fe6bdcR231-R231">MODULE.bazel [231]</a> </summary> ```starlark register_toolchains("@llvm_toolchain//:all") ``` </details> ### Solution Walkthrough: #### Before: ```starlark # MODULE.bazel # ... # Define LLVM toolchain versions llvm_ext.toolchain( name = "llvm_toolchain", llvm_versions = { ... }, ) use_repo(llvm_ext, "llvm_toolchain") # Register the toolchain globally, making it the default for C/C++ register_toolchains("@llvm_toolchain//:all") ``` #### After: ```starlark # MODULE.bazel # ... # Define LLVM toolchain versions llvm_ext.toolchain( name = "llvm_toolchain", llvm_versions = { ... }, ) use_repo(llvm_ext, "llvm_toolchain") # DO NOT register toolchain globally. # Instead, configure Rust to use the LLD linker, for example in .bazelrc: # build:rust --linkopt=-fuse-ld=lld # build:rust --@rules_rust//rust/settings:linker_binary=@llvm_toolchain//:lld ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies a significant risk in globally registering the LLVM toolchain, which could unintentionally break non-Rust builds, and proposes a much safer, targeted fix that aligns with the original issue. </details></details></td><td align=center>High </td></tr><tr><td rowspan=2>General</td> <td> <details><summary>Improve build portability across Linux distributions</summary> ___ **To improve build portability, replace the specific platform key <br><code>ubuntu-22.04-x86_64</code> with the more generic <code>linux-x86_64</code> in the <code>llvm_versions</code> <br>dictionary.** [MODULE.bazel [222-229]](https://github.com/carverauto/serviceradar/pull/2674/files#diff-6136fc12446089c3db7360e923203dd114b6a1466252e71667c6791c20fe6bdcR222-R229) ```diff llvm_ext.toolchain( name = "llvm_toolchain", llvm_versions = { "darwin-aarch64": "17.0.4", "darwin-x86_64": "15.0.7", - "ubuntu-22.04-x86_64": "17.0.4", + "linux-x86_64": "17.0.4", }, ) ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies that using a specific OS version like `ubuntu-22.04-x86_64` harms build portability and suggests using the more generic `linux-x86_64`, which is a significant improvement for maintainability. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Move flag to build section</summary> ___ **In <code>.bazelrc</code>, move the <code>--enable_platform_specific_config</code> flag from the <code>common</code> <br>section to the <code>build</code> section for more precise scoping.** [.bazelrc [15]](https://github.com/carverauto/serviceradar/pull/2674/files#diff-544556920c45b42cbfe40159b082ce8af6bd929e492d076769226265f215832fR15-R15) ```diff -common --enable_platform_specific_config +build --enable_platform_specific_config ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 2</summary> __ Why: While scoping the flag to `build` is slightly more precise, using `common` is also correct and more flexible for future platform-specific configurations for other commands (e.g., `test`). This is a minor stylistic preference with low impact. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>