feat: Windows ARM64 support and tool installation fixes

[larseggert]

• Set ilammy/msvc-dev-cmd arch to amd64_arm64 on ARM64 runners for cross-compilation targeting ARM64
• Install cargo-fuzz from source so it uses the same nightly toolchain as the fuzzing job
• Install cargo-careful from source on non-Linux until binary releases work on macOS/Windows

[Copilot]

Pull request overview

This PR updates the rust composite action to better support Windows ARM64 runners and to make Rust tool installation behave correctly for specific tools that need source builds.

Changes:

• Configure ilammy/msvc-dev-cmd to use amd64_arm64 on Windows ARM64 runners (otherwise amd64).
• Special-case tool installation so cargo-fuzz is installed via cargo install (to compile with the active toolchain) and cargo-careful is installed via cargo install on non-Linux runners.
• Expose runner.os to the install script via RUNNER_OS env var.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Clean, targeted PR. The three changes are well-scoped: ARM64 MSVC configuration, and two justified special-cases for tool installation.

The install loop is starting to accumulate tool-specific branches. If more tools need custom handling in the future, consider extracting this into a small helper script or a lookup table to keep the loop body from growing further — but for two cases this is fine.

See inline comments for minor suggestions on the MSVC arch choice and --locked consistency.

[github-actions]

The rust action changes are well-targeted — ARM64 MSVC setup and the two source-install special-cases are justified and clearly commented.

The main concern is that windows-11-arm is also added to the test-nss matrix, but nss/action.yml has no ARM64 Windows support (msys2 paths, MSVC arch, mingw64 packages). This will likely cause CI failures. See inline comment.

Previous inline feedback on rust/action.yml (redundant RUNNER_OS env, --locked for cargo-careful, native arm64 vs cross-compiler arch) still applies — those comments are on the prior review.

[github-actions]

The rust action changes are well-scoped — the MSVC arch conditional and the two source-install special cases are clearly motivated. Two items need addressing before merge:

1. test-nss matrix will break on ARM64: nss/action.yml has hardcoded x86_64 msys2 paths, x86_64-only pacman packages, and no arch parameter on its own MSVC setup. Adding windows-11-arm to test-nss will fail. Exclude it from that matrix and handle NSS ARM64 in a follow-up.

2. Missing --locked on cargo-careful: Unlike cargo-fuzz (which reasonably omits it due to nightly toolchain churn), cargo-careful has no such constraint. Add --locked for reproducibility, consistent with the generic fallback path.

Minor items (non-blocking):

• The RUNNER_OS env mapping is redundant — it's a default runner variable already available in shell steps.
• Consider using the native arm64 MSVC toolchain instead of the amd64_arm64 cross-compiler, if the runner image supports it.
• The cargo-fuzz and cargo-careful install paths have no CI coverage (test-rust only installs cargo-nextest). A test job exercising these would catch regressions.

[github-actions]

The rust/action.yml changes are in good shape after the fixup commits — native ARM64 MSVC selection (arm64), intentional --locked omission for cargo-fuzz (nightly compatibility), and the redundant RUNNER_OS env mapping removal all look correct.

The nsinstall replacement in nss/action.yml has two issues that need fixing before merge (see inline comment): an unpinned fetch from a mutable Git ref, and $HOME/bin not being on PATH/not being created.

I concur with the prior review feedback that test-nss should exclude windows-11-arm until nss/action.yml gains ARM64 awareness — the hardcoded C:/msys64/mingw64/bin path (x86_64-only), and the ilammy/msvc-dev-cmd step without an arch parameter, will both cause failures on ARM64 runners. The test-rust inclusion is fine since rust/action.yml handles the arch correctly.

Minor: the cargo-fuzz and cargo-careful special-case paths aren't exercised by any CI job (only cargo-nextest is installed in test-rust). Consider adding a matrix entry or dedicated job to smoke-test these installs on at least one platform.

[github-actions]

Summary

Well-scoped PR. The rust/action.yml ARM64 support and tool installation special-cases are clean. The Renovate custom datasource for Firefox tags is correctly wired. The fixup commits addressed most prior review feedback well:

• ✅ Native arm64 MSVC toolchain (not amd64_arm64 cross-compiler)
• ✅ --locked on cargo-careful, intentionally omitted on cargo-fuzz
• ✅ Redundant RUNNER_OS env mapping removed
• ✅ nsinstall pinned to FIREFOX_149_0_2_RELEASE with Renovate automation
• ✅ $HOME/bin created and added to $GITHUB_PATH

Remaining items

Blocking:

1. test-nss on windows-11-arm (see inline comment): nss/action.yml still has x86_64 msys2 assumptions (C:/msys64/mingw64/bin) and it's unclear whether windows-11-arm runners ship msys2. Exclude from test-nss and handle in a follow-up.

2. No integrity check on nsinstall.py fetch (see inline comment): Every other external fetch in this repo is checksummed or SHA-pinned. This downloads executable code without verification. Add a SHA256 check and extend the Renovate packageRules reminder to cover firefox.

Non-blocking (from prior reviews, still valid):

• The cargo-fuzz and cargo-careful special-case install paths have no CI coverage (test-rust only exercises cargo-nextest). Consider a smoke-test in a follow-up.

[github-actions]

Summary

