close
Skip to content

Fix skeleton update format fallback and sdist URL generation#384

Merged
agriyakhetarpal merged 3 commits into
pyodide:mainfrom
henryiii:fix/skeleton-fallback-and-urls
Jun 30, 2026
Merged

Fix skeleton update format fallback and sdist URL generation#384
agriyakhetarpal merged 3 commits into
pyodide:mainfrom
henryiii:fix/skeleton-fallback-and-urls

Conversation

@henryiii

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Part of #376

Bug 1 — dead-code format fallback in update_package

source_fmt = source_fmt or old_fmt made source_fmt always truthy, so the fallback branches ["wheel","sdist"] / ["sdist","wheel"] were unreachable. A recipe using an sdist whose new release ships only wheels raised MkpkgFailedException instead of transparently updating to the wheel (and vice versa). Format resolution now happens after the version check and restores the intended semantics: explicit source_fmt is strict, absent source_fmt prefers the old format with fallback to the other.

Bug 2 — sdist predictable URL ignores actual filename

The predictable-URL generation always produced {normalized_name}-{version}.tar.gz for sdists, ignoring the actual filename from PyPI metadata. .zip sdists and packages with non-normalized filenames (e.g. ruamel.yaml) got URLs that 404 while the stored sha256 is from the real file, producing unbuildable recipes. The actual filename from the PyPI metadata is now used.

Tests

Seven new tests: sdist→wheel and wheel→sdist fallback, strict explicit source_fmt, three parametrized predictable-URL cases (.tar.gz, .zip, non-normalized names), and an end-to-end .zip sdist URL check. Full unit suite passes (4 pre-existing failures on main, unrelated).

henryiii added 2 commits June 12, 2026 16:02
…eration

Bug 1: In update_package, `source_fmt = source_fmt or old_fmt` made source_fmt
always truthy, rendering the format-fallback branches (["wheel","sdist"] /
["sdist","wheel"]) dead code. A recipe using an sdist whose new release ships
only wheels would raise MkpkgFailedException instead of falling back to the
wheel. Fixed by moving format resolution after the version check and restoring
the intended semantics: explicit source_fmt is strict; absent source_fmt prefers
the old format with fallback to the other.

Bug 2: _make_predictable_url for sdists always produced a URL ending in
{normalized_name}-{version}.tar.gz, ignoring the actual filename. .zip sdists
and packages with non-normalized filenames (e.g. ruamel.yaml) got broken URLs
that 404 while the stored sha256 is from the real file. Fixed by using the
actual filename in the generated URL.

Also updates the pre-existing test expectation for scikit-learn sdist URL to
reflect the correct behavior (real filename uses dash, not underscore).

Assisted-by: ClaudeCode:claude-sonnet-4-6
Assisted-by: ClaudeCode:claude-fable-5
@agriyakhetarpal
agriyakhetarpal self-requested a review June 15, 2026 07:36

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

Thanks! A note: a predictable URL of the form https://files.pythonhosted.org/packages/source/s/scikit_learn/scikit_learn-1.6.1.tar.gz also works right now because PyPI normalises it before downloading, but I guess it makes a case for normalisation, which is a good thing. The https://files.pythonhosted.org/packages/source/r/ruamel_yaml/ruamel.yaml-0.18.6.tar.gz change is useful to have – I don't think we've run into that one before.

Let me make the CHANGELOG less verbose while fixing the conflicts and merge.

@agriyakhetarpal

Copy link
Copy Markdown
Member

@henryiii, just to confirm: zip-style sdists are all pre-PEP-517 behaviour, right? I don't think (or hope) anyone is doing something like python setup.py sdist --formats=zip anymore, but I am fine with keeping this just in case.

@agriyakhetarpal
agriyakhetarpal merged commit 4f19581 into pyodide:main Jun 30, 2026
9 checks passed
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.

2 participants