Skip to content

fix: use path.Join instead of filepath.Join for tar entry names#898

Merged
ericcurtin merged 2 commits intomainfrom
fix/tarball-forward-slash-paths
May 1, 2026
Merged

fix: use path.Join instead of filepath.Join for tar entry names#898
ericcurtin merged 2 commits intomainfrom
fix/tarball-forward-slash-paths

Conversation

@ilopezluna
Copy link
Copy Markdown
Contributor

Problem

On Windows, docker model package --from <model> --chat-template <file> fails with:

missing blob sha256:... for manifest

Root Cause

tarball/target.go used filepath.Join to build tar entry names. On Windows, this produces backslash-separated paths (e.g., blobs\sha256\hex). When the daemon (Linux) reads the tarball, the reader splits entry names on / and skips entries with backslashes, so all blobs are silently ignored. The manifest then references blobs that were never stored → "missing blob".

This only affects code paths that go through the tarball (i.e., when canUseDaemonRepackage is false): --chat-template, --license, --mmproj, --gguf, --safetensors-dir, --dduf, etc. The simple --from --context-size case uses a direct daemon API and was never affected.

Fix

Replace filepath.Join with path.Join (package path, not path/filepath) in tarball/target.go. path.Join always uses forward slashes regardless of the OS.

Test

Added TestTargetEntryNamesUseForwardSlashes which builds a model with GGUF + chat template, writes it to a tarball, and verifies no entry names contain backslashes.

Fixes #894

On Windows, filepath.Join produces backslash-separated paths
(e.g., blobs\sha256\hex). When the CLI packages a model into a
tarball and sends it to the daemon, the reader splits entry names
on '/' and skips entries with backslashes, causing 'missing blob'
errors.

Replace filepath.Join with path.Join in tarball Target so entry
names always use forward slashes regardless of the OS.

Fixes #894
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request replaces the use of path/filepath with path for tar entry names to ensure consistent forward-slash separators across different operating systems, addressing a regression where backslashes were used on Windows. It also includes a new regression test and renames a parameter to avoid shadowing the path package. Feedback was provided to explicitly set the file mode to 0755 for directory entries in the tar archive to avoid permission issues during extraction.

Comment thread pkg/distribution/tarball/target.go
Address review feedback: set an explicit file mode on directory
entries so they are not created with zero permissions if the tar
is ever extracted to disk.
Copy link
Copy Markdown
Contributor

@ericcurtin ericcurtin left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @ilopezluna

@ericcurtin ericcurtin merged commit a21f237 into main May 1, 2026
14 checks passed
@ericcurtin ericcurtin deleted the fix/tarball-forward-slash-paths branch May 1, 2026 12:20
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.

Unable to get docker model package to package with --chat-template

2 participants