Solid work adding Windows ARM64 support across the Rust and NSS actions, including a well-structured set of NSPR/NSS patches with clear documentation and removal conditions. The patch approach (individual, self-contained files applied at build time) is the right call for upstream-pending changes.

Key observations

Architecture: The 9 NSS/NSPR patches correctly address the full surface area needed for ARM64 MSVC: preprocessor guards (_M_ARM64), compiler intrinsics (Interlocked→_Interlocked), alignment attributes, GCM/AES/SHA crypto paths, and gyp configuration. Each patch is well-documented with upstream removal conditions.

Supply chain: The nsinstall.py fetch is pinned to FIREFOX_149_0_2_RELEASE with Renovate tracking — good. However, per prior review feedback, there's still no SHA256 integrity check on the downloaded executable code, unlike other external fetches in this repo. I concur this should be addressed.

Status of prior review threads

Thread	Status
rust/action.yml arm64 vs amd64_arm64	Addressed — now uses native 'arm64' toolchain
rust/action.yml redundant RUNNER_OS	Addressed — removed from env block
rust/action.yml cargo-careful --locked	Addressed — --locked added
test-nss windows-11-arm exclusion	Partially addressed — nss/action.yml now has full ARM64 support (MSVC arch, patches, nsinstall, gyp-next). The msys64/mingw64/bin concern remains valid (see inline comment)
nsinstall.py SHA256 check	Still open — concur this should be added

[github-actions]

Summary

Well-executed PR adding Windows ARM64 support across both the Rust and NSS composite actions, including a comprehensive set of 9 NSPR/NSS patches. The patch-per-issue structure with clear upstream-removal conditions is good engineering for carrying platform workarounds.

Prior review threads — status

All previously flagged blocking issues have been addressed:

Prior concern	Status
arm64 vs amd64_arm64 MSVC toolchain	Resolved — native arm64 used
Redundant RUNNER_OS env in rust/action.yml	Resolved — removed
cargo-careful --locked	Resolved — added
nsinstall.py unpinned fetch	Resolved — pinned to FIREFOX_149_0_2_RELEASE with Renovate tracking
nsinstall.py missing SHA256 check	Resolved — sha256sum -c added, firefox added to Renovate packageRules
$HOME/bin not on PATH	Resolved — mkdir -p + GITHUB_PATH append
VSPATH fallback :-1	Resolved — uses ${VSINSTALLDIR:?...}
mingw64/bin unconditional on ARM64	Resolved — conditional exclusion
Interlocked intrinsics incomplete	Resolved — all 5 functions redirected
test-nss on windows-11-arm	Resolved — nss/action.yml now has full ARM64 support

New observations (see inline comments)

• sed commands on nspr/configure and Makefile.in are fragile — if upstream changes the matched patterns, the sed silently no-ops and the build produces wrong-architecture output. Adding grep -q guards would make failures loud.
• IS_LITTLE_ENDIAN guard inconsistency in nss-aes-armv8-msvc.patch — the MSVC path omits the endianness check that the GCC/Clang path and the GHASH patch both require.
• /EHsc only on ARM64 in nss-config-gypi-arm64.patch — worth confirming this is intentional if the x64 block doesn't set it.

Still open from prior reviews (non-blocking)

• The cargo-fuzz and cargo-careful source-install paths have no CI coverage (test-rust only installs cargo-nextest). Consider a smoke-test in a follow-up.

No blocking issues identified. The PR is in good shape.

[github-actions]

Summary

Well-executed PR adding Windows ARM64 support across both composite actions and a comprehensive set of 9 NSPR/NSS patches. The patch-per-issue structure with clear upstream-removal conditions is excellent for carrying platform workarounds.

All previously flagged blocking issues from prior reviews have been addressed: native arm64 MSVC toolchain, --locked on cargo-careful, redundant RUNNER_OS removal, nsinstall.py SHA256 verification + Renovate tracking, $HOME/bin PATH setup, VSINSTALLDIR fail-fast via :?, and conditional mingw64/bin exclusion.

New observations

• Cross-patch dependency (see inline): nss-aes-armv8-msvc.patch depends on nss-config-gypi-arm64.patch for __ARM_FEATURE_CRYPTO. The "self-contained and can be removed individually" comment in nss/action.yml should note this exception.
• mozfile dependency (see inline): Missing a brief comment explaining why it's needed.

Still open from prior reviews (non-blocking, concur)

• The cargo-fuzz and cargo-careful source-install paths have no CI coverage (test-rust only exercises cargo-nextest). A smoke-test in a follow-up would catch regressions.
• IS_LITTLE_ENDIAN bypass in the AES patch is safe but inconsistent with the GHASH patch pattern.

No blocking issues. The PR is in good shape.

[github-actions]

Summary

Well-executed PR adding Windows ARM64 support across both composite actions, backed by a comprehensive set of 10 NSPR/NSS patches. The patch-per-issue structure with clear upstream-removal conditions and the use of patch -p1 (which fails loudly on context mismatch, unlike sed) is good engineering for carrying platform workarounds.

Crypto correctness

Verified all 10 patches touching NSPR/NSS crypto code. The changes are exclusively preprocessor guard extensions (_M_ARM64 alongside __aarch64__) and MSVC compatibility shims — no algorithmic changes:

• AES/GCM/GHASH: Guard extensions + __builtin_bswap64 → _byteswap_uint64, __builtin_assume_aligned → no-op (safe on ARM64 — unaligned NEON access is supported), poly64_t cast removal (correct for MSVC's vmull_p64 signature)
• SHA-1/SHA-256: _M_ARM64 exclusion from 64-bit SHA_HW_t (prevents stride mismatch with vld1q_u32), __attribute__((aligned)) → __declspec(align) portability macro
• Interlocked intrinsics: All 5 functions redirected (CompareExchange, Increment, Decrement, Exchange, ExchangeAdd), placed correctly after <windows.h> to shadow function declarations with intrinsic macros
• IS_LITTLE_ENDIAN: Checked consistently on both GCC/Clang and MSVC paths in both aes-armv8.c and ghash-aarch64.c

Prior review threads — resolution status

All previously flagged blocking issues have been addressed in fixup commits:

Prior concern	Status
arm64 vs amd64_arm64 MSVC toolchain	Resolved — native arm64 used
Redundant RUNNER_OS env in rust/action.yml	Resolved — removed
cargo-careful --locked	Resolved — added
nsinstall.py unpinned fetch	Resolved — pinned to FIREFOX_149_0_2_RELEASE with Renovate tracking
nsinstall.py missing SHA256 check	Resolved — sha256sum -c added, firefox added to Renovate packageRules
$HOME/bin not on PATH	Resolved — mkdir -p + GITHUB_PATH append
VSPATH fallback	Resolved — uses ${VSINSTALLDIR:?...} for fail-fast
mingw64/bin unconditional on ARM64	Resolved — conditional exclusion
Interlocked intrinsics incomplete	Resolved — all 5 functions redirected
test-nss on windows-11-arm	Resolved — nss/action.yml now has full ARM64 support
sed fragility	Resolved — replaced with patch -p1 which fails loudly on context mismatch

Non-blocking (concur with prior reviews)

• The cargo-fuzz and cargo-careful source-install paths have no CI coverage (test-rust only exercises cargo-nextest). A smoke-test in a follow-up would catch regressions.

No blocking issues identified. See inline comments for minor suggestions.

[github-actions]

Summary

Solid PR adding Windows ARM64 support across both composite actions, with a well-structured set of 10 NSPR/NSS patches. The patch-per-file approach with patch -p1 (which fails loudly on context mismatch) and clear upstream-removal conditions in each patch header is good engineering for carrying platform workarounds.

Crypto patch correctness

Verified all NSPR/NSS patches — the changes are exclusively preprocessor guard extensions (_M_ARM64 alongside __aarch64__) and MSVC compatibility shims with no algorithmic changes:

• Interlocked intrinsics: All 5 functions redirected, placed correctly after <windows.h> to shadow declarations with intrinsic macros. ARM64 kernel32.lib lacking these as callable symbols is a known platform constraint.
• __builtin_assume_aligned → no-op: Safe — ARM64 NEON supports unaligned access; the hint is a compiler optimization, not a correctness requirement.
• vmull_p64 cast removal (ghash patch): Correct — MSVC's vmull_p64 expects poly64x1_t vectors (returned by vget_low_p64), while GCC/Clang expect poly64_t scalars. Dropping the cast is compatible with both.
• SHA-1 SHA_HW_t exclusion: Correctly prevents 64-bit stride mismatch with vld1q_u32 in the ARM HW SHA-1 path.
• IS_LITTLE_ENDIAN: Checked on MSVC path in both aes-armv8.c and ghash-aarch64.c — consistent.

Prior review threads

All previously flagged blocking issues from the extensive prior review history have been addressed. Concur with the standing non-blocking note that the cargo-fuzz / cargo-careful source-install paths lack CI coverage (test-rust only exercises cargo-nextest).

New observations

See inline comments — a duplicate comment line from editing and a minor curl hardening suggestion. No blocking issues.

[github-actions]

Summary

Well-structured PR adding Windows ARM64 support across both composite actions, backed by 10 focused NSPR/NSS patches. The patch-per-file approach with patch -p1 (fails loudly on context mismatch) and clear upstream-removal conditions in each patch header is the right strategy for carrying platform workarounds.

Independent crypto patch verification

Reviewed all 10 NSPR/NSS patches for correctness. The changes are exclusively preprocessor guard extensions (_M_ARM64 alongside __aarch64__) and MSVC compatibility shims — no algorithmic changes:

• Interlocked intrinsics (nspr-winnt-interlocked-intrinsics.patch): All 5 functions (CompareExchange, Increment, Decrement, Exchange, ExchangeAdd) redirected to _Interlocked* compiler intrinsics via #define, placed correctly after <windows.h> to shadow function declarations.
• GHASH vmull_p64 cast removal (nss-ghash-aarch64-msvc.patch): Correct — vget_low_p64 returns poly64x1_t; GCC/Clang implicitly convert to poly64_t for vmull_p64, while MSVC expects poly64x1_t directly. Dropping the cast is compatible with both.
• SHA-1 SHA_HW_t exclusion (nss-sha-fast-arm64.patch): Correctly prevents 64-bit stride mismatch with vld1q_u32 in the ARM HW SHA-1 path.
• __builtin_assume_aligned → no-op (nss-arm64-gyp-and-aes.patch): Safe — this is a compiler optimization hint, not a correctness requirement. ARM64 NEON supports unaligned access.
• __ARM_FEATURE_CRYPTO via gyp PreprocessorDefinitions: Correct — Windows ARM64 ABI mandates crypto extensions, and blinit.c's CheckARMSupport (patched by nss-blinit-arm64.patch) provides runtime feature detection as an additional safety net.
• IS_LITTLE_ENDIAN: Checked consistently on the MSVC path in both aes-armv8.c and ghash-aarch64.c.
• Coupled patch dependency (nss-arm64-gyp-and-aes.patch): The __ARM_FEATURE_CRYPTO #error guard in aes-armv8.c requires the gyp PreprocessorDefinitions from the same patch — correctly documented as bundled.

Prior review threads

All previously flagged blocking issues have been addressed across fixup commits. Concur with the standing non-blocking observation that the cargo-fuzz and cargo-careful source-install paths in rust/action.yml lack CI coverage (test-rust only exercises cargo-nextest).

See inline comments for two minor suggestions.

[github-actions]

Summary

Well-structured PR adding Windows ARM64 support through 10 isolated NSPR/NSS patches, MSVC toolchain selection, and improved tool installation. The patch-based approach is significantly more maintainable than the earlier sed approach, and each patch file has excellent documentation (problem, fix, removal criteria).

Key improvements since earlier review rounds (addressing prior feedback):

• nsinstall.py pinned to a Firefox release tag with SHA256 verification (supply-chain risk mitigated)
• curl hardened with --proto '=https' --tlsv1.2
• VSPATH uses ${VSINSTALLDIR:?...} for fail-fast on missing env
• ilammy/msvc-dev-cmd arch uses native arm64 instead of amd64_arm64 cross-compiler
• || true added to prevent non-zero exit from the mingw64/bin conditional
• Renovate integration for tracking Firefox releases with manual SHA256 update notes

Remaining observations:

• The nsinstall replacement and gyp-next minimum bump affect all Windows builds, not just ARM64 (see inline comment). This is likely fine but broader than the PR description suggests.
• The cargo-fuzz and cargo-careful special-case install paths in rust/action.yml still lack CI coverage (as noted in a prior review).
• The PR description mentions amd64_arm64 for cross-compilation but the code now uses arm64 (native toolchain) — consider updating the description.

Cryptographic correctness: The NSPR/NSS patches correctly extend __aarch64__ guards to _M_ARM64, add MSVC-compatible intrinsic redirects, and handle the vmull_p64 type signature difference between MSVC and GCC/Clang. The runtime feature detection in blinit.c (IsProcessorFeaturePresent) provides a safety net for the unconditional __ARM_FEATURE_CRYPTO compile-time define.

[github-actions]

Well-executed PR — the 10 NSPR/NSS patches are individually well-documented with clear upstream-removal conditions, the crypto changes are exclusively guard extensions and MSVC compat shims (no algorithmic changes), and all prior review feedback has been addressed.

One blocking issue: the renovate.json change inadvertently removes the == match pattern, which breaks Renovate tracking for gyp-next (see inline comment).

The standing non-blocking observation from prior reviews still applies: the cargo-fuzz and cargo-careful source-install paths in rust/action.yml have no CI coverage (test-rust only installs cargo-nextest).

Minor: the PR description still references amd64_arm64 for cross-compilation, but the code now correctly uses native arm64 — consider updating the description to match.

[github-actions]

Well-structured PR. The 10 NSPR/NSS patches are individually documented with clear upstream-removal conditions, the crypto changes are strictly preprocessor guard extensions and MSVC compat shims (no algorithmic changes), and all prior blocking feedback has been addressed across fixup commits.

Two items remain (see inline comments):

1. renovate.json: The pip == match pattern was dropped when adding the Firefox datasource pattern, breaking Renovate tracking for gyp-next. Concur with the prior review — this needs to be restored before merge.

2. VSPATH workaround removal: The MSVC 18 workaround in the "Set up build environment (Windows)" step was silently removed. If windows-2025 runners carry MSVC 18, x64 NSS builds will regress. Worth confirming this is intentional.

Minor (non-blocking):

• The PR description still references amd64_arm64 for cross-compilation, but the code correctly uses native arm64 — consider updating to match.
• Concur with prior reviews that cargo-fuzz / cargo-careful source-install paths lack CI coverage (test-rust only exercises cargo-nextest).

[github-actions]

The ARM64 Windows patches for NSPR/NSS are well-structured — each patch is narrowly scoped, clearly documented with removal criteria, and the coupled gyp+AES pair is correctly bundled. The rust/action.yml tool installation logic and the nsinstall.py supply-chain hardening (SHA256 check, TLS pinning) are solid.

Blocking: nss-msvc-arm64.patch is referenced in the Build step but does not exist in nss/patches/. This will break all Windows NSS builds since the patch is applied unconditionally (not just ARM64). This appears to be either a missing commit or a rename that wasn't propagated.

Also still open from prior reviews:

• The == Renovate regex pattern was dropped, breaking gyp-next version tracking (see inline comment concurring with prior feedback).
• The VSPATH/GYP workaround for MSVC 18 (VS 2026) was removed — the missing nss-msvc-arm64.patch was presumably its replacement, so fixing the missing patch should resolve this too.

[github-actions]

Review summary

Well-structured PR adding Windows ARM64 support across the rust and nss composite actions. The patch-per-issue approach for NSPR/NSS with detailed header documentation is excellent — it clearly communicates intent and makes incremental removal straightforward as upstream support lands.

Previously raised issues now resolved

• nss-msvc-arm64.patch missing (comment): The patch file now exists in nss/patches/ and handles both the ARM64 target-arch mapping and VS 2026 catalog_productLineVersion compat.
• windows-11-arm in test-nss before NSS had ARM64 support (comment): The NSS action now has full ARM64 support (patches, nsinstall replacement, MSVC arch config), making this matrix entry valid.
• Debug echo statements (comment): No longer present in the current code.

Still-open blocking issues from prior reviews

These remain unaddressed and should be fixed before merge:

1. Renovate matchStrings regression (comment, comment): Two patterns were removed when adding the Firefox raw-file pattern:

   • The == pip pattern — breaks tracking of gyp-next==0.22.2 in both macOS and Windows dependency steps.
   • The version+digest pattern — breaks tracking of cargo-binstall (BINSTALL_VERSION/BINSTALL_SHA in rust/action.yml), since no remaining pattern matches indented [A-Z_]+=value without quotes or v prefix.

   Both removed patterns should be restored alongside the new Firefox pattern.

Minor note

The PR description mentions setting ilammy/msvc-dev-cmd arch to amd64_arm64 (cross-compiler), but the actual code uses arm64 (native toolchain) — which is the better choice for native ARM64 runners. Consider updating the description to match.

[github-actions]

Good work getting ARM64 Windows support into both the Rust and NSS actions, including the VS 2026 compatibility fix. The patch-based approach for NSPR/NSS is well-structured — each patch is independently documented, scoped to a single concern, and annotated with removal criteria. The nsinstall.py supply-chain hardening (pinned release tag + SHA256) is solid.

Resolved since prior reviews:

• nss-msvc-arm64.patch now exists in nss/patches/.
• Debug echo statements have been removed.
• The removed VSPATH/GYP inline workaround is now handled by nss-msvc-arm64.patch (change 2 — VS 2026 catalog_productLineVersion mapping in msvc.sh).

Still open from prior reviews — please address before merging:

1. renovate.json — removed match patterns (thread): Two matchStrings patterns were removed when adding the Firefox one. The == pattern is needed to track gyp-next==0.22.2 in both the macOS and Windows install steps of nss/action.yml. The version+digest pattern is needed for cargo-binstall (version + commit SHA pair in rust/action.yml). The new Firefox raw-file pattern should be added alongside the existing ones, not replace them.

2. nss-msvc-arm64.patch — sed vc_crt derivation (thread): The sed 's/20\([0-9]\).*/14\1/' fallback captures only the third character of the year string, producing wrong CRT directory names for VS 2019 (141 instead of 142) and VS 2022 (142 instead of 143). Only VS 2017 → 141 is correct. Since VS 2026 is handled by the explicit 18) case, this only affects hypothetical use on older VS versions — but it's still a latent bug.

Minor note: The PR description mentions setting ilammy/msvc-dev-cmd arch to amd64_arm64 (cross-compiler), but the code correctly uses arm64 (native toolchain). Consider updating the description to match.

[github-actions]

Summary

Well-engineered PR that adds Windows ARM64 support through a comprehensive set of NSPR/NSS patches, each clearly documented with upstream removal criteria. The nsinstall replacement (pinned tag + SHA256) and the tool installation special-casing are sound design choices.

Blocking

• renovate.json: two Renovate patterns were accidentally removed when the Firefox raw-file pattern was added. The pip == pattern (tracking gyp-next) and the version+digest pattern (tracking cargo-binstall version+SHA pairs) must be restored — without them, Renovate either silently stops tracking dependencies or produces broken version bumps with stale commit SHAs. See inline comment.

• nss-msvc-arm64.patch: the sed fallback for vc_crt produces wrong CRT directory names for VS 2019 and VS 2022 (141/142 instead of 142/143). Since this patch is applied unconditionally on all Windows targets, this affects windows-2022 runners. An explicit case statement is safer. See inline comment.

Observations (already raised by prior reviewers, concurring)

• The native arm64 MSVC toolchain selection is the right choice over amd64_arm64 cross-compilation on native ARM64 runners.
• Debug echo statements have been cleaned up (resolving earlier review feedback).
• The nss-msvc-arm64.patch file exists in the PR despite an earlier review comment suggesting otherwise — that concern is resolved.
• The patch coupling between nss-arm64-gyp-and-aes.patch (__ARM_FEATURE_CRYPTO in PreprocessorDefinitions) and aes-armv8.c's #error guard is correctly documented in the patch header.

