Remove the shlex dependency in favor of an in-tree implementation - #1726
Open
zerosnacks wants to merge 5 commits into
Open
Remove the shlex dependency in favor of an in-tree implementation#1726zerosnacks wants to merge 5 commits into
zerosnacks wants to merge 5 commits into
Conversation
1:1 port of the iterator path of the shlex 1.3.0 crate (the only piece
cc-rs ever exercised) into src/parse.rs, with attribution preserved.
The parser state machine (parse_word / parse_double / parse_single /
next_char + the whitespace and comment skip loop) is byte-identical to
upstream bytes::Shlex; for any &str input,
crate::parse::Shlex::new(s).collect::<Vec<String>>()
yields the same sequence of String items as
shlex::Shlex::new(s).collect::<Vec<String>>()
with identical had_error and line_no semantics. The bytes-side API
surface and the str-side newtype/Deref shim are not vendored since
cc-rs only uses the str iterator and never consumed them.
The retained SPLIT_TEST_ITEMS table is the upstream test corpus; the
expanded test set covers whitespace, quoting, escapes, comments,
unterminated input, trailing backslash, adjacent quoted segments and
UTF-8 passthrough.
cc-rs sits in the build-dep closure of nearly every Rust crate that
compiles native code, so removing this transitive dep shrinks the
audit surface for a large portion of the ecosystem.
Amp-Thread-ID: https://ampcode.com/threads/T-019e2783-3e9c-75f3-8039-9be8b3a382b3
Co-authored-by: Amp <amp@ampcode.com>
cc-rs only iterates the words produced by Shlex; neither line_no nor had_error is ever read outside parse.rs's own tests. Removing them strips dead code from the production binary and from every cc consumer. Parser behavior is unchanged: the iterator still stops at exactly the same input positions because parse_word/parse_double/parse_single already return None/Err on EOF inside quotes or after a trailing backslash; the field writes were merely additional side effects. Test corpus updates: - SPLIT_TEST_ITEMS: schema goes from (input, Option<&[&str]>) to (input, &[&str]). The previously-None rows all become &[] because those inputs all hit an unterminated quote or trailing backslash inside the only word being assembled, so the iterator yields nothing. - test_lineno is replaced by newline_separates_and_skips_leading, which uses the same "\nfoo\nbar" input but asserts the externally observable consequence (yielded words) instead of the now-removed line_no counter; a comment explains the trade. Amp-Thread-ID: https://ampcode.com/threads/T-019e2783-3e9c-75f3-8039-9be8b3a382b3 Co-authored-by: Amp <amp@ampcode.com>
- rustfmt: expand compact match arms into block form across parse_word, parse_double, parse_single and Iterator::next. - clippy: replace 'char as u8' casts with byte literals (b'\\n', b'\\\\') for ASCII characters where the cast is bit-identical, and drop the no-op 'ch as u8' cast where ch is already u8. - Comments: tighten the inline escape-handling comments in parse_word and parse_double, and update the loop header in Iterator::next from 'skip initial whitespace' to also mention line comments, which is what the loop actually skips. No behavior change: the parser yields the same String sequence for every &str input. Amp-Thread-ID: https://ampcode.com/threads/T-019e2783-3e9c-75f3-8039-9be8b3a382b3 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019e2783-3e9c-75f3-8039-9be8b3a382b3 Co-authored-by: Amp <amp@ampcode.com>
Use the dual-license disclaimer text from upstream shlex 1.3.0 verbatim, and link directly to the specific upstream source files we adapted (bytes.rs and the str wrapper in lib.rs) rather than just the crate page. Amp-Thread-ID: https://ampcode.com/threads/T-019e2783-3e9c-75f3-8039-9be8b3a382b3 Co-authored-by: Amp <amp@ampcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1725
Note: this PR was created with assistance of Amp (AI) and carefully manually reviewed and locally differentially tested w/ fuzzed inputs against Shlex 1.3.0
What
Replaces the
shlexcrate dependency with a ~100-line vendored word splitter insrc/parse.rs.shlexis used in exactly one place in cc-rs: parsing*FLAGSenvironment variables (CFLAGS,CXXFLAGS,ARFLAGS,RANLIBFLAGS) when shell-style splitting is opted into viaBuild::shell_escaped_flagsorCC_SHELL_ESCAPED_FLAGS=1. The single call site is inBuild::envflags.Why
cc-rs sits in the build-dep closure of nearly every Rust crate that touches native code, so any transitive dep is effectively part of the supply chain for a large portion of the ecosystem.
Of the ~1,300 LOC in
shlex1.3.0, cc-rs only exercises the iterator (~120 LOC). The remaining ~700 LOC is the quoting-side API (Quoter/escape side), which cc-rs never calls. Vendoring the small subset cc-rs actually uses:shlexfrom the dep graph of every crate that uses cc as a build-dep,Commit structure
The PR is split into commits for easier review:
src/parse.rsis byte-identical to upstreambytes::Shlex, with attribution preserved.line_noandhad_error. cc-rs never reads them; the iterator stops at exactly the same input positions because the parse helpers already returnNone/Err(())on EOF inside quotes or after a trailing backslash.'\n' as u8→b'\n'and'\\' as u8→b'\\'(clippy::char_lit_as_u8), and dropping a no-opch as u8cast (clippy::unnecessary_cast). All bit-identical for ASCII inputs; no behavior change.Note
The parser state machine is kept byte-for-byte identical to upstream rather than refactored, so equivalence with the original
shlex::Shlexiterator is easy to audit. The upstreamSPLIT_TEST_ITEMScorpus is preserved verbatim as the primary equivalence proof, with additional targeted tests layered on top. Stylistic modernization was deliberately kept out of scope for this PR.