Skip to content

ci: use builder on windows platforms#1961

Open
hoolioh wants to merge 4 commits into
mainfrom
julio/use-builder-on-windows
Open

ci: use builder on windows platforms#1961
hoolioh wants to merge 4 commits into
mainfrom
julio/use-builder-on-windows

Conversation

@hoolioh
Copy link
Copy Markdown
Contributor

@hoolioh hoolioh commented May 8, 2026

What does this PR do?

Switches the Windows release-artifact build over to the shared builder crate, so all platforms (Linux/glibc, Linux/musl, macOS, Windows) now go through the same path. To make that possible:

  • builder/src/arch/windows.rs: adds Windows support to the builder — copies the PDB and the *.dll.lib import library, uses the _ffi-suffixed artifact names produced by the FFI crate, and no-ops the pkg-config step (Windows has no
    pkg-config).
  • builder/src/arch/mod.rs + profiling.rs (refactor: move add_pkg_config to arch): hoists the per-platform add_pkg_config into each arch/*.rs, with the shared file-templating logic exposed as arch::generate_pkg_config. This is
    what lets Windows opt out cleanly instead of carrying a #[cfg] in profiling.rs.
  • builder/src/builder.rs: skips creating bin/ and pkgconfig/ directories on Windows, where they're unused.
  • windows/build-artifacts.ps1: deleted — its responsibilities now live in the builder.
  • CI cleanup (separate commits, batched in because they unblock the Windows-on-builder rollout):
    • cargo nextest run invocations now pass --no-tests=pass so the workflow doesn't fail when a partition/affected-crate set yields zero tests.
    • .github/workflows/test.yml no longer hand-filters builder / test_spawn_from_lib out of the package list — --no-tests=pass makes that filtering unnecessary, and dropping it means the builder crate's own tests are actually
      exercised by CI when it changes.
    • Same fix applied to miri.yml and coverage.yml.

Motivation

We've been maintaining two parallel build pipelines: every other platform goes through the builder crate, while Windows had its own PowerShell script (windows/build-artifacts.ps1) that re-implemented overlapping logic (artifact naming,
output layout, header post-processing). Any change to how artifacts are produced had to be made (and kept in sync) in two places. Routing Windows through the builder consolidates this and lets future build changes ship in one place.

The CI changes are required collateral: once builder is the path used for the Windows release build, the workflow's hard-coded --exclude builder is wrong, and partition-style invocations need --no-tests=pass so the workflow tolerates
empty test sets.

Additional Notes

  • Companion change in libddprof-build: this PR depends on a matching branch in the private libddprof-build repo. The last commit (dfeffd0b4 chore: reference libddprof branch that matches the changes in the builder) temporarily points
    .gitlab-ci.yml's DOWNSTREAM_BRANCH at that branch. This commit must be reverted to main before merge — please flag this in review if I forget.
  • Windows artifact names move from datadog_profiling.{dll,lib,pdb} to datadog_profiling_ffi.{dll,lib,pdb} plus the new datadog_profiling_ffi.dll.lib import library. Downstream consumers that hard-coded the old names will need to follow.
  • No changes to public APIs or to non-Windows artifact layout.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

📚 Documentation Check Results

⚠️ 236 documentation warning(s) found

📦 builder - 236 warning(s)


Updated: 2026-05-13 09:28:49 UTC | Commit: 01fe761 | missing-docs job results

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

Clippy Allow Annotation Report

Comparing clippy allow annotations between branches:

  • Base Branch: origin/main
  • PR Branch: origin/julio/use-builder-on-windows

Summary by Rule

Rule Base Branch PR Branch Change

Annotation Counts by File

File Base Branch PR Branch Change

Annotation Stats by Crate

Crate Base Branch PR Branch Change
clippy-annotation-reporter 5 5 No change (0%)
datadog-ffe-ffi 1 1 No change (0%)
datadog-ipc 21 21 No change (0%)
datadog-live-debugger 6 6 No change (0%)
datadog-live-debugger-ffi 10 10 No change (0%)
datadog-profiling-replayer 4 4 No change (0%)
datadog-remote-config 3 3 No change (0%)
datadog-sidecar 57 57 No change (0%)
libdd-common 13 13 No change (0%)
libdd-common-ffi 12 12 No change (0%)
libdd-data-pipeline 5 5 No change (0%)
libdd-ddsketch 2 2 No change (0%)
libdd-dogstatsd-client 1 1 No change (0%)
libdd-profiling 13 13 No change (0%)
libdd-telemetry 20 20 No change (0%)
libdd-tinybytes 4 4 No change (0%)
libdd-trace-normalization 2 2 No change (0%)
libdd-trace-obfuscation 8 8 No change (0%)
libdd-trace-stats 1 1 No change (0%)
libdd-trace-utils 15 15 No change (0%)
Total 203 203 No change (0%)

About This Report

This report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

🔒 Cargo Deny Results

⚠️ 1 issue(s) found, showing only errors (advisories, bans, sources)

📦 builder - 1 error(s)

Show output
error[unsound]: Rand is unsound with a custom logger using `rand::rng()`
   ┌─ /home/runner/work/libdatadog/libdatadog/Cargo.lock:85:1
   │
85 │ rand 0.8.5 registry+https://github.com/rust-lang/crates.io-index
   │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ unsound advisory detected
   │
   ├ ID: RUSTSEC-2026-0097
   ├ Advisory: https://rustsec.org/advisories/RUSTSEC-2026-0097
   ├ It has been reported (by @lopopolo) that the `rand` library is [unsound](https://rust-lang.github.io/unsafe-code-guidelines/glossary.html#soundness-of-code--of-a-library) (i.e. that safe code using the public API can cause Undefined Behaviour) when all the following conditions are met:
     
     - The `log` and `thread_rng` features are enabled
     - A [custom logger](https://docs.rs/log/latest/log/#implementing-a-logger) is defined
     - The custom logger accesses `rand::rng()` (previously `rand::thread_rng()`) and calls any `TryRng` (previously `RngCore`) methods on `ThreadRng`
     - The `ThreadRng` (attempts to) reseed while called from the custom logger (this happens every 64 kB of generated data)
     - Trace-level logging is enabled or warn-level logging is enabled and the random source (the `getrandom` crate) is unable to provide a new seed
     
     `TryRng` (previously `RngCore`) methods for `ThreadRng` use `unsafe` code to cast `*mut BlockRng<ReseedingCore>` to `&mut BlockRng<ReseedingCore>`. When all the above conditions are met this results in an aliased mutable reference, violating the Stacked Borrows rules. Miri is able to detect this violation in sample code. Since construction of [aliased mutable references is Undefined Behaviour](https://doc.rust-lang.org/stable/nomicon/references.html), the behaviour of optimized builds is hard to predict.
   ├ Announcement: https://github.com/rust-random/rand/pull/1763
   ├ Solution: Upgrade to >=0.10.1 OR <0.10.0, >=0.9.3 OR <0.9.0, >=0.8.6 (try `cargo update -p rand`)
   ├ rand v0.8.5
     └── (dev) libdd-common v4.0.0
         ├── builder v33.0.0
         └── tools v33.0.0
             └── builder v33.0.0 (*)

advisories FAILED, bans ok, sources ok

Updated: 2026-05-13 09:30:48 UTC | Commit: 01fe761 | dependency-check job results

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 8, 2026

Codecov Report

❌ Patch coverage is 0% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.55%. Comparing base (866d7c8) to head (8a4955f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1961      +/-   ##
==========================================
- Coverage   72.58%   72.55%   -0.03%     
==========================================
  Files         451      452       +1     
  Lines       74095    74111      +16     
==========================================
- Hits        53782    53773       -9     
- Misses      20313    20338      +25     
Components Coverage Δ
libdd-crashtracker 65.32% <ø> (+0.01%) ⬆️
libdd-crashtracker-ffi 37.68% <ø> (ø)
libdd-alloc 98.77% <ø> (ø)
libdd-data-pipeline 85.97% <ø> (ø)
libdd-data-pipeline-ffi 71.04% <ø> (ø)
libdd-common 79.81% <ø> (ø)
libdd-common-ffi 74.41% <ø> (ø)
libdd-telemetry 69.89% <ø> (ø)
libdd-telemetry-ffi 19.37% <ø> (ø)
libdd-dogstatsd-client 82.64% <ø> (ø)
datadog-ipc 76.22% <ø> (ø)
libdd-profiling 81.57% <ø> (ø)
libdd-profiling-ffi 64.51% <ø> (ø)
libdd-sampling 97.25% <ø> (ø)
datadog-sidecar 29.09% <ø> (-0.05%) ⬇️
datdog-sidecar-ffi 9.67% <ø> (ø)
spawn-worker 54.69% <ø> (ø)
libdd-tinybytes 93.16% <ø> (ø)
libdd-trace-normalization 81.71% <ø> (ø)
libdd-trace-obfuscation 87.39% <ø> (ø)
libdd-trace-protobuf 68.25% <ø> (ø)
libdd-trace-utils 88.97% <ø> (+0.11%) ⬆️
libdd-tracer-flare 86.88% <ø> (ø)
libdd-log 74.83% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@datadog-datadog-prod-us1-2
Copy link
Copy Markdown

datadog-datadog-prod-us1-2 Bot commented May 8, 2026

🎯 Code Coverage (details)
Patch Coverage: 0.00%
Overall Coverage: 72.56%

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 8a4955f | Docs | Datadog PR Page | Give us feedback!

@hoolioh hoolioh force-pushed the julio/use-builder-on-windows branch 2 times, most recently from 9c30399 to f3b0432 Compare May 12, 2026 11:34
@hoolioh hoolioh marked this pull request as ready for review May 12, 2026 11:46
@hoolioh hoolioh requested review from a team as code owners May 12, 2026 11:46
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dfeffd0b47

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .gitlab-ci.yml
# If these are set witih value and description, then it gives you UI elements
DOWNSTREAM_BRANCH:
value: "main"
value: "julio/use-builder-on-windows"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Revert the downstream CI branch

Leaving DOWNSTREAM_BRANCH pointed at julio/use-builder-on-windows means every GitLab pipeline that reaches trigger_internal_build will trigger the downstream DataDog/apm-reliability/libddprof-build project from this temporary feature branch instead of the production main branch. This matches the commit note saying it must be reverted before merge, and would make merged/main pipelines depend on a developer branch.

Useful? React with 👍 / 👎.

@hoolioh hoolioh requested a review from gleocadie May 12, 2026 11:54
@hoolioh hoolioh force-pushed the julio/use-builder-on-windows branch from 4e141e3 to 8a4955f Compare May 13, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants