Skip to content

replace github.com/golang/snappy with klauspost/compress/snappy#13048

Merged
williammartin merged 1 commit into
cli:trunkfrom
thaJeztah:snappier
Mar 31, 2026
Merged

replace github.com/golang/snappy with klauspost/compress/snappy#13048
williammartin merged 1 commit into
cli:trunkfrom
thaJeztah:snappier

Conversation

@thaJeztah

Copy link
Copy Markdown
Contributor

The github.com/golang/snappy repository was archived and is no longer maintained. klauspost/compress provides a drop-in replacement, which is actively maintained, and the klauspost/compress module is already an existing (indirect) dependency.

cc @babakks

The github.com/golang/snappy repository was archived and is no longer
maintained. klauspost/compress provides a drop-in replacement, which
is actively maintained, and the klauspost/compress module is already
an existing (indirect) dependency.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah requested a review from a team as a code owner March 26, 2026 22:02
Copilot AI review requested due to automatic review settings March 26, 2026 22:02
@thaJeztah thaJeztah requested a review from a team as a code owner March 26, 2026 22:02
@thaJeztah thaJeztah requested a review from williammartin March 26, 2026 22:02
@github-actions github-actions Bot added external pull request originating outside of the CLI core team needs-triage needs to be reviewed unmet-requirements labels Mar 26, 2026
@github-actions

Copy link
Copy Markdown

Thanks for your pull request! Unfortunately, it doesn't meet the minimum requirements for review:

  • None of the referenced issues have the help wanted label