Minor (non-blocking)

• PR description references amd64_arm64 but the code correctly uses arm64 — description could be updated for accuracy.

[github-actions]

Well-structured PR. The patch-per-issue approach for NSPR/NSS with patch -p1 (fails loudly on context mismatch) and clear upstream-removal criteria in each header is excellent maintainability engineering. Independent verification of the 10 crypto/concurrency patches confirms they are exclusively preprocessor guard extensions (_M_ARM64 alongside __aarch64__) and MSVC compatibility shims — no algorithmic changes. The _InterlockedExchangePointer spin-lock in nspr-ntmisc-arm64.patch correctly provides full-barrier semantics needed on ARM64's weakly-ordered memory model, and the IS_LITTLE_ENDIAN guard is now consistently checked on both GCC/Clang and MSVC paths in the AES and GHASH patches.

Blocking

renovate.json pattern regression (see inline comment): Two existing matchStrings patterns were replaced instead of supplemented — this silently breaks Renovate tracking for gyp-next (pip == pattern) and cargo-binstall version+SHA pairs (version+digest pattern). This has been flagged across multiple prior review rounds and remains unaddressed.

Resolved since prior reviews

All other previously-flagged blocking issues have been addressed: native arm64 MSVC toolchain, --locked on cargo-careful, redundant RUNNER_OS removal in rust/action.yml, nsinstall.py SHA256 verification + Renovate tracking, curl TLS hardening, $HOME/bin PATH setup, VSINSTALLDIR fail-fast via :?, conditional mingw64/bin exclusion, nss-msvc-arm64.patch now exists, and debug echo statements removed.

