Skip to content

refactor: replace direct windows-sys usage with windows/windows-registry crates - #2778

Merged
DavSanchez merged 3 commits into
mainfrom
refactor/remove-direct-windows-sys
Aug 7, 2026
Merged

refactor: replace direct windows-sys usage with windows/windows-registry crates#2778
DavSanchez merged 3 commits into
mainfrom
refactor/remove-direct-windows-sys

Conversation

@DavSanchez

@DavSanchez DavSanchez commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

While working on #2760 I noticed an area of improvement regarding the Windows API crates.

Summary

  • agent-control's is_elevated.rs now uses windows::Win32::{Foundation,Security,System::Threading} instead of windows_sys, same logic, Result-based error handling.
  • resource-detection's registry read (MachineGuid lookup) is rewritten on top of windows-registry's Key API, dropping ~100 lines of unsafe RegOpenKeyExW/RegQueryValueExW FFI entirely.
  • Drops the direct windows-sys dependency from both crates' Cargo.toml (workspace-level entry stays — fs on main still needs it until fix(fs): make Windows admin ACE inheritable #2760 merges).

Follow-up to #2760, but branched independently from main since there's no file overlap.

Test plan

  • cargo check --workspace (host target)
  • cargo zigbuild --target x86_64-pc-windows-gnu -p newrelic_agent_control -p resource-detection — real Windows cross-compile, not just cfg skip
  • cargo fmt --check on touched crates
  • CI Windows runner (push_pr_checks_tests.yml) as final confirmation

@DavSanchez
DavSanchez requested a review from a team as a code owner August 5, 2026 17:03
sigilioso
sigilioso previously approved these changes Aug 7, 2026

@sigilioso sigilioso left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@DavSanchez
DavSanchez merged commit c0ff440 into main Aug 7, 2026
38 checks passed
@DavSanchez
DavSanchez deleted the refactor/remove-direct-windows-sys branch August 7, 2026 08:43
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.

2 participants