Skip to content

chore: replace chocolatey with native binaries#6639

Open
mihaibuzgau wants to merge 1 commit intomainfrom
chore/cli-1379_useNativeWinDeps
Open

chore: replace chocolatey with native binaries#6639
mihaibuzgau wants to merge 1 commit intomainfrom
chore/cli-1379_useNativeWinDeps

Conversation

@mihaibuzgau
Copy link
Contributor

@mihaibuzgau mihaibuzgau commented Mar 12, 2026

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages
    are release-note ready, emphasizing
    what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

  • Migrates Windows CI from Chocolatey to native installers for Node.js, Python 3.12.8, .NET SDK 8, Maven, Gradle, and GNU Make, each with pinned versions and SHA256 verification.
  • Centralizes Windows PATH management via C:\tools-cache\snyk-env.ps1, making installed tools consistently available across CircleCI steps and jobs.

Where should the reviewer start?

  • .circleci/config.yml
  • .circleci/windows/*.ps1

How should this be manually tested?

In CircleCI, run the full Windows pipeline on a test branch:

  • Confirm build windows amd64 and sign windows amd64 pass and produce expected artifacts.
  • Confirm acceptance-tests windows amd64 pass, including SBOM tests that rely on Maven and Python.

What's the product update that needs to be communicated to CLI users?

There is no functional CLI behavior change for end users; this PR only changes how Windows CI builds and tests the CLI.

@mihaibuzgau mihaibuzgau requested review from a team as code owners March 12, 2026 15:22
@mihaibuzgau mihaibuzgau marked this pull request as draft March 12, 2026 15:23
@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch 3 times, most recently from 821cecc to bdef5ed Compare March 12, 2026 16:31
@snyk-io
Copy link

snyk-io bot commented Mar 12, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch 2 times, most recently from cdd8b29 to 69056fa Compare March 16, 2026 08:44
@mihaibuzgau
Copy link
Contributor Author

/review

@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau
Copy link
Contributor Author

/review

@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from 110e69f to 765b462 Compare March 16, 2026 12:40
@mihaibuzgau
Copy link
Contributor Author

/review

@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from 765b462 to 42b39b5 Compare March 16, 2026 12:56
@mihaibuzgau
Copy link
Contributor Author

/review

@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from 42b39b5 to aa336dc Compare March 16, 2026 13:10
@mihaibuzgau
Copy link
Contributor Author

/review

@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from aa336dc to 9d7b8ac Compare March 16, 2026 13:26
@mihaibuzgau
Copy link
Contributor Author

/review

@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch 4 times, most recently from 48e9b8d to 87bc5f9 Compare March 16, 2026 15:32
@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from 87bc5f9 to e3ce3eb Compare March 16, 2026 16:34
@mihaibuzgau
Copy link
Contributor Author

/review

@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from e3ce3eb to 267dab2 Compare March 17, 2026 07:52
@mihaibuzgau
Copy link
Contributor Author

/review

@snyk-pr-review-bot

This comment has been minimized.

@mihaibuzgau mihaibuzgau force-pushed the chore/cli-1379_useNativeWinDeps branch from 267dab2 to f606343 Compare March 17, 2026 08:50
@mihaibuzgau mihaibuzgau marked this pull request as ready for review March 17, 2026 08:50
@snyk-pr-review-bot
Copy link

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential Path Mismatch 🟠 [major]

The script hardcodes several candidate directories for Python 3.12 (e.g., C:\Program Files\Python312). However, the installer version is set to 3.12.8. If the native Python installer changes its default installation path pattern in future 3.12.x releases, or if it installs to a version-specific subdirectory not covered by the candidateDirs list, the script will fail to find the installation and will not update the snyk-env.ps1 script, leading to missing python commands in subsequent CI steps.

$candidateDirs = @(
  "C:\Program Files\Python312",
  "C:\Python312",
  (Join-Path $Env:LOCALAPPDATA 'Programs\Python\Python312')
)
Ambiguous File Resolution 🟠 [major]

The search logic for make.exe uses Get-ChildItem -Recurse and throws an error if more than one make.exe is found. If the downloaded archive contains multiple copies of the binary (e.g., in different architecture folders or debug/release folders) or if a previous failed run left artifacts behind, the build will fail with an "Ambiguous make.exe location" error.

if ($candidates.Count -gt 1) {
  $paths = ($candidates | Select-Object -ExpandProperty FullName) -join ', '
  throw "Ambiguous make.exe location after extraction; found multiple candidates: $paths"
}
Version Coupling Risk 🟡 [minor]

The expectedNodeVersion is hardcoded to 20.11.1 and the script throws an error if it doesn't match .nvmrc. This creates a maintenance burden where developers must remember to update this PowerShell script every time the Node version is bumped in .nvmrc, otherwise the Windows CI will fail immediately.

$expectedNodeVersion = '20.11.1'
$expectedSha256 = 'c54f5f7e2416e826fd84e878f28e3b53363ae9c3f60a140af4434b2453b5ae89'

# Resolve repo root from script location (script is in .circleci/windows)
$repoRoot = Resolve-Path (Join-Path $PSScriptRoot '..\..')
$nvmrcPath = Join-Path $repoRoot '.nvmrc'

if (-not (Test-Path $nvmrcPath)) {
  throw ".nvmrc not found at $nvmrcPath"
}

$nvmVersion = (Get-Content $nvmrcPath -Raw).Trim()
if ([string]::IsNullOrWhiteSpace($nvmVersion)) {
  throw ".nvmrc is empty at $nvmrcPath"
}

if ($nvmVersion -ne $expectedNodeVersion) {
  throw ".nvmrc version '$nvmVersion' does not match expected Node.js version '$expectedNodeVersion' used by Windows CI."
}
📚 Repository Context Analyzed

This review considered 23 relevant code sections from 14 files (average relevance: 0.85)

CC: << parameters.c_compiler >>
MACOSX_DEPLOYMENT_TARGET: 13.0
command: make << parameters.make_target >> GOOS=<< parameters.go_target_os >> GOARCH=<< parameters.go_arch >> STATIC_NODE_BINARY=true CGO_ENABLED=0
# Windows static build (use PowerShell and ensure GNU make is on PATH)
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: This block adds quite some duplication which is difficult to maintain just to load environment variables, is there a way to avoid this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: maybe re-use this approach.

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