Non-blocking

• The vc_crt sed derivation in nss-msvc-arm64.patch is incorrect for pre-2026 VS versions (see inline comment) — latent bug, not triggered by the current static-build use case.
• The cargo-fuzz / cargo-careful source-install paths in rust/action.yml have no CI coverage (test-rust only exercises cargo-nextest). Concur with prior reviews that a smoke-test in a follow-up would catch regressions.
• The PR description mentions amd64_arm64 (cross-compiler) but the code correctly uses arm64 (native toolchain) — consider updating the description.

[github-actions]

Summary

Well-structured PR adding Windows ARM64 support to the NSS and Rust actions. The patch-based approach (13 individual patches with per-file rationale and removal criteria) is a pragmatic way to track upstream without maintaining a fork. The nsinstall.py replacement with SHA256 verification and Renovate tracking is a clear improvement over the previous pacman-based approach.

Architecture notes

• The ilammy/msvc-dev-cmd arch parameter correctly uses arm64 (native ARM64 toolchain) rather than amd64_arm64 (x64→ARM64 cross-compiler under emulation). The PR description mentions amd64_arm64 — worth updating for accuracy.
• The nss-msvc-arm64.patch is applied unconditionally on all Windows targets since its VS 2026 catalog_productLineVersion fix benefits all architectures — this is the right call.
• The nspr-ntmisc-arm64.patch spin-lock implementations correctly use _InterlockedExchangePointer (full barrier) for both lock acquisition and release, which is stronger than needed for release but correct for ARM64's weakly-ordered memory model.

Open concerns from prior reviews (still applicable)

These have been raised multiple times in prior review rounds and remain unresolved — I won't duplicate them:

1. Renovate pattern regression (renovate.json): the == pip pattern was removed, breaking tracking of gyp-next==0.22.2 in both macOS and Windows steps.
2. nss-msvc-arm64.patch sed CRT mapping: sed 's/20\([0-9]\).*/14\1/' produces incorrect vc_crt values for VS 2019 (141 instead of 142) and VS 2022 (142 instead of 143). Only the explicit 18) case for VS 2026 is correct.

New finding

• mozfile unpinned (see inline comment on nss/action.yml:219): supply-chain and reproducibility concern for an import dependency of nsinstall.py.

[github-actions]

Review Summary

Well-structured ARM64 Windows bring-up. The 13 modular NSPR/NSS patches are individually documented with upstream-removability criteria, and the two-tier application (nss-msvc-arm64.patch on all Windows for VS 2026 compat, remaining patches ARM64-only) is a clean design. The nsinstall.py replacement with SHA256 pinning, the ilammy/msvc-dev-cmd arch plumbing, and the cargo-fuzz/cargo-careful source-build special cases all look correct.

Several issues raised in prior reviews appear addressed in the current revision (debug echo statements removed, curl now has --proto '=https' --tlsv1.2, SHA256 verification present for nsinstall.py).

Still blocking (from prior reviews — not re-raising, just synthesizing)

1. Renovate matchStrings patterns removed (multiple prior threads on renovate.json:44): The pip == and version+digest patterns were replaced by the Firefox raw-file pattern instead of being added alongside. Both removed patterns are actively used — gyp-next==0.22.2 (in both nss/action.yml macOS and Windows blocks) needs the pip pattern, and cargo-binstall (version + commit SHA in rust/action.yml) needs the digest pattern. Without the digest pattern, Renovate will bump BINSTALL_VERSION but leave BINSTALL_SHA stale, causing the install script fetch to use a mismatched commit.

2. nss-msvc-arm64.patch sed fallback (prior threads on line 49): sed 's/20\([0-9]\).*/14\1/' captures only the third character of the year, producing wrong CRT directory names for VS 2019 (→VC141, should be VC142) and VS 2022 (→VC142, should be VC143). Low practical impact since VS 2026 is handled by the explicit case 18) branch, but the fallback is silently wrong for any other VS version.

Minor open items (from prior reviews)

• mozfile unpinned (nss/action.yml:219): Unlike gyp-next==0.22.2, mozfile has no version constraint — installs whatever PyPI resolves at build time.
• cargo install cargo-fuzz omits --locked: Intentional for nightly compatibility per previous discussion, but worth a brief inline comment so it isn't "fixed" later.

[github-actions]

Summary

Thorough and well-structured PR that adds Windows ARM64 support across the Rust and NSS actions, along with VS 2026 compatibility fixes and tool installation improvements. The 12 NSPR/NSS patches are individually documented with clear "removable once upstream supports X" notes, which is excellent for long-term maintenance.

Architecture observations

VS 2026 compatibility is now patch-dependent. The previous GITHUB_ENV-based workaround was decoupled from NSS source changes. The new approach (patching msvc.sh directly) is cleaner but means all Windows CI breaks if the patch fails to apply against a future NSS release. Consider separating the VS 2026 fix from the ARM64-specific changes so the blast radius is scoped.

Patch application order. The for p in *.patch glob is implicit. The patches are currently named to sort correctly, but this is fragile — an explicit list or naming convention would make the dependency ordering clearer and prevent accidental inclusion of non-ARM64 patches.

Outstanding blocking issues from prior reviews

The following issues were raised in earlier reviews and appear to still be present. I'm noting them here for visibility rather than duplicating inline comments:

1. renovate.json — removed matchStrings patterns (prior discussion): The pip == pattern (needed for gyp-next==0.22.2 tracking in both nss/action.yml and macOS deps) and the version+digest pattern were dropped when adding the Firefox raw-file pattern. Both should be restored alongside the new pattern.

2. nss-msvc-arm64.patch — sed CRT mapping (prior discussion): sed 's/20\([0-9]\).*/14\1/' produces incorrect vc_crt for VS 2019 (141 instead of 142) and VS 2022 (142 instead of 143). The VS 2026 explicit case is correct, but if the fallback is hit on a VS 2022 runner, WIN32_REDIST_DIR points to a non-existent Microsoft.VC142.CRT directory.

What looks good

• The nsinstall.py replacement with SHA256 verification and Renovate tracking via a Firefox custom datasource is a clean solution.
• The NSPR/NSS patches are technically sound — the _InterlockedExchangePointer spin-lock in ntmisc.c correctly implements the same acquire/release semantics as the x86 xchg path, and the various __aarch64__ → _M_ARM64 guard extensions are all correct for the LLP64/little-endian Windows ARM64 ABI.
• The cargo-fuzz source install (nightly toolchain compat) and cargo-careful source install (non-Linux binary availability) are well-motivated special cases.

[github-actions]

Well-structured PR. The 10 NSPR/NSS patches are individually documented with clear upstream-removal conditions, and the separation between the unconditional VS 2026 fix (nss-msvc-arm64.patch) and the ARM64-only patch set is clean. The crypto patches (AES, GHASH, SHA) correctly extend preprocessor guards for MSVC intrinsic compatibility, and the ntmisc.c spin-lock implementation using _InterlockedExchangePointer provides correct (and slightly stronger) ordering guarantees compared to the GCC/Clang path.

The nsinstall.py replacement with SHA256 verification and the conditional mingw64/bin exclusion on ARM64 are good improvements over the prior revision.

Blocking: The renovate.json matchStrings change inadvertently removes two existing patterns (the == pip pattern and the version+digest pair pattern), breaking Renovate tracking for gyp-next and cargo-binstall. See inline comment for the fix — the Firefox pattern should be added alongside the existing ones, not in place of them.

[github-actions]

Summary

Well-structured PR that adds Windows ARM64 support for the NSS/NSPR build pipeline and improves Rust tool installation. The 13 NSS/NSPR patch files are individually documented with clear "Removable once…" notes — a pragmatic approach given upstream doesn't yet support ARM64 MSVC.

Key changes reviewed:

• ilammy/msvc-dev-cmd arch parameter for ARM64 on both rust/action.yml and nss/action.yml
• Replacement of pacman -S nsinstall with a direct download + SHA256-verified nsinstall.py wrapper (applies to all Windows, not just ARM64)
• nss-msvc-arm64.patch: VS 2026 compatibility fix and ARM64 toolchain support in coreconf/msvc.sh
• ARM64-specific NSPR/NSS patches for interlocked intrinsics, CPU detection, GCM/GHASH/AES/SHA crypto paths, gyp configuration
• cargo-fuzz and cargo-careful source-build special-casing in rust/action.yml
• Renovate configuration for Firefox datasource tracking

Blocking issues (from prior reviews, still unresolved):

1. renovate.json: Two existing matchStrings patterns were removed instead of supplemented — breaks gyp-next Renovate tracking (see inline comment).
2. nss-msvc-arm64.patch: The sed-based CRT version derivation is incorrect for VS 2019/2022 (see inline comment).

Previously reported issues now resolved:

• nss-msvc-arm64.patch file now exists in nss/patches/ (was flagged as missing in an earlier revision)
• Debug echo statements have been removed

Architecture notes:

