Skip to content

fix(publish): make allow_public actually reach crates.io - #341

Merged
fslabs-bot[bot] merged 2 commits into
mainfrom
fix-crates-io-publish
Aug 27, 2026
Merged

fix(publish): make allow_public actually reach crates.io#341
fslabs-bot[bot] merged 2 commits into
mainfrom
fix-crates-io-publish

Conversation

@loispostula

@loispostula loispostula commented Aug 26, 2026

Copy link
Copy Markdown
Member

The bug

allow_public inserted a registry named crates.io. Cargo does not accept that as a registry name, so the public publish was unreachable for every repo relying on it.

publish/mod.rs:1155 passes the registry name verbatim to cargo publish --registry, and cargo rejects it:

error: invalid character `.` in registry name: `crates.io`,
characters must be Unicode XID characters

Real consequence: opd-parser shipped 0.6.0 to the private registry while crates.io stayed at 0.5.0.

The fix

Canonicalize on cargo's own spelling, crates-io.

That makes the allow_public entry collapse with any crates-io already contributed by package.publish, instead of sitting beside it as a second, unusable entry. It also makes the pre-existing filter(|r| *r != "crates-io") at check_workspace/mod.rs:876 correct for both sources — that line already assumed this spelling.

Also defaults the sparse index for crates-io, since it has no [registries] entry to read one from (cargo knows the index natively). This sits after the env and config merges, so an explicit argument, CARGO_REGISTRIES_CRATES_IO_INDEX, or a config entry still wins.

Verification

Against real cargo, not just unit tests:

cargo publish --dry-run --registry crates-io                  # reaches Uploading
CARGO_REGISTRIES_CRATES_IO_INDEX=... _TOKEN=... \
  cargo publish --dry-run --registry crates-io                # also reaches Uploading

The second matters because get_registry_env injects exactly those vars, and cargo could plausibly have rejected an index override on its built-in registry. It does not.

243 tests pass, cargo fmt --check clean. Also verified green with CARGO_REGISTRIES_CRATES_IO_INDEX set to the real index, matching how CI runs.

Correction to my first description of this PR

I originally claimed a second, independent failure: that the missing index made check_crate_exists fail in production, and its caller swallowed the error into publish = false.

The CI failure on the first push disproved that. My test asserted the default index by constructing a registry through CargoRegistry::new, and it failed on Prow — because Prow does set CARGO_REGISTRIES_CRATES_IO_INDEX from vault, so the env supplied an index and the default never applied. Which means the registry very likely resolved an index in the real publish job too, and the existence check was not necessarily the thing that broke.

The illegal registry name is fatal on its own and is proven, so the fix stands unchanged. The index default is now a robustness improvement for environments that do not set that var, not a claimed root cause. The test was rewritten to assert default_index() directly rather than read ambient env.

Consumers need one more thing

crates-io must be listed in package.publish. A publish list naming only the private registry still blocks the upload — cargo's own allowlist, not this code. For opd-parser that is fslabs/opd_parser#17.

Unrelated, not fixed here

cargo clippy --all-targets -- -D warnings fails on generate_wix/mod.rs:917 (iter_kv_map). Confirmed pre-existing by stashing this change and re-running. Toolchain drift: this repo pins 1.93, my local cargo is 1.98. Left alone as out of scope.

allow_public added a registry named "crates.io", which no part of the
publish path could use.

Cargo rejects that name outright ('invalid character `.` in registry
name'), and publish passes the name straight to 'cargo publish
--registry', so the upload could never have run. Before that, the
registry resolved with index: None, because crates-io has no
[registries] entry to read one from and cargo knows the index natively.
check_crate_exists therefore failed, and its caller swallows an error
into publish = false, so the crate silently never went out. opd-parser
shipped 0.6.0 to the private registry while crates.io stayed on 0.5.0.

Use cargo's own spelling, crates-io, so the entry collapses with any
crates-io contributed by package.publish instead of sitting beside it as
a second unusable one, and default the sparse index behind the existing
env and config overrides.

Verified against real cargo: 'cargo publish --dry-run --registry
crates-io' reaches Uploading, both bare and with the
CARGO_REGISTRIES_CRATES_IO_INDEX/_TOKEN vars get_registry_env injects.

Note that repos relying on this need 'crates-io' in package.publish;
a publish list naming only the private registry still blocks the
upload, which is cargo's check, not this one.

Signed-off-by: Loïs Postula <lois@postu.la>
@fslabs-bot fslabs-bot Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 26, 2026
test_crates_io_defaults_to_its_sparse_index built a registry through
CargoRegistry::new, which merges CARGO_REGISTRIES_CRATES_IO_INDEX before
applying the default. CI sets that var from vault, so the assertion held
locally and failed there.

Split the fallback into default_index() and assert on that, which is the
logic the test meant to cover. The override test keeps going through new:
merge only fills fields that are still None, so an explicitly passed
index holds whatever the env supplies.

Signed-off-by: Loïs Postula <lois@postu.la>
@fslabs-bot

fslabs-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JulianRuiseco

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fslabs-bot fslabs-bot Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2026
@fslabs-bot
fslabs-bot Bot merged commit 2f11659 into main Aug 27, 2026
2 checks passed
@fslabs-bot
fslabs-bot Bot deleted the fix-crates-io-publish branch August 27, 2026 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants