Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive Chocolatey package support for slcli, enabling Windows users to install the CLI tool through the Chocolatey package manager. The implementation follows a remote download model where the package downloads the binary from GitHub releases rather than embedding it directly.
- Chocolatey packaging infrastructure with automated build and publish workflow
- Remote artifact download model with SHA256 integrity verification
- CI/CD integration for automated package publishing on tagged releases
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/build_chocolatey.py | Automates Chocolatey package creation with checksum injection and token replacement |
| packaging/choco/slcli.nuspec | Package metadata specification for Chocolatey |
| packaging/choco/tools/chocolateyinstall.ps1 | PowerShell script for downloading and installing slcli from GitHub releases |
| packaging/choco/tools/chocolateyuninstall.ps1 | Minimal uninstall script for Chocolatey |
| README.md | Added Chocolatey installation instructions and usage notes |
| .github/workflows/release.yml | Extended CI workflow with Chocolatey build and publish jobs |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| # '$version$' is a build-time token that should be replaced during packaging. | ||
| if ($env:ChocolateyPackageVersion) { $version = $env:ChocolateyPackageVersion } else { $version = '$version$' } | ||
| if ($version -eq '$version$') { | ||
| throw "The package version could not be determined. Ensure that the build-time token `\$version\$` is replaced or the ChocolateyPackageVersion environment variable is set." |
There was a problem hiding this comment.
In PowerShell, the proper way to escape a dollar sign in a double-quoted string is with a backtick, not a backslash. The current usage of \$version\$ should be written as `$version`$ to correctly display the literal text "$version$" in the error message. While the current code may work, it doesn't follow PowerShell conventions and could be confusing.
| throw "The package version could not be determined. Ensure that the build-time token `\$version\$` is replaced or the ChocolateyPackageVersion environment variable is set." | |
| throw "The package version could not be determined. Ensure that the build-time token `$version$` is replaced or the ChocolateyPackageVersion environment variable is set." |
| print("Warning: checksum token not found in install script", file=sys.stderr) | ||
| return |
There was a problem hiding this comment.
The checksum token is mandatory for Chocolatey package security (as mentioned in the PR description). If the $checksum$ token is not found in the install script, the function should exit with an error rather than just printing a warning and continuing. This would prevent accidentally packaging an install script without checksum verification, which would be a security issue.
| print("Warning: checksum token not found in install script", file=sys.stderr) | |
| return | |
| print("✗ Error: checksum token ($checksum$) not found in install script. Aborting build.", file=sys.stderr) | |
| sys.exit(1) |
| Path to prepared nuspec | ||
| """ | ||
| target = work_dir / "slcli.nuspec" | ||
| content = NUSPEC.read_text(encoding="utf-8").replace("$version$", version) |
There was a problem hiding this comment.
The function should verify that the $version$ token exists in the nuspec file before performing the replacement. If the token is missing, the package would be created with an invalid version, which could cause issues during publication or installation. Consider adding a check similar to the one in inject_checksum (but as an error rather than a warning).
| content = NUSPEC.read_text(encoding="utf-8").replace("$version$", version) | |
| content = NUSPEC.read_text(encoding="utf-8") | |
| if "$version$" not in content: | |
| print("✗ Error: $version$ token not found in nuspec template.", file=sys.stderr) | |
| sys.exit(1) | |
| content = content.replace("$version$", version) |
Added
dist/slcli.zip, injection into install script, andchoco pack.Modified
choco-buildjob (build + pack + upload) andpublish-chocolateyjob (push on tagged release with API key). Included.nupkgin release assets.*.nupkg.Workflow Details
dist/slcli.zip→ checksum calculated → checksum injected →.nupkgpacked → artifact uploaded → optional publish job pushes after release tag.CHOCOLATEY_API_KEYsecret; only runs on version tags (v*).Security & Compliance
.nupkg(download model).Testing Guidance
poetry run python scripts/build_chocolatey.pythenchoco install slcli -s dist -y.$checksum$absence in packed.nupkg.Risk
Low; isolated to packaging, docs, and CI additions—runtime CLI unaffected.