• The patch-glob approach (for p in patches/*.patch) is clean but ordering is locale-dependent and implicit. If patch ordering ever matters, an explicit list would be more robust.
• The -v flag added to build.sh applies to all platforms — consider gating it if it was only for ARM64 debugging.

[github-actions]

Review summary

This PR adds Windows ARM64 support for NSS builds via 14 out-of-tree patches, fixes tool installation for cargo-fuzz/cargo-careful, and wires up ilammy/msvc-dev-cmd for ARM64. The patch set is well-structured — each patch is clearly documented with upstream context and "removable once" notes, which is the right approach for carrying platform support ahead of upstream.

Resolved since prior review rounds

• nss-msvc-arm64.patch now exists in nss/patches/ (was missing in earlier iteration)
• Debug echo statements have been removed
• nsinstall.py fetch now uses --proto '=https' --tlsv1.2 and SHA256 verification
• Fragile sed-based inline patching of NSPR configure/Makefile replaced with proper .patch files
• NSS action.yml now has full ARM64 Windows support (was flagged as missing earlier)

Still-open blocking issues (from prior reviews — not re-raised inline)

1. renovate.json: two matchStrings patterns dropped — the pip == pattern (tracks gyp-next) and the version+digest pattern (tracks cargo-binstall as a version+SHA pair) must be restored. See inline comment with concrete suggestion.
2. nss-msvc-arm64.patch: sed CRT version mapping bug — s/20\([0-9]\).*/14\1/ produces wrong VC CRT directory names for VS 2019 (VC141 instead of VC142) and VS 2022 (VC142 instead of VC143). The VS 2026 (case 18) path is correct. This matters if the action runs on windows-2022 runners. Already raised in multiple prior threads.

Architectural notes

• Applying nss-msvc-arm64.patch unconditionally on all Windows is a deliberate coupling: VS 2026 compatibility for x64 runners now depends on this patch rather than the previous inline VSPATH/GYP_MSVS_VERSION workaround. This is cleaner but means the patch cannot be removed when ARM64 goes upstream without re-introducing the VS 2026 workaround. Worth documenting in the patch header.
• The for p in *.patch glob ordering is locale-dependent. Currently the patches touch disjoint files so order doesn't matter, but if future patches overlap, consider an explicit ordered list or numeric prefixes.
• cargo-fuzz intentionally omits --locked for nightly toolchain compatibility — this is correct and should have a brief inline comment so it doesn't get "fixed" later.

[github-actions]

Summary

Well-structured PR that adds Windows ARM64 support to both the Rust and NSS actions, with a clean patch architecture for upstream-pending NSPR/NSS changes. The patch files are individually documented with clear removal criteria, which is the right approach for carrying temporary platform support.

Resolved since prior reviews: debug echo statements removed, nss-msvc-arm64.patch now exists in the tree, nsinstall.py fetch is now properly secured (pinned release tag + SHA256 checksum + TLS hardening).

Blocking issues from prior reviews that remain open — not re-raising, just confirming they still apply to HEAD:

1. renovate.json — two matchStrings patterns removed (raised in multiple threads): the pip == pattern and the version+digest pattern were replaced by the Firefox raw-file pattern instead of supplemented. This breaks Renovate tracking for gyp-next (both macOS and Windows entries in nss/action.yml) and cargo-binstall (version+SHA in rust/action.yml). All three patterns should coexist.

2. nss-msvc-arm64.patch — sed CRT mapping is incorrect for VS 2019/2022 (raised in multiple threads): s/20\([0-9]\).*/14\1/ captures only the third character of the year string, producing VC141 for VS 2019 (should be VC142) and VC142 for VS 2022 (should be VC143). Since this patch is applied unconditionally on all Windows, this affects x64 builds on VS 2022 runners too. The VS 2026 explicit case arm is correct; only the * fallback is wrong.

3. mozfile is unpinned in nss/action.yml line 219, unlike gyp-next==0.22.2 on the line above.

Architectural note: Applying nss-msvc-arm64.patch unconditionally on all Windows is reasonable (it fixes VS 2026 compat broadly), but it means the sed bug in item 2 has a wider blast radius than the PR title suggests. Consider splitting the VS 2026 compat portion into its own patch if maintainability is a concern.

[github-actions]

Tip

Consider adding --fuzz=0 to both patch invocations (here and in the loop below) to enforce exact context matching. Default fuzz of 2 means patch can silently apply a hunk at the wrong offset if an upstream NSS/NSPR release shifts code by a few lines — a strict match is safer for automated CI where you'd rather fail loudly than patch the wrong location.

Suggested change

	patch -p1 < "$GITHUB_ACTION_PATH/patches/nss-msvc-arm64.patch"
	patch --fuzz=0 -p1 < "$GITHUB_ACTION_PATH/patches/nss-msvc-arm64.patch"

[github-actions]

Tip

Same --fuzz=0 suggestion here for consistency:

Suggested change

	patch -p1 < "$p"
	patch --fuzz=0 -p1 < "$p"

[github-actions]

Note

The ARM64 PR_StackPush and PR_StackPop spin-loops use a non-atomic *tos read as a fast-path check before the _InterlockedExchangePointer. On ARM64's weakly-ordered memory model, this read could observe a stale value (e.g. see -1 after the lock was already released), causing extra spins. This doesn't affect correctness — the subsequent interlocked exchange is the real synchronization point — but on high-contention workloads it could spin longer than necessary. A ReadAcquire((volatile LONG64*)tos) or __iso_volatile_load64 would make the fast-path read more timely, though it's a marginal improvement for code that is inherently a spin-lock.

[github-actions]

Note

The redirect list covers the five Interlocked functions NSPR uses (_MD_ATOMIC_* macros and direct calls in ntio.c/prmwait.c), but InterlockedExchangePointer is absent. This is correct — on 64-bit Windows, InterlockedExchangePointer is already defined as _InterlockedExchangePointer by the SDK headers, so no redirect is needed. Worth a brief comment in the patch header to explain the deliberate omission, since a future reader might think it was accidentally left out.