Skip to content

Fix self-update: download tarball and extract binary#198

Merged
sdairs merged 2 commits into
ClickHouse:mainfrom
graphaelli:fix-update-tarball-extraction
May 19, 2026
Merged

Fix self-update: download tarball and extract binary#198
sdairs merged 2 commits into
ClickHouse:mainfrom
graphaelli:fix-update-tarball-extraction

Conversation

@graphaelli
Copy link
Copy Markdown
Member

The release workflow uploads clickhousectl-{target}-v{version}.tar.gz archives, but update.rs looked for a raw binary asset named clickhousectl-{target} and wrote the response bytes directly to disk. This meant chctl update always failed with "No compatible binary found for this platform", and would have clobbered the running binary with a gzipped tarball if the name had matched.

Build the asset name from release.tag_name to match the workflow, then stream the download through GzDecoder + tar::Archive and pull out the clickhousectl entry before the atomic rename.

closes #196

The release workflow uploads `clickhousectl-{target}-v{version}.tar.gz`
archives, but `update.rs` looked for a raw binary asset named
`clickhousectl-{target}` and wrote the response bytes directly to disk.
This meant `chctl update` always failed with "No compatible binary found
for this platform", and would have clobbered the running binary with a
gzipped tarball if the name had matched.

Build the asset name from `release.tag_name` to match the workflow, then
stream the download through `GzDecoder` + `tar::Archive` and pull out
the `clickhousectl` entry before the atomic rename.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 19, 2026

CLA assistant check
All committers have signed the CLA.

@graphaelli graphaelli marked this pull request as ready for review May 19, 2026 14:50
@sdairs
Copy link
Copy Markdown
Collaborator

sdairs commented May 19, 2026

Nice fix — diagnosis and approach match what I had in #199 (which I've now closed in favour of this), and your per-step error messages + negative test are better than what I wrote.

One small suggestion: the four Error::Download(...) wraps inside extract_binary_from_archive are semantically about extraction, not download (the download has already succeeded by the time we get here). Error::Extract already exists in error.rs for exactly this case — swapping to it would give users a clearer "Extraction failed: …" prefix when an archive is malformed. Pure polish, doesn't block.

For context: I'll open a separate small PR on top of this with the version bumps (clickhousectl, clickhouse-cloud-api, the dep ref between them, and npm/package.json to 0.2.2 in lockstep) and a CLAUDE.md note that releases need all three bumped together now. Keeping it separate so this fix can land cleanly.

@graphaelli
Copy link
Copy Markdown
Member Author

good call, i can change those real quick

The download has already succeeded by the time we enter
extract_binary_from_archive, so failures here are about parsing the
tarball, not fetching it. Error::Extract already exists in error.rs
with an "Extraction failed: ..." prefix that surfaces this clearly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sdairs sdairs merged commit 43b872b into ClickHouse:main May 19, 2026
1 check passed
graphaelli pushed a commit to graphaelli/clickhousectl that referenced this pull request May 19, 2026
Bumps clickhousectl, clickhouse-cloud-api (and the dep reference
between them), and npm/package.json to 0.2.2 in lockstep ready for
release. CLAUDE.md now reflects this as the policy: previously it said
clickhouse-cloud-api should only be bumped when its source changed and
npm/package.json was bumped automatically by the workflow. Going
forward, all three bump together at release time so the source-of-
truth matches the tag.

Stacked on ClickHouse#198, which contains the actual `chctl update` fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

darwin/arm64 chctl update failing

3 participants