Please update your PR to address the above. Requirements:

  1. Include a detailed description of what this PR does
  2. Link to an issue with the help wanted label (use Fixes #123 or Closes #123 if it resolves the issue)

This PR will be automatically closed in 7 days if these requirements are not met.

@github-actions github-actions Bot removed the needs-triage needs to be reviewed label Mar 26, 2026

Copilot AI 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.

Pull request overview

Replaces the archived github.com/golang/snappy dependency with the actively maintained drop-in replacement github.com/klauspost/compress/snappy.

Changes:

  • Swapped Snappy imports in attestation API code and tests to github.com/klauspost/compress/snappy.
  • Updated go.mod to remove github.com/golang/snappy and add github.com/klauspost/compress as a direct dependency (previously indirect).
  • Cleaned up go.sum to drop the removed github.com/golang/snappy checksums.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
pkg/cmd/attestation/api/client.go Updates Snappy import used for bundle decompression.
pkg/cmd/attestation/api/mock_httpClient_test.go Updates Snappy import used to generate compressed test responses.
go.mod Removes archived module and promotes klauspost/compress to a direct requirement.
go.sum Removes checksums for the dropped github.com/golang/snappy module.

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

@thaJeztah

Copy link
Copy Markdown
Contributor Author

None of the referenced issues have the help wanted label

🙈 probably should've read contributing.md

Let me know if this is still acceptable, without a related ticket @williammartin 😅

@andyfeller

Copy link
Copy Markdown
Contributor

@malancas : since this is changing an underlying dependency of attestation feature set, I thought you and the Package Security team would want to review this.

@kommendorkapten

Copy link
Copy Markdown
Contributor

The change looks solid.

@malancas do you know if we have any unit tests that exercises snappy decompress of attestations prior to verification?

@malancas

Copy link
Copy Markdown
Contributor

The change looks solid.

@malancas do you know if we have any unit tests that exercises snappy decompress of attestations prior to verification?

The decompression functionality is tested before verification through tests within trunk/pkg/cmd/attestation/api/client_test.go that call LiveClient#getBundle. We could add more assert statements around the returned bundles.

sungdark pushed a commit to sungdark/claude-builders-bounty that referenced this pull request Mar 27, 2026
CLI: claude-review --pr <URL>
GitHub Action: .github/workflows/review.yml for automated PR reviews
Structured Markdown output: Summary / Risks / Suggestions / Confidence Score
Tested on 2 real GitHub PRs (cli/cli#13046, cli/cli#13048)

Payment address: eB51DWp1uECrLZRLsE2cnyZUzfRWvzUzaJzkatTpQV9
@thaJeztah

thaJeztah commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for looking! If there were any reasons for not switching that I'm not aware of; happy to close (and no harm done).

I noticed the dependency on an archived repo when I was working on buildx; archived doesn't always mean "bad"; some code is just "complete and stable", but then found that @klauspost offered a drop-in replacement; I know his module is well maintained and widely used (❤️❤️❤️), so switching seemed like a logical choice. Of course assuming all fully compatible.

@malancas

Copy link
Copy Markdown
Contributor

Thanks for looking! If there were any reasons for not switching that I'm not aware of; happy to close (and no harm done).

I noticed the dependency on an archived repo when I was working on buildx; archived doesn't always mean "bad"; some code is just "complete and stable", but then found that @klauspost offered a drop-in replacement; I know his module is well maintained and widely used (❤️❤️❤️), so switching seemed like a logical choice. Of course assuming all fully compatible.

Switching to the actively maintained dependency makes sense to me. These changes LGTM after reviewing the current code and testing. I'll wait until Monday in case @kommendorkapten or @andyfeller had any other thoughts but thank you for opening the pull request

@thaJeztah

Copy link
Copy Markdown
Contributor Author

For sure, no rush, enjoy the weekend!!

In case relevant; there is a newer patch-release of this module; I think the patch fixes a bug in the zstd implementation, and no changes in snappy, so I didn't want to conflate things with my change (but can update in this PR if you prefer).

Owenrh0482

This comment was marked as spam.

@kommendorkapten kommendorkapten 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.

Thanks!

@babakks

babakks commented Mar 30, 2026

Copy link
Copy Markdown
Member

Thanks for the PR, @thaJeztah! 🙏

Great to see the changes are transparent, but I need to callout there maybe a bit of concern around the license file in github.com/klauspost/compress. The repo seems to have both BSD-3 and Apache 2 licenses, concatenated in a single file. I need to check if our license extraction script is happy with it.

So, please don't merge this until I've confirmed it's safe to do so.

@babakks

babakks commented Mar 30, 2026

Copy link
Copy Markdown
Member

Tl;DR is we're good. We just print a few more licenses (two) that we don't really need to embed.

As for the details, these are the extracted licenses by running ./script/licenses linux amd64 (all other supported os/arch result in the same output as I checked):

internal/licenses/embed/linux-amd64/third-party/github.com/klauspost/compress/
├── LICENSE                            (BSD-3 + Apache 2.0 + MIT, total 5 licenses)
├── internal/snapref/LICENSE           (BSD-3)
├── s2/LICENSE                         (BSD-3)
├── snappy/LICENSE                     (BSD-3)
└── zstd/internal/xxhash/LICENSE.txt   (MIT)

The extracted licenses belong to the packages we need to build gh (so not necessarily all packages/licenses in the new module end up here). And this is the summary part of the gh licenses output (the top part):

github.com/klauspost/compress (v1.18.4) - MIT - https://github.com/klauspost/compress/blob/v1.18.4/LICENSE
github.com/klauspost/compress (v1.18.4) - Apache-2.0 - https://github.com/klauspost/compress/blob/v1.18.4/LICENSE
github.com/klauspost/compress (v1.18.4) - BSD-3-Clause - https://github.com/klauspost/compress/blob/v1.18.4/LICENSE
github.com/klauspost/compress/internal/snapref (v1.18.4) - BSD-3-Clause - https://github.com/klauspost/compress/blob/v1.18.4/internal/snapref/LICENSE
github.com/klauspost/compress/s2 (v1.18.4) - BSD-3-Clause - https://github.com/klauspost/compress/blob/v1.18.4/s2/LICENSE
github.com/klauspost/compress/snappy (v1.18.4) - BSD-3-Clause - https://github.com/klauspost/compress/blob/v1.18.4/snappy/LICENSE
github.com/klauspost/compress/zstd/internal/xxhash (v1.18.4) - MIT - https://github.com/klauspost/compress/blob/v1.18.4/zstd/internal/xxhash/LICENSE.txt

gh licenses prints the content of the license files discovered (above list). Since the the license file at the root is a mixed one and we print it all, two unused MIT licenses will also be printed (associated with paths s2/cmd/internal/filepathx/* and s2/cmd/internal/readahead/*). They're unused because we don't need those packages (under s2/cmd/) to build gh.

I think it's fine, but I'd like to have @williammartin confirmation on it as he's our licenses hero. 🦸

@williammartin williammartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Image

It does bear a striking resemblance to me.

If the tests pass and there's nothing obviously concerning, I'm generally fine with dependency changes, otherwise we'll spend forever doing manual validation in response to dependabot. If something breaks, lesson learned!

No worries about the multi-licenses if they are MIT/BSD/Apache permissive licenses. Our license attribution is best-effort, the ecosystem makes it hard to do better. If someone takes an issue then we would try to work something out.

Thanks for the PR.

@williammartin williammartin merged commit 40da058 into cli:trunk Mar 31, 2026
21 of 22 checks passed
@thaJeztah

Copy link
Copy Markdown
Contributor Author

😂 licensing is always tricky! You try to do the right thing, but it's complicated to navigate all the intricate details 😅 - thanks everyone!

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request May 7, 2026
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cli/cli](https://github.com/cli/cli) | minor | `v2.89.0` → `v2.90.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>cli/cli (cli/cli)</summary>

### [`v2.90.0`](https://github.com/cli/cli/releases/tag/v2.90.0): GitHub CLI 2.90.0

[Compare Source](cli/cli@v2.89.0...v2.90.0)

#### Manage agent skills with `gh skill` (Public Preview)

[Agent skills](https://agentskills.io) are portable sets of instructions, scripts, and resources that teach AI coding agents how to perform specific tasks. The new `gh skill` command makes it easy to discover, install, manage, and publish agent skills from GitHub repositories - right from the CLI.

```

# Discover skills
gh skill search copilot

# Preview a skill without installing
gh skill preview github/awesome-copilot documentation-writer

# Install a skill
gh skill install github/awesome-copilot documentation-writer

# Pin to a specific version
gh skill install github/awesome-copilot documentation-writer --pin v1.2.0

# Check installed skills for updates
gh skill update --all

# Validate and publish your own skills
gh skill publish --dry-run
```

Skills are automatically installed to the correct directory for your agent host. `gh skill` supports GitHub Copilot, Claude Code, Cursor, Codex, Gemini CLI, and Antigravity. Target a specific agent and scope with `--agent` and `--scope` flags.

`gh skill publish` validates skills against the [Agent Skills specification](https://agentskills.io/specification) and checks remote settings like tag protection and immutable releases to improve supply chain security.

Read the full announcement on the [GitHub Blog](https://github.blog/changelog/2026-04-16-manage-agent-skills-with-github-cli/).

`gh skill` is launching in public preview and is subject to change without notice.

#### Official extension suggestions

When you run a command that matches a known official extension that isn't installed (e.g. `gh stack`), the CLI now offers to install it instead of showing a generic "unknown command" error.

This feature is available for [github/gh-aw](https://github.com/github/gh-aw) and [github/gh-stack](https://github.com/github/gh-stack).

When possible, you'll be prompted to install immediately. When prompting isn't possible, the CLI prints the `gh extension install` command to run.

#### `gh extension install` no longer requires authentication

`gh extension install` previously required a valid auth token even though it only needs to download a public release asset. The auth check has been removed, so you can install extensions without being logged in.

#### What's Changed

##### ✨ Features

- Add `gh skill` command group: install, preview, search, update, publish by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13165](cli/cli#13165)
- Suggest and install official extensions for unknown commands by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13175](cli/cli#13175)
- `gh skill publish`: auto-push unpushed commits before publish by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13171](cli/cli#13171)
- Disable auth check for `gh extension install` by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13176](cli/cli#13176)

##### 🐛 Fixes

- Fix infinite loop in `gh release list --limit 0` by [@&#8203;Bahtya](https://github.com/Bahtya) in [#&#8203;13097](cli/cli#13097)
- Ensure `api` and `auth` commands record agentic invocations by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;13046](cli/cli#13046)
- Disable auth check for local-only skill flags by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13173](cli/cli#13173)
- URL-encode parentPath in skills discovery API call by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13172](cli/cli#13172)
- Fix: use target directory remotes in skills publish by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13169](cli/cli#13169)
- Fix: preserve namespace in skills search deduplication by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13170](cli/cli#13170)

##### 📚 Docs & Chores

- docs: include PGP key fingerprints by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13112](cli/cli#13112)
- docs: add sha/md5 checksums of keyring files by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13150](cli/cli#13150)
- docs: fix SHA512 checksum for GPG key by [@&#8203;timsu92](https://github.com/timsu92) in [#&#8203;13157](cli/cli#13157)
- docs(skill): polish skill commandset docs by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13183](cli/cli#13183)
- Document dependency CVE policy in SECURITY.md by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13119](cli/cli#13119)
- Replace github.com/golang/snappy with klauspost/compress/snappy by [@&#8203;thaJeztah](https://github.com/thaJeztah) in [#&#8203;13048](cli/cli#13048)
- chore: bump to go1.26.2 by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13116](cli/cli#13116)
- chore: delete experimental script/debian-devel by [@&#8203;babakks](https://github.com/babakks) in [#&#8203;13127](cli/cli#13127)
- Suggest first party extensions by [@&#8203;williammartin](https://github.com/williammartin) in [#&#8203;13182](cli/cli#13182)
- Add cli/skill-reviewers as CODEOWNERS for skills packages by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13189](cli/cli#13189)
- Add [@&#8203;cli/code-reviewers](https://github.com/cli/code-reviewers) to all CODEOWNERS rules by [@&#8203;BagToad](https://github.com/BagToad) in [#&#8203;13190](cli/cli#13190)
- Address post-merge review feedback for skills commands by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13185](cli/cli#13185)
- Fix skills-publish-dry-run acceptance test error message mismatch by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13187](cli/cli#13187)
- Skills: replace real git in publish tests with CommandStubber by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13188](cli/cli#13188)
- Remove redundant nil-client fallback in skills publish by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13168](cli/cli#13168)
- Publish: use shared discovery logic instead of requiring skills/ directory by [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) in [#&#8203;13167](cli/cli#13167)

##### :dependabot: Dependencies

- chore(deps): bump github.com/klauspost/compress from 1.18.4 to 1.18.5 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13071](cli/cli#13071)
- chore(deps): bump github.com/yuin/goldmark from 1.7.16 to 1.8.2 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13045](cli/cli#13045)
- chore(deps): bump charm.land/bubbles/v2 from 2.0.0 to 2.1.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13051](cli/cli#13051)
- chore(deps): bump github.com/sigstore/timestamp-authority/v2 from 2.0.3 to 2.0.6 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13152](cli/cli#13152)
- chore(deps): bump github.com/google/go-containerregistry from 0.21.3 to 0.21.4 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13129](cli/cli#13129)
- chore(deps): bump github.com/sigstore/protobuf-specs from 0.5.0 to 0.5.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13128](cli/cli#13128)
- chore(deps): bump github.com/in-toto/attestation from 1.1.2 to 1.2.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13044](cli/cli#13044)
- chore(deps): bump advanced-security/filter-sarif from 1.0.1 to 1.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;12918](cli/cli#12918)
- chore(deps): bump google.golang.org/grpc from 1.79.3 to 1.80.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13076](cli/cli#13076)
- chore(deps): bump github.com/hashicorp/go-version from 1.8.0 to 1.9.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;13065](cli/cli#13065)

#### New Contributors

- [@&#8203;thaJeztah](https://github.com/thaJeztah) made their first contribution in [#&#8203;13048](cli/cli#13048)
- [@&#8203;Bahtya](https://github.com/Bahtya) made their first contribution in [#&#8203;13097](cli/cli#13097)
- [@&#8203;timsu92](https://github.com/timsu92) made their first contribution in [#&#8203;13157](cli/cli#13157)
- [@&#8203;SamMorrowDrums](https://github.com/SamMorrowDrums) made their first contribution in [#&#8203;13173](cli/cli#13173)

**Full Changelog**: <cli/cli@v2.89.0...v2.90.0>

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMjMuNiIsInVwZGF0ZWRJblZlciI6IjQzLjEyMy42IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external pull request originating outside of the CLI core team unmet-requirements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants