Skip to content

import: Handle small files#42150

Merged
bluca merged 2 commits into
systemd:mainfrom
pothos:pull-small
May 18, 2026
Merged

import: Handle small files#42150
bluca merged 2 commits into
systemd:mainfrom
pothos:pull-small

Conversation

@pothos
Copy link
Copy Markdown
Contributor

@pothos pothos commented May 18, 2026

When systemd-pull encountered a file shorter than the compression magic
headers it looks for, then it would complete the download in the
analysis state and fail.
When we are still in the analysis state and the download is done, we
know there is no compression and we should leave the analysis state and
continue writing out to disk as usual.

To call into pull_job_write_(un)compressed and pull_job_open_disk from
pull_job_curl_on_finished it has to be defined after them. This is in
preparation for a bug fix for small files where we need to leave the
compression analysis state to finish the download successfully.
Comment thread src/import/pull-job.c
return j->content_length;
}

static int pull_job_write_uncompressed(const void *p, size_t sz, void *userdata) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

FYI: This and the other two functions are just moved. It's a separate commit.
The only new function is pull_job_begin_running.

@github-actions
Copy link
Copy Markdown

ghost commented May 18, 2026

Claude review of PR #42150 (5f299e5)

Nits

  • Redundant assert (dismissed) — src/import/pull-job.c:411assert(result == CURLE_OK) is always true since the error case returns early above
  • Test artifact left behindtest/units/TEST-72-SYSUPDATE.sh:576 — SHA256SUMS overwritten with tiny-file-only content and not restored

Workflow run

Comment thread src/import/pull-job.c
if (j->state == PULL_JOB_ANALYZING) {
/* When curl finished the download while we were still looking for a compression magic
* header the content isn't compressed but should be written out as is. */
assert(result == CURLE_OK);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude: nit: assert(result == CURLE_OK) is always true here because the result != CURLE_OK case returns early at line 358. The assert is redundant, though it does document the invariant.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's pretty much the point of an assert

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This could be extended to also cover the http code check that was done before but that would require capturing this in a variable that does not exist yet and I think it's already good enough to indicate that the download was successful here from the checks above.

Comment thread test/units/TEST-72-SYSUPDATE.sh
When systemd-pull encountered a file shorter than the compression magic
headers it looks for, then it would complete the download in the
analysis state and fail.
When we are still in the analysis state and the download is done, we
know there is no compression and we should leave the analysis state and
continue writing out to disk as usual.
@poettering poettering added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels May 18, 2026
@bluca bluca merged commit 82b47c7 into systemd:main May 18, 2026
68 checks passed
@pothos pothos deleted the pull-small branch May 19, 2026 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants