Skip to content

Fix Cargo member globs with [..] classes (#1833) - #1866

Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-cargo-member-glob-classes
Open

Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-cargo-member-glob-classes

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1833

Summary

A Cargo workspace whose members list uses a character class, such as
members = ["crates/[ab]*"], expanded to no members. Vendored Cargo,
the upstream Cargo redirect and hosted Cargo (disk and in-memory) all read
members through cargo_workspace::member_manifests*, so they never saw
those crates.

Root cause

utils/cargo_workspace.rs matched each glob segment with its own
wildcard_match, which read only * and ? and treated [ as a
literal. Cargo expands members with the glob crate, which also reads
[abc], [a-z] and [!x].

Change

  • expand_from matches segments through
    workspace_globs::segment_glob_matches (the npm-family matcher), and
    wildcard_match is deleted.
  • The glob crate negates only on !, so [^x] is the class of ^
    and x. The shared matcher also negates on ^, so the cargo walk
    puts a NUL member (which no directory name can hold) ahead of a ^
    that opens a class.
  • New is_member_pattern: a segment is a glob when it has *, ? or a
    [..] class that closes. The member walk and the lockless Cargo
    redirect refusal (cargo_lockless_other_dependencies) both use it, so
    the refusal names crates/[ab] as a members pattern, not a missing
    literal member. An unclosed [ stays on the literal path as before.

Tests (red → green)

Issue Test main this PR
#1833 cargo_workspace::tests::member_globs_read_character_classes ([ab]*, [a-b]?*, [!a]*, [^x]*) FAIL (only unrelated libs/* hits) pass
#1833 cargo_workspace::tests::a_bracket_class_matches_a_literal_bracket (x[[]?) FAIL pass
#1833 cargo_workspace::tests::member_patterns_are_wildcards_or_closed_classes n/a (new helper) pass
#1833 redirect::tests lockless refusal names crates/[ab] as a pattern FAIL (named as literal member) pass

Commands run locally:

  • cargo fmt --all -- --check: clean
  • cargo clippy --workspace --all-features -- -D warnings: clean
  • cargo test -p socket-patch-core --all-features: 6996 passed, 4 failed. The 4 (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched, pypi_requirements::wire_failure_rolls_back_already_written_files) depend on chmod 0555 blocking writes, which it doesn't for root (this sandbox runs as uid 0). They don't touch Cargo code.
  • cargo test -p socket-patch-cli --all-features --test e2e_cargo --test e2e_redirect_cargo_shapes --test e2e_vendor_cargo_build --test e2e_redirect_cargo_build -- --include-ignored: 25 + 35 + 40 + 23 passed
  • The full cargo test --workspace didn't fit in this sandbox's disk allowance (it ran out of space linking every CLI test binary), so CI's test job covers the rest.

No wrapper changes: npm/, pypi/ and gem/ don't read Cargo workspaces.

/code-review high findings not fixed here

These all predate this PR. Each rule already applied to * and ?;
the PR only extends it to classes:

  • exclude is globbed. Cargo reads exclude as literal path
    prefixes, but member_manifests_with already globbed it with *.
    Making it prefix-based changes * excludes too, so that belongs in its
    own issue.
  • Dot directories. A wildcard segment never matches a dot-directory,
    but cargo's glob() uses require_literal_leading_dot: false. This
    rule is older than this PR, and * already followed it.
  • Backtracking cost. segment_glob_matches backtracks with no memo,
    and so did the deleted wildcard_match. This PR doesn't change the
    cost.
  • Unclosed [. Cargo refuses an unclosed [ (could not parse pattern). Here it stays a literal path, as it was on main, and no new
    test locks that in.

Picked over the p1 refactors #1844 and #1839 because both would rewrite
files that open PRs #1865 and #1855 are changing. #1862 (p2) builds on
go_build_list.rs, which open PR #1843 rewrites.

https://claude.ai/code/session_011f9aeSAqJp6HmcFwxb9HL5


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A workspace whose members list uses a character class, such as
members = ["crates/[ab]*"], expanded to no members: the member walk's
own matcher read only * and ?, while cargo's glob crate also reads
[abc], [a-z] and [!x]. Vendored Cargo, the upstream Cargo redirect and
hosted Cargo then never saw those crates.

Match each glob segment through workspace_globs::segment_glob_matches,
the matcher the npm-family walks already use, and drop the copy.

Fixes #1833

Assisted-by: Claude Code:claude-opus-5-5
Cargo's glob crate negates a class only on `!`, so members =
["crates/[^x]*"] names crates starting with `^` or `x`; the shared
matcher read it as negated and picked the opposite crates. A `^`
opening a class is now kept as a member.

A segment counts as a glob only when it has `*`, `?` or a class that
closes, so an unclosed `[` stays a literal directory, and the lockless
Cargo redirect refusal now names `crates/[ab]` as a members pattern
instead of a missing member, matching the member walk.

Refs #1833

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 30, 2026 01:06
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 30, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at 54c38dad5 (head).


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief: 54c38da

What it does. Cargo workspace members globs with a character class (crates/[ab]*, [!x]*) expanded to nothing, because cargo_workspace had its own matcher that understood only * and ?. The member walk now uses the shared workspace_globs::segment_glob_matches matcher, with a cargo-specific tweak so [^x] stays literal ^/x, as in the glob crate. The lockless redirect refusal now also reports class entries as patterns.

Risk: low. The change is limited to member-glob expansion (vendored, redirect and hosted Cargo all share it). Patterns using only */? match exactly as before, and an unclosed [ stays literal.

Look here

  • cargo_workspace.rs:383-405:`` is_member_pattern / `segment_is_pattern` and the `[^` → `[\0^` rewrite
  • cargo_workspace.rs:421 and :445-451:``` the walk switched to the shared matcher
  • cargo_workspace.rs:633-687:```` new tests ([ab]*, `[a-b]?`, `[!a]`, `[^x]*`, `[[]`)
  • redirect/mod.rs:2722: the refusal message uses the new helper

Verified

  • Read the full diff (2 files, +98/−26). CHANGELOG.md is untouched. No other member-glob check still uses the old */?-only test (grepped).
  • Ran cargo test -p socket-patch-core --lib -- cargo_workspace:: lockless: 23/23 pass. The old matcher read [ as a literal, so member_globs_read_character_classes can't pass without the fix.
  • CI: 82 checks on the head (32 success, 50 skipped, 0 failing). ci-ok and clippy are green. Bugbot: no issues on 54c38da. No open review threads.

Changes I made: none.

Open questions (non-blocking): segment_is_pattern strips a leading ^ as if it negated, so the degenerate member x[^] is still read as a literal directory name. The glob crate would read it as the class {^}. Nobody is likely to write that.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

A members entry like x[^] is a class matching `^` to cargo's glob
crate, which negates only on `!`. The pattern check treated the `^`
as a negation and read the entry as a literal directory instead.

Refs #1833

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Fixed the review brief's open question in d177158: segment_is_pattern now treats only ! as negation, as cargo's glob crate does, so x[^] is read as the class {^} instead of a literal directory. Added to member_patterns_are_wildcards_or_closed_classes.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d177158. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief: d177158 (follow-up to the 54c38da brief above)

What it does. Cargo workspace members globs with a character class (crates/[ab]*, [!x]*) expanded to nothing, because cargo_workspace had its own matcher that understood only * and ?. The member walk now uses the shared workspace_globs matcher, and [^x] stays a literal ^/x class, as in cargo's glob crate. Since the last brief, one commit (d177158) also reads the degenerate x[^] as the class {^}, which settles the open question from the earlier brief.

Risk: low. Only member-glob expansion changes. Patterns that use only */? match as before, and an unclosed [ stays literal. The new commit changes one strip_prefix call.

Look here

  • cargo_workspace.rs:390-397:`` segment_is_pattern now strips only `!` as a negation, matching the `glob` crate
  • cargo_workspace.rs:383-405:``` is_member_pattern and the `[^` → `[\0^` rewrite
  • cargo_workspace.rs:680-694:```` the pattern table, which now includes x[^]
  • redirect/mod.rs:2722: the lockless refusal uses the new helper

Verified

  • Read the 54c38da..d177158 delta (one commit, +10/−2). The earlier brief covers the rest of the diff. Traced x[^], x[^a], [^]] and x[!] through segment_is_pattern and the [\0^ rewrite: only x[^] changes classification (literal → class {^}), and that matches the glob crate. The new x[^] table entry fails on the old code. CHANGELOG.md is untouched.
  • Ran cargo test -p socket-patch-core --lib -- cargo_workspace:: lockless on d177158: 23/23 pass.
  • CI on the head: ci-ok and clippy are green, with no failing checks. Bugbot: success on d177158. No open review threads.

Changes I made: none.

Open questions: none.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cargo workspace member globs ignore [...] character classes, so members = ["crates/[ab]*"] expands to no members

2 participants