Repository navigation
Read and splice nuget.config through formats::nuget in hosted, vendored and restore #594
Description
Activity
- addedpm:nugetNuGet / dotnetNuGet / dotnetarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)Filed by a scheduled architecture audit routine (see the architecture review discussion)refactorStructural change: duplicated code or logic, missing abstraction, layering, dead codeStructural change: duplicated code or logic, missing abstraction, layering, dead code
on Oct 2, 2026 mikolalysenko commented
on Oct 2, 2026 CollaboratorAuthorMore actions[agent] Triage: priority:p3 (NuGet refactor). The hosted half (part (a): span view + hosted rewriter, closing #561 and #585) is addressed by open PR #597. Vendored and restore (part (b)) remain.
Generated by Claude Code
mikolalysenko commented
on Oct 3, 2026 CollaboratorAuthorMore actions[agent] Shares root cause with #685: the vendored
nuget.configwriter (vendor/nuget_feed.rs) matches section close tags as literal substrings, so</packageSources >gets a duplicate section. Routing it throughformats::nugetas proposed here fixes that symptom too. Will be fixed together.
Generated by Claude Code
mikolalysenko commented
on Oct 5, 2026 CollaboratorAuthorMore actions[agent] Re-checked on
main@4646693. The hosted part is done: #597 deletednuget_package_source_keys, and hosted routing and splice anchors now go throughformats::nuget::parse_config(used atredirect/mod.rs#L5084, restore and VEX).What remains is the vendored side.
vendor/nuget_feed.rsstill keeps its own reader,parse_config_source_keys,`` and its ownfind-based splice anchors in `build_config_edit` (`visible.find("")`, the self-closing span at L1114). It doesn't use the insertion spans that #597 added to the shared model. This issue now covers moving those onto `formats::nuget` and deleting the vendored reader.
Generated by Claude Code
mikolalysenko commented
on Oct 9, 2026 CollaboratorAuthorMore actions[agent] Claiming this issue for the architecture refactor routine (highest leverage: the vendored
nuget.configwriter is the last reader outsideformats::nuget::parse_configin a free file; routing it through the shared tokenizer deletesblank_comments,parse_config_source_keys,attr_valueandself_closing_package_sourcesand fixes #685). Slice: vendored writer and its idempotence check only. Hosted restoreremove_source(upstream/nuget.rs, in a maintainer PR) and the vendored revert excision remain. Branch: arch-refactor/594-vendored-nuget-config. Claim-ID: 2026-10-09T14:56:13Z-ead28c
Generated by Claude Code
mikolalysenko commented
on Oct 9, 2026 CollaboratorAuthorMore actions- added a commit that references this issue
on Oct 9, 2026
[agent] Filed by the scheduled architecture audit routine (ecosystems and formats). Register: E11.
Kind: refactor. Source: review §3.7 #3, Part 3.4 and Part 5.4; register E11 (the NuGet half of E10).
Problem
nuget.configis read and spliced by six private code paths with four different XML rules. Only one of them is a real tokenizer.Readers of the
<packageSources>keys:formats::nuget::parse_configis a bounded tokenizer. It skips comments, CDATA and PIs, and only records<add>directly underconfiguration/packageSources. Upstream restore and VEX use it.nuget_package_source_keysruns two regexes over the raw text, so a commented-out<add>counts (Hosted NuGet mapping reads commented-out package sources #561).parse_config_source_keysusesfind("<packageSources")plus a substringattr_valueoverblank_commentsoutput.Splice anchors and removers:
insert_nuget_sourceandnuget_mapping_open_endtake the first regex match of<packageSources>/<packageSourceMapping>, including one inside a comment (Hosted NuGet splices the Socket source (and mapping) into a commented-out <packageSources> / <packageSourceMapping> block, so every restore fails NU1100 while scan reports success and its in-run VEX attests not_affected #585).nuget_after_last_clearcarries its own comment masker.remove_sourcebuilds an `<add … key=…/>` regex and uses another private [`attr_value`](https://github.com/SocketDev/socket-patch/blob/203e092bdb91f573df9d0120eae2c2004a50d358/crates/socket-patch-core/src/patch/redirect/upstream/nuget.rs#L39-L48).``build_config_editanchors on the comment-blanked text, andexcise_source_mappingmatches an exact 4-space byte pattern.Config file names are spelled four times:
redirect::NUGET_CONFIG_FILE_NAMES,``vendor::nuget_config::CONFIG_NAMES, `hosted/memory/roots.rs` and `formats/registry.rs`. They haven't drifted yet.The code paths themselves have drifted. The vendored writer is comment-safe; the hosted writer and reader are not. The reader that upstream restore uses (
parse_config) and the one the hosted rewriter uses disagree about the same file.Symptoms
<packageSources>/<packageSourceMapping>(anchors). Restore then fails NU1100 while scan reports success.*fanned only to nuget.org).Impact
Every hosted NuGet bug about comments, self-closing sections or attribute spelling has to be fixed up to three times, and the reader each writer trusts is not the one restore and VEX trust. Size: about 250 production lines of regex and substring scanning.
Proposed change
formats::nugetwith a span view:parse_config_spans(text) -> Option<NugetConfigSpans>records the byte offsets that every writer needs (the<configuration>open-tag end, the<packageSources>open/close or self-closing span, the<packageSourceMapping>ditto, the last<clear/>end in each, and each<add>/<packageSource>element span with its key). It is computed by the same tokenizer asparse_config, so comments and CDATA are never anchors.add_nuget_sourceandrewrite_nugettake the keys and anchors from it. Deletenuget_package_source_keys, theNUGET_PACKAGE_SOURCES_REGION_RE/NUGET_ADD_KEY_REstatics, the regexes ininsert_nuget_source/nuget_mapping_open_end/add_nuget_source, andnuget_after_last_clear's comment masker.remove_sourceand vendoredexcise_source_mapping/parse_config_source_keysuse the element spans. Delete both privateattr_values andblank_comments(if no other caller remains).formats::nuget::CONFIG_FILE_NAMES; delete the other three lists.This can land as two PRs if it's too big: (a) the span view plus hosted (closes #561 and #585), (b) vendored and restore.
Size and scope
formats/nuget/mod.rs,patch/redirect/mod.rs(NuGet section only),patch/redirect/upstream/nuget.rs,vendor/nuget_feed.rs,vendor/nuget_config.rs,hosted/memory/roots.rs. About +200 / −300 production lines. Out of scope: Maven/Gradle XML (rest of E10) and thepackages.lock.jsonwalks (#593).Acceptance criteria
formats::nuget). No regex or substring scan overnuget.configtext remains inredirect/,hosted/orvendor/.cargo test -p socket-patch-core --lib nuget, the redirect equivalence suites,e2e_nuget*).<add>, commented<packageSources>before the real one, commented<packageSourceMapping>, self-closing sections, single-quoted andkey = "…"attributes, CRLF) run through the hosted rewriter, the vendored writer andparse_config. The Socket source and mapping land in the live sections, and the keys each writer sees equalparse_config's.Dependencies
Supersedes the narrower fix in #561 (either can land first; if #561 lands first, this deletes its remaining regex). Touches
rewrite_nugetlike #593, so sequence the two. Blocks the Maven/Gradle half of E10.