From 1d576b36dbf19d57bbc8c24192bc24e32cd80392 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 05:24:09 +0000 Subject: [PATCH 1/5] Start fix for #397, #398 Assisted-by: Claude Code:claude-opus-5-5 From 6f66ee8157851f00976b3a4319b9b5689c275736 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 05:38:19 +0000 Subject: [PATCH 2/5] Patch the NuGet folder the project actually uses Agent-mode apply, scan and vex picked the first package root holding a package, and the default ~/.nuget/packages came before the project's own restore folders. A project that relocates its packages folder (globalPackagesFolder in nuget.config, RestorePackagesPath, or repositoryPath for packages.config) had the user-wide copy patched instead, apply reported success, and vex attested not_affected while the build used the unpatched copy. With a cold default cache, a project two levels below the solution root was not found at all. Roots are now resolved the way NuGet does: - the packages.config folder honors repositoryPath (nearest nuget.config wins, relative to that config's directory); - every obj/project.assets.json under the root, at any depth, contributes the packageFolders its restore used, ahead of the global folder; - the global folder is NUGET_PACKAGES, else the nearest globalPackagesFolder (user config alone in -g mode), else the default; a relocated folder replaces the default one. The nuget.config reader now records the configuration/config settings. Fixes #397 Fixes #398 Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/nuget_crawler.rs | 494 +++++++++++++++--- .../src/formats/nuget/mod.rs | 78 ++- .../src/vendor/nuget_config.rs | 83 +++ 3 files changed, 573 insertions(+), 82 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs index 397104586..740a80743 100644 --- a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs @@ -26,15 +26,22 @@ impl NuGetCrawler { /// Get NuGet package paths based on options. /// - /// In global mode, returns the global NuGet packages folder - /// (`NUGET_PACKAGES` env var or `~/.nuget/packages/`). + /// In global mode, returns the global packages folder NuGet uses + /// ([`global_packages_folder`] over the user config alone). /// /// In local mode, discovery is gated on `cwd` actually being a .NET /// project (see [`is_dotnet_project`]). When that gate passes, paths - /// are returned in priority order: - /// 1. `/packages/` folder (legacy packages.config layout) - /// 2. Global cache (`NUGET_PACKAGES` / `~/.nuget/packages/`) - /// 3. Paths discovered from `obj/project.assets.json` + /// are returned in priority order, the folders the project's own + /// restore uses first, since the first root to yield a purl wins + /// (#397, #398): + /// 1. The packages.config folder: nuget.config `repositoryPath`, else + /// `/packages/` + /// 2. The `packageFolders` of every `obj/project.assets.json` under + /// `cwd` (they record `RestorePackagesPath` and a configured + /// `globalPackagesFolder` as the restore resolved them) + /// 3. The global packages folder NuGet would use for `cwd` + /// ([`global_packages_folder`]); the default `~/.nuget/packages` + /// only when nothing relocates it pub async fn get_nuget_package_paths( &self, options: &CrawlerOptions, @@ -62,14 +69,13 @@ impl NuGetCrawler { if roots.paths.is_empty() { return Vec::new(); } - let local_packages = options.cwd.join("packages"); run_blocking(move || { let mut packages = Vec::new(); let mut seen = HashSet::new(); for pkg_path in &roots.paths { match &roots.scope { - Some(libraries) if *pkg_path != local_packages => { + Some(libraries) if roots.legacy.as_ref() != Some(pkg_path) => { packages.extend(locate_libraries(pkg_path, libraries, &mut seen)); } _ => packages.extend(scan_package_dir(pkg_path, &mut seen)), @@ -385,6 +391,22 @@ fn nuget_home() -> Option { crate::utils::fs::home_dir().map(|home| home.join(".nuget").join("packages")) } +/// The global packages folder NuGet uses for a build in `start` (`None`: +/// global mode, the user config alone): `NUGET_PACKAGES`, else the +/// nearest `globalPackagesFolder` setting, else `~/.nuget/packages` +/// (NuGet's `SettingsUtility.GetGlobalPackagesFolder` order). A relocated +/// folder replaces the default one: NuGet never reads the default then, so +/// a warm copy left there by another project must not win (#397). +async fn global_packages_folder(start: Option<&Path>) -> Option { + if std::env::var_os("NUGET_PACKAGES").is_some_and(|v| !v.is_empty()) { + return nuget_home(); + } + match crate::vendor::nuget_config::configured_folder(start, "globalPackagesFolder").await { + Some(folder) => Some(folder), + None => nuget_home(), + } +} + /// What `cwd` itself holds of a .NET build. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum DotnetRoot { @@ -481,8 +503,12 @@ struct PackageRoots { /// The roots, in priority order (see /// [`NuGetCrawler::get_nuget_package_paths`]). paths: Vec, + /// The packages.config folder among `paths` (`repositoryPath` or + /// `/packages`): always walked, never scoped, since a + /// packages.config project has no restore to scope it by. + legacy: Option, /// The `(id, version)` pairs every project under `cwd` restored - /// ([`restore_scope`]), when `cwd` is itself a project + /// ([`restore_walk`]), when `cwd` is itself a project /// ([`DotnetRoot::Project`]): the shared roots (global folder, /// `packageFolders`) are then looked up for these instead of walked. /// `None` keeps the walk: global mode, a root without a project file, @@ -496,7 +522,7 @@ async fn package_roots(options: &CrawlerOptions) -> PackageRoots { if options.global || options.global_prefix.is_some() { if let Some(ref custom) = options.global_prefix { roots.paths.push(custom.clone()); - } else if let Some(home) = nuget_home() { + } else if let Some(home) = global_packages_folder(None).await { if is_dir(&home).await { roots.paths.push(home); } @@ -520,60 +546,39 @@ async fn package_roots(options: &CrawlerOptions) -> PackageRoots { return roots; } - // 1. Check /packages/ (legacy packages.config layout) - let packages_dir = options.cwd.join("packages"); - if is_dir(&packages_dir).await && seen.insert(packages_dir.clone()) { - roots.paths.push(packages_dir); + // 1. The packages.config folder: `repositoryPath` relocates + // `/packages/` (#398). + let legacy = + crate::vendor::nuget_config::configured_folder(Some(&options.cwd), "repositoryPath") + .await + .unwrap_or_else(|| options.cwd.join("packages")); + if is_dir(&legacy).await && seen.insert(legacy.clone()) { + roots.paths.push(legacy.clone()); + roots.legacy = Some(legacy); } - // 2. Fall back to the global cache. - if let Some(home) = nuget_home() { - if is_dir(&home).await && seen.insert(home.clone()) { - roots.paths.push(home); + // 2. The folders the restores under `cwd` actually used, at any depth. + let cwd = options.cwd.clone(); + let walk = run_blocking(move || restore_walk(&cwd)).await; + for p in walk.package_folders { + if is_dir(&p).await && seen.insert(p.clone()) { + roots.paths.push(p); } } - // 3. Check obj/ dirs for project.assets.json - for p in discover_paths_from_assets(&options.cwd).await { - if is_dir(&p).await && seen.insert(p.clone()) { - roots.paths.push(p); + // 3. The global packages folder NuGet would restore into. + if let Some(home) = global_packages_folder(Some(&options.cwd)).await { + if is_dir(&home).await && seen.insert(home.clone()) { + roots.paths.push(home); } } if root == DotnetRoot::Project { - let cwd = options.cwd.clone(); - roots.scope = run_blocking(move || restore_scope(&cwd)).await; + roots.scope = walk.libraries; } roots } -/// Discover additional package paths from `obj/project.assets.json` files: -/// `cwd`'s own and those of its direct subdirectories (multi-project -/// solutions). -async fn discover_paths_from_assets(cwd: &Path) -> Vec { - let mut paths = Vec::new(); - - // Look for obj/project.assets.json in cwd - if let Some(root) = read_project_assets(&cwd.join("obj").join("project.assets.json")).await { - paths.extend(root.package_folders); - } - - // Also check subdirectories one level deep for multi-project solutions - for entry in crate::utils::fs::list_dir_entries(cwd).await { - if !crate::utils::fs::entry_is_dir(&entry).await { - continue; - } - let sub_assets = cwd - .join(entry.file_name()) - .join("obj") - .join("project.assets.json"); - if let Some(sub) = read_project_assets(&sub_assets).await { - paths.extend(sub.package_folders); - } - } - paths -} - /// What one `project.assets.json` says. #[derive(Debug, Default)] struct ProjectAssets { @@ -586,13 +591,6 @@ struct ProjectAssets { libraries: Option>, } -/// Read and parse one `project.assets.json`; `None` when the file is -/// unreadable (or not a regular file) or not JSON. -async fn read_project_assets(path: &Path) -> Option { - let content = crate::utils::fs::read_regular_to_string(path).await.ok()?; - parse_project_assets(&content) -} - /// Parse one `project.assets.json` ([`ProjectAssets`]). fn parse_project_assets(content: &str) -> Option { let json: serde_json::Value = serde_json::from_str(content).ok()?; @@ -622,44 +620,63 @@ fn parse_project_assets(content: &str) -> Option { }) } -/// Directories [`restore_scope`] never descends into: build output, the +/// Directories [`restore_walk`] never descends into: build output, the /// restore's own `obj/`, a legacy `packages/` folder, and JS dependencies. const SCOPE_SKIPPED_DIRS: [&str; 4] = ["bin", "obj", "packages", "node_modules"]; -/// Directories [`restore_scope`] lists before giving up on a scope (the +/// Directories [`restore_walk`] lists before giving up on a scope (the /// crawl then walks the shared roots, as before). const SCOPE_DIR_BUDGET: usize = 10_000; -/// The packages every project under `cwd` restored: the union of the -/// `libraries` of each project directory's `obj/project.assets.json`. +/// What the restores under a directory say ([`restore_walk`]). +#[derive(Debug, Default)] +struct RestoreWalk { + /// The `packageFolders` of every `obj/project.assets.json` found, in + /// walk order (`cwd`'s own first): the folders those restores used. + package_folders: Vec, + /// The packages every project restored ([`PackageRoots::scope`]). + libraries: Option>, +} + +/// Walk `cwd` for restores: every directory's `obj/project.assets.json` +/// contributes its `packageFolders` (best effort, whatever the walk could +/// see), and the projects' `libraries` form the scope. /// -/// `None` (keep the walk) unless EVERY project file found under `cwd` -/// (`.csproj`/`.fsproj`/`.vbproj`, at any depth) has a parsed assets file -/// with a `libraries` object, so a project the scope cannot see (not -/// restored yet, a relocated `obj/`, an unreadable directory) never loses -/// its packages. Symlinked directories, hidden ones and -/// [`SCOPE_SKIPPED_DIRS`] are not entered; more than -/// [`SCOPE_DIR_BUDGET`] directories also means `None`. -fn restore_scope(cwd: &Path) -> Option> { - let mut libraries = Vec::new(); +/// The scope is `None` (keep the walk) unless EVERY project file found +/// under `cwd` (`.csproj`/`.fsproj`/`.vbproj`, at any depth) has a parsed +/// assets file with a `libraries` object, so a project the scope cannot see +/// (not restored yet, a relocated `obj/`, an unreadable directory) never +/// loses its packages. Symlinked directories, hidden ones and +/// [`SCOPE_SKIPPED_DIRS`] are not entered; more than [`SCOPE_DIR_BUDGET`] +/// directories also means `None`. +fn restore_walk(cwd: &Path) -> RestoreWalk { + let mut walk = RestoreWalk { + package_folders: Vec::new(), + libraries: Some(Vec::new()), + }; let mut pending = vec![cwd.to_path_buf()]; let mut listed = 0usize; while let Some(dir) = pending.pop() { listed += 1; if listed > SCOPE_DIR_BUDGET { - return None; + walk.libraries = None; + break; } let (entries, complete) = super::listing::list_dir_sync_complete(&dir); if !complete { - return None; + walk.libraries = None; } - let mut is_project = false; + let (mut is_project, mut has_obj) = (false, false); for entry in &entries { let Some(name) = entry.name.to_str() else { continue; }; - let kind = entry.file_type?; + let Some(kind) = entry.file_type else { + walk.libraries = None; + continue; + }; if kind.is_dir() { + has_obj |= name == "obj"; if !name.starts_with('.') && !SCOPE_SKIPPED_DIRS.contains(&name) { pending.push(dir.join(name)); } @@ -667,13 +684,28 @@ fn restore_scope(cwd: &Path) -> Option> { is_project = true; } } + if !(is_project || has_obj) { + continue; + } + let assets = dir.join("obj").join("project.assets.json"); + let parsed = crate::utils::fs::read_regular_to_string_sync(&assets) + .ok() + .and_then(|content| parse_project_assets(&content)); + let libraries = match parsed { + Some(assets) => { + walk.package_folders.extend(assets.package_folders); + assets.libraries + } + None => None, + }; if is_project { - let assets = dir.join("obj").join("project.assets.json"); - let content = crate::utils::fs::read_regular_to_string_sync(&assets).ok()?; - libraries.extend(parse_project_assets(&content)?.libraries?); + match (walk.libraries.as_mut(), libraries) { + (Some(all), Some(own)) => all.extend(own), + _ => walk.libraries = None, + } } } - Some(libraries) + walk } /// Look up each resolved `(id, version)` in a shared (global-layout) root, @@ -1029,7 +1061,12 @@ mod tests { .await .unwrap(); - let paths = discover_paths_from_assets(dir.path()).await; + let paths = run_blocking({ + let cwd = dir.path().to_path_buf(); + move || restore_walk(&cwd) + }) + .await + .package_folders; assert_eq!(paths.len(), 1); assert_eq!(paths[0], pkg_folder); } @@ -2180,4 +2217,301 @@ mod tests { "the readable sub-project assets file must still be discovered, got {paths:?}" ); } + + // ── Configured package folders (#397, #398) ─────────────────────── + + /// Points `HOME`/`USERPROFILE` at a temp dir, clears `NUGET_PACKAGES` + /// and installs `user_config` as the user-level NuGet config, restoring + /// all of it on drop. Callers are `#[serial]`. + struct NugetEnv { + saved: Vec<(&'static str, Option)>, + } + + impl NugetEnv { + fn new(home: &Path, user_config: Option) -> Self { + let saved = ["HOME", "USERPROFILE", "NUGET_PACKAGES"] + .into_iter() + .map(|k| (k, std::env::var_os(k))) + .collect(); + std::env::set_var("HOME", home); + std::env::set_var("USERPROFILE", home); + std::env::remove_var("NUGET_PACKAGES"); + crate::vendor::nuget_config::tests_support::USER_CONFIG + .with(|c| *c.borrow_mut() = user_config); + Self { saved } + } + } + + impl Drop for NugetEnv { + fn drop(&mut self) { + for (k, v) in &self.saved { + match v { + Some(v) => std::env::set_var(k, v), + None => std::env::remove_var(k), + } + } + crate::vendor::nuget_config::tests_support::USER_CONFIG + .with(|c| *c.borrow_mut() = None); + } + } + + /// Install `newtonsoft.json@13.0.3` under `root` in the global layout. + async fn install_global(root: &Path) -> PathBuf { + let dir = root.join("newtonsoft.json").join("13.0.3"); + tokio::fs::create_dir_all(&dir).await.unwrap(); + tokio::fs::write(dir.join("newtonsoft.json.nuspec"), "") + .await + .unwrap(); + dir + } + + /// A home whose default `~/.nuget/packages` another project warmed + /// with the same package. + async fn warm_home(root: &Path) -> (PathBuf, PathBuf) { + let home = root.join("home"); + let copy = install_global(&home.join(".nuget").join("packages")).await; + (home, copy) + } + + async fn write(path: &Path, text: &str) { + tokio::fs::create_dir_all(path.parent().unwrap()) + .await + .unwrap(); + tokio::fs::write(path, text).await.unwrap(); + } + + /// Where agent `apply` resolves the purl: the first root (in + /// `get_nuget_package_paths` order) holding it, as `merge_first_wins` + /// keeps it. + async fn applied_copy(cwd: &Path) -> Option { + let purl = "pkg:nuget/Newtonsoft.Json@13.0.3".to_string(); + for root in NuGetCrawler + .get_nuget_package_paths(&local(cwd)) + .await + .unwrap() + { + let found = NuGetCrawler + .find_by_purls(&root, std::slice::from_ref(&purl)) + .await + .unwrap(); + if let Some(pkg) = found.get(&purl) { + return Some(pkg.path.clone()); + } + } + None + } + + fn crawled_copy(packages: &[CrawledPackage]) -> Option { + packages + .iter() + .find(|p| p.purl == "pkg:nuget/newtonsoft.json@13.0.3") + .map(|p| p.path.clone()) + } + + /// The #397 repro: a solution root, `src/App/App.csproj`, and a + /// nuget.config `globalPackagesFolder` of `.nuget/packages`, restored. + async fn relocated_solution(root: &Path) -> (PathBuf, PathBuf) { + let proj = root.join("proj"); + write( + &proj.join("nuget.config"), + r#" + + + +"#, + ) + .await; + write(&proj.join("App.sln"), "").await; + let app = proj.join("src").join("App"); + write(&app.join("App.csproj"), "").await; + let folder = proj.join(".nuget").join("packages"); + let copy = install_global(&folder).await; + write_assets( + &app, + &folder, + serde_json::json!({ + "Newtonsoft.Json/13.0.3": { "type": "package", "path": "newtonsoft.json/13.0.3" } + }), + ) + .await; + (proj, copy) + } + + /// #397: with a warm default cache, the project's configured + /// `globalPackagesFolder` copy is the one patched and crawled, not the + /// user-wide one. Red on main (`~/.nuget/packages` won). + #[tokio::test] + #[serial_test::serial] + async fn configured_global_packages_folder_beats_a_warm_default_cache() { + let dir = tempfile::tempdir().unwrap(); + let (home, _default_copy) = warm_home(dir.path()).await; + let _env = NugetEnv::new(&home, None); + let (proj, copy) = relocated_solution(dir.path()).await; + + assert_eq!(applied_copy(&proj).await, Some(copy.clone())); + assert_eq!( + crawled_copy(&NuGetCrawler.crawl_all(&local(&proj)).await), + Some(copy.clone()) + ); + // From the project directory too (restore-scoped crawl). + let app = proj.join("src").join("App"); + assert_eq!(applied_copy(&app).await, Some(copy.clone())); + assert_eq!( + crawled_copy(&NuGetCrawler.crawl_all(&local(&app)).await), + Some(copy) + ); + } + + /// #397: with a cold default cache, a project two levels below the + /// solution root is still found through its restore. Red on main + /// (assets were read only one level deep: "matched no installed + /// package"). + #[tokio::test] + #[serial_test::serial] + async fn deep_project_restore_is_found_with_a_cold_default_cache() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path().join("home"); + tokio::fs::create_dir_all(&home).await.unwrap(); + let _env = NugetEnv::new(&home, None); + let (proj, copy) = relocated_solution(dir.path()).await; + // Without the config, only the restore names the folder. + tokio::fs::remove_file(proj.join("nuget.config")) + .await + .unwrap(); + + assert_eq!(applied_copy(&proj).await, Some(copy)); + } + + /// #397: `RestorePackagesPath` (MSBuild, recorded only in the + /// restore's `packageFolders`) beats a warm default cache. Red on main. + #[tokio::test] + #[serial_test::serial] + async fn restore_packages_path_beats_a_warm_default_cache() { + let dir = tempfile::tempdir().unwrap(); + let (home, _default_copy) = warm_home(dir.path()).await; + let _env = NugetEnv::new(&home, None); + let proj = dir.path().join("proj"); + write(&proj.join("App.csproj"), "").await; + let pkgs = proj.join("pkgs"); + let copy = install_global(&pkgs).await; + write_assets( + &proj, + &pkgs, + serde_json::json!({ + "Newtonsoft.Json/13.0.3": { "type": "package", "path": "newtonsoft.json/13.0.3" } + }), + ) + .await; + + assert_eq!(applied_copy(&proj).await, Some(copy.clone())); + assert_eq!( + crawled_copy(&NuGetCrawler.crawl_all(&local(&proj)).await), + Some(copy) + ); + } + + /// #397 (global mode): a `globalPackagesFolder` in the user-level + /// NuGet.Config relocates the folder `-g` scans. Red on main (only + /// `~/.nuget/packages` was crawled). + #[tokio::test] + #[serial_test::serial] + async fn global_mode_reads_the_user_config_global_packages_folder() { + let dir = tempfile::tempdir().unwrap(); + let (home, _default_copy) = warm_home(dir.path()).await; + let gpf = dir.path().join("gpf"); + let copy = install_global(&gpf).await; + let user_config = home.join(".nuget").join("NuGet").join("NuGet.Config"); + write( + &user_config, + &format!( + r#""#, + gpf.display() + ), + ) + .await; + let _env = NugetEnv::new(&home, Some(user_config)); + let options = CrawlerOptions { + cwd: dir.path().to_path_buf(), + global: true, + global_prefix: None, + }; + + assert_eq!( + NuGetCrawler + .get_nuget_package_paths(&options) + .await + .unwrap(), + [gpf] + ); + assert_eq!( + crawled_copy(&NuGetCrawler.crawl_all(&options).await), + Some(copy) + ); + } + + /// `NUGET_PACKAGES` still overrides a configured folder, as in NuGet. + #[tokio::test] + #[serial_test::serial] + async fn nuget_packages_env_overrides_the_configured_folder() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path().join("home"); + let _env = NugetEnv::new(&home, None); + let proj = dir.path().join("proj"); + write( + &proj.join("nuget.config"), + r#""#, + ) + .await; + write(&proj.join("App.sln"), "").await; + install_global(&proj.join("cfg")).await; + let env_folder = dir.path().join("env"); + let copy = install_global(&env_folder).await; + std::env::set_var("NUGET_PACKAGES", &env_folder); + + assert_eq!(applied_copy(&proj).await, Some(copy)); + } + + /// #398: a packages.config solution whose nuget.config sets + /// `repositoryPath` is patched in that folder, not in the global + /// folder nuget.exe also extracted into. Red on main (only + /// `/packages` was considered, so `~/.nuget/packages` won). + #[tokio::test] + #[serial_test::serial] + async fn repository_path_folder_beats_the_global_folder() { + let dir = tempfile::tempdir().unwrap(); + let (home, _default_copy) = warm_home(dir.path()).await; + let _env = NugetEnv::new(&home, None); + let repro = dir.path().join("repro"); + write( + &repro.join("nuget.config"), + r#" + +"#, + ) + .await; + write(&repro.join("App.sln"), "").await; + write( + &repro.join("App").join("packages.config"), + r#""#, + ) + .await; + write(&repro.join("App").join("App.csproj"), "").await; + let copy = repro + .join("lib") + .join("pkgs") + .join("Newtonsoft.Json.13.0.3"); + write(©.join("Newtonsoft.Json.nuspec"), "").await; + + assert_eq!(applied_copy(&repro).await, Some(copy.clone())); + let crawled: Vec<_> = NuGetCrawler + .crawl_all(&local(&repro)) + .await + .into_iter() + .filter(|p| p.name.eq_ignore_ascii_case("newtonsoft.json")) + .map(|p| p.path) + .collect(); + assert_eq!(crawled.first(), Some(©)); + // From the project directory, the parent's config still applies. + assert_eq!(applied_copy(&repro.join("App")).await, Some(copy)); + } } diff --git a/crates/socket-patch-core/src/formats/nuget/mod.rs b/crates/socket-patch-core/src/formats/nuget/mod.rs index 6a2e949fb..aed746512 100644 --- a/crates/socket-patch-core/src/formats/nuget/mod.rs +++ b/crates/socket-patch-core/src/formats/nuget/mod.rs @@ -5,8 +5,9 @@ //! The reader is a minimal, bounded XML tokenizer (no parser dependency) //! that records only what NuGet routes by: `` directly under //! `configuration/packageSources`, the mapping elements directly under -//! `configuration/packageSourceMapping`, and `disabledPackageSources` -//! entries whose `value` is `true`. Comments, CDATA, processing instructions +//! `configuration/packageSourceMapping`, `disabledPackageSources` +//! entries whose `value` is `true`, and the `` settings under +//! `configuration/config` (where the package folders live). Comments, CDATA, processing instructions //! and DOCTYPEs are skipped; an unterminated tag or comment, an unquoted //! attribute or a mismatched close tag makes the whole file `None`. @@ -36,6 +37,12 @@ pub(crate) struct NugetConfig { /// farther config (parent directories, the user config) defined are /// dropped, and only the ones after it count. pub(crate) sources_cleared: bool, + /// `configuration/config/add` `(key, value)`, document order: the + /// settings such as `globalPackagesFolder` and `repositoryPath`. + pub(crate) settings: Vec<(String, String)>, + /// `configuration/config` holds a ``: the settings every + /// farther config defined are dropped. + pub(crate) settings_cleared: bool, /// Where each `mappings` row sits (parallel to it): the whole /// `` element and each of its pattern tags, so a writer can /// set one aside and put it back byte-exact. @@ -87,6 +94,10 @@ fn record_clear(cfg: &mut NugetConfig, parents: &[&str], end: usize) { // NuGet drops what the file itself defined before the ``. cfg.sources.clear(); } + if parents == ["configuration", "config"] { + cfg.settings_cleared = true; + cfg.settings.clear(); + } let section = match parents { ["configuration", "packageSources"] => cfg.package_sources.as_mut(), ["configuration", "packageSourceMapping"] => cfg.source_mapping.as_mut(), @@ -217,6 +228,11 @@ fn visit(stack: &[&str], tag: &Tag<'_>, cfg: &mut NugetConfig, open_mapping: &mu cfg.sources.push((key.to_string(), value.to_string())); } } + (["configuration", "config"], "add") => { + if let (Some(key), Some(value)) = (tag.attr("key"), tag.attr("value")) { + cfg.settings.push((key.to_string(), value.to_string())); + } + } (["configuration", "disabledPackageSources"], "add") => { if let (Some(key), Some(value)) = (tag.attr("key"), tag.attr("value")) { if value.trim().eq_ignore_ascii_case("true") { @@ -372,6 +388,33 @@ pub(crate) fn effective_source_keys<'a>( keys } +/// The value of the `config` setting `key` (matched ignoring case, as NuGet +/// does) in `chain`, nearest config first: the nearest config defining it +/// wins, and a config whose `config` section holds a `` hides every +/// farther one. `(index into chain, value)` so the caller can resolve a +/// relative value against that config's directory. +pub(crate) fn effective_setting<'a>( + chain: impl IntoIterator, + key: &str, +) -> Option<(usize, &'a str)> { + for (idx, cfg) in chain.into_iter().enumerate() { + // A later `` of the same key overrides an earlier one in the + // same file. + if let Some((_, value)) = cfg + .settings + .iter() + .rev() + .find(|(k, _)| k.eq_ignore_ascii_case(key)) + { + return Some((idx, value.as_str())); + } + if cfg.settings_cleared { + return None; + } + } + None +} + /// The comment a writer sets a competing mapping element aside in while the /// Socket source `key` is wired: ``. fn set_aside_open(key: &str) -> String { @@ -508,6 +551,37 @@ mod tests { assert_eq!(super::effective_source_keys([&parent, &cfg]), ["new"]); } + /// #397/#398: the `config` settings are read, the nearest config + /// wins, and a `` hides farther ones. + #[test] + fn config_settings_resolve_nearest_first() { + let near = super::parse_config( + "", + ) + .unwrap(); + let far = super::parse_config( + "", + ) + .unwrap(); + let cleared = super::parse_config( + "", + ) + .unwrap(); + assert_eq!( + super::effective_setting([&near, &far], "repositorypath"), + Some((0, "lib/pkgs")) + ); + assert_eq!( + super::effective_setting([&near, &far], "globalPackagesFolder"), + Some((1, "/far")) + ); + assert!(cleared.settings.is_empty()); + assert_eq!( + super::effective_setting([&near, &cleared, &far], "globalPackagesFolder"), + None + ); + } + const COMPETING: &str = "\n \n \n \n \n \n \n \n \n \n \n \n \n \n \n \n\n"; /// #462: another source's exact pattern for the id is set aside (the diff --git a/crates/socket-patch-core/src/vendor/nuget_config.rs b/crates/socket-patch-core/src/vendor/nuget_config.rs index de0322520..5c5dc852b 100644 --- a/crates/socket-patch-core/src/vendor/nuget_config.rs +++ b/crates/socket-patch-core/src/vendor/nuget_config.rs @@ -167,6 +167,89 @@ pub(crate) async fn inherited_source_keys_traced( ) } +// ── package folder settings ── + +/// The configs NuGet merges for a build in `start` (every directory from +/// `start` up to the filesystem root, then the user config), NEAREST +/// first, each with the directory its relative paths resolve against. A +/// file that cannot be read or parsed is skipped, as in +/// [`inherited_source_keys_traced`]. `None` reads the user config alone +/// (global mode: no project directory takes part). +async fn settings_chain( + start: Option<&std::path::Path>, +) -> Vec<(std::path::PathBuf, crate::formats::nuget::NugetConfig)> { + use crate::formats::nuget::parse_config; + async fn read(path: &std::path::Path) -> Option { + let text = crate::utils::fs::read_regular_to_string(path).await.ok()?; + parse_config(crate::formats::text::strip_bom(&text)) + } + let mut chain = Vec::new(); + for dir in start.into_iter().flat_map(std::path::Path::ancestors) { + for name in CONFIG_NAMES { + let path = dir.join(name); + if !crate::utils::fs::file_exists(&path).await { + continue; + } + if let Some(cfg) = read(&path).await { + chain.push((dir.to_path_buf(), cfg)); + } + break; + } + } + if let Some(path) = user_config_path() { + if let (Some(cfg), Some(dir)) = (read(&path).await, path.parent()) { + chain.push((dir.to_path_buf(), cfg)); + } + } + chain +} + +/// The folder the `config` setting `key` (`globalPackagesFolder`, +/// `repositoryPath`) names for a build in `start` ([`settings_chain`]): +/// the nearest config defining it wins, `%VAR%` references are expanded +/// (NuGet expands them on every OS), and a relative value resolves against +/// the directory of the config that set it. `None` when no config sets it, +/// or its value is empty or names an unset variable. +pub(crate) async fn configured_folder( + start: Option<&std::path::Path>, + key: &str, +) -> Option { + let chain = settings_chain(start).await; + let (idx, value) = crate::formats::nuget::effective_setting(chain.iter().map(|(_, c)| c), key)?; + let value = expand_env_vars(value.trim())?; + if value.is_empty() { + return None; + } + let path = std::path::PathBuf::from(value); + Some(if path.is_absolute() { + path + } else { + chain[idx].0.join(path) + }) +} + +/// Expand every `%NAME%` in `value`; `None` when one names an unset (or +/// empty) variable, so a half-expanded path is never probed. +fn expand_env_vars(value: &str) -> Option { + let mut out = String::with_capacity(value.len()); + let mut rest = value; + while let Some(start) = rest.find('%') { + let Some(len) = rest[start + 1..].find('%') else { + break; + }; + let name = &rest[start + 1..start + 1 + len]; + out.push_str(&rest[..start]); + if name.is_empty() { + out.push('%'); + } else { + out.push_str(&std::env::var(name).ok().filter(|v| !v.is_empty())?); + } + rest = &rest[start + len + 2..]; + } + out.push_str(rest); + Some(out) +} + // ── project walk ── /// Directories the project walk never enters: build output, the restore's From 31ac0830cb386b85c1e9aed3f1f9ef219437f744 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 05:54:48 +0000 Subject: [PATCH 3/5] Test -g apply against a configured NuGet folder Covers the global-mode leg of #397 end to end: a globalPackagesFolder in the user-level NuGet.Config must be the copy `scan -g --sync` patches, and the stale default ~/.nuget/packages copy must stay untouched. Assisted-by: Claude Code:claude-opus-5-5 --- .../in_process_remote_ecosystems_apply.rs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs b/crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs index 69903f754..b893d19cd 100644 --- a/crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs +++ b/crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs @@ -663,6 +663,68 @@ async fn maven_handcrafted_discovery() { std::env::remove_var("MAVEN_REPO_LOCAL"); } +/// #397 (global mode): a `globalPackagesFolder` in the user-level +/// NuGet.Config relocates the folder NuGet restores into. `scan -g --sync` +/// patches that copy and leaves the stale default `~/.nuget/packages` one +/// (warmed by another project) alone. Red on main: the default copy was +/// patched and the configured one stayed unpatched. +#[tokio::test] +#[serial] +async fn nuget_global_apply_patches_the_user_configured_global_packages_folder() { + let _home = NoJvmCaches::new(); + std::env::remove_var("NUGET_PACKAGES"); + let home = PathBuf::from(std::env::var_os("HOME").unwrap()); + let tmp = tempfile::tempdir().expect("tempdir"); + let original = b"MIT License\nCopyright (c) 2007 James Newton-King\n"; + let install = |root: &Path| { + let pkg_dir = root.join("newtonsoft.json").join("13.0.3"); + std::fs::create_dir_all(&pkg_dir).unwrap(); + std::fs::write(pkg_dir.join("newtonsoft.json.nuspec"), "").unwrap(); + std::fs::write(pkg_dir.join("LICENSE.md"), original).unwrap(); + pkg_dir.join("LICENSE.md") + }; + let default_copy = install(&home.join(".nuget").join("packages")); + let gpf = tmp.path().join("gpf"); + let configured_copy = install(&gpf); + let user_config = home.join(".nuget").join("NuGet").join("NuGet.Config"); + std::fs::create_dir_all(user_config.parent().unwrap()).unwrap(); + std::fs::write( + &user_config, + format!( + r#""#, + gpf.display() + ), + ) + .unwrap(); + + let mut patched = original.to_vec(); + patched.extend_from_slice(b"\n# SOCKET-PATCH-E2E-MARKER\n"); + let server = MockServer::start().await; + setup_apply_mock( + &server, + "pkg:nuget/Newtonsoft.Json@13.0.3", + "39739739-7397-4397-8397-397397397397", + "package/LICENSE.md", + &git_sha256(original), + &git_sha256(&patched), + &patched, + ) + .await; + + let code = scan_run(default_scan_args(tmp.path(), "nuget", server.uri())).await; + assert_eq!(code, 0, "scan -g --sync should apply the nuget patch"); + assert_eq!( + std::fs::read(&configured_copy).unwrap(), + patched, + "the configured globalPackagesFolder copy must be patched" + ); + assert_eq!( + std::fs::read(&default_copy).unwrap(), + original, + "the unused default ~/.nuget/packages copy must stay untouched" + ); +} + #[tokio::test] #[serial] async fn nuget_handcrafted_discovery() { From 01ff0ff68021fc272db4761792b5251699a76010 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 06:12:45 +0000 Subject: [PATCH 4/5] Harden NuGet folder resolution from review - Read parent configs when --cwd is relative (the default `.`): the config chain starts from the absolute path. - Build the config chain once per crawl and query both settings. - Expand %VAR% like .NET: an unset variable stays literal instead of dropping the setting and falling back to the default folder. - Honor repositoryPath only when a packages.config lives under the root, so a shared store set in a parent or user config is not walked unscoped ahead of a PackageReference project's restore. - Skip the configured stores and already-found package folders in the restore walk, so a big in-tree store can't use up its budget. - Use DOTNET_CLI_HOME for the default folder off Windows, matching the user-config lookup. - Make the -g CLI test write the user config where Windows reads it. Assisted-by: Claude Code:claude-opus-5-5 --- .../in_process_remote_ecosystems_apply.rs | 21 ++- .../src/crawlers/nuget_crawler.rs | 130 ++++++++++++---- .../src/vendor/nuget_config.rs | 144 +++++++++++++----- .../tests/crawler_nuget_e2e.rs | 4 +- 4 files changed, 228 insertions(+), 71 deletions(-) diff --git a/crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs b/crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs index b893d19cd..978a32ab4 100644 --- a/crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs +++ b/crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs @@ -672,8 +672,22 @@ async fn maven_handcrafted_discovery() { #[serial] async fn nuget_global_apply_patches_the_user_configured_global_packages_folder() { let _home = NoJvmCaches::new(); + let saved: Vec<_> = ["NUGET_PACKAGES", "DOTNET_CLI_HOME", "APPDATA"] + .into_iter() + .map(|k| (k, std::env::var_os(k))) + .collect(); std::env::remove_var("NUGET_PACKAGES"); + std::env::remove_var("DOTNET_CLI_HOME"); let home = PathBuf::from(std::env::var_os("HOME").unwrap()); + // NuGet's user config: %APPDATA%\NuGet\NuGet.Config on Windows, else + // ~/.nuget/NuGet/NuGet.Config. + let user_config = if cfg!(windows) { + let appdata = home.join("AppData").join("Roaming"); + std::env::set_var("APPDATA", &appdata); + appdata.join("NuGet").join("NuGet.Config") + } else { + home.join(".nuget").join("NuGet").join("NuGet.Config") + }; let tmp = tempfile::tempdir().expect("tempdir"); let original = b"MIT License\nCopyright (c) 2007 James Newton-King\n"; let install = |root: &Path| { @@ -686,7 +700,6 @@ async fn nuget_global_apply_patches_the_user_configured_global_packages_folder() let default_copy = install(&home.join(".nuget").join("packages")); let gpf = tmp.path().join("gpf"); let configured_copy = install(&gpf); - let user_config = home.join(".nuget").join("NuGet").join("NuGet.Config"); std::fs::create_dir_all(user_config.parent().unwrap()).unwrap(); std::fs::write( &user_config, @@ -712,6 +725,12 @@ async fn nuget_global_apply_patches_the_user_configured_global_packages_folder() .await; let code = scan_run(default_scan_args(tmp.path(), "nuget", server.uri())).await; + for (k, v) in saved { + match v { + Some(v) => std::env::set_var(k, v), + None => std::env::remove_var(k), + } + } assert_eq!(code, 0, "scan -g --sync should apply the nuget patch"); assert_eq!( std::fs::read(&configured_copy).unwrap(), diff --git a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs index 740a80743..f799fc8d8 100644 --- a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs @@ -5,6 +5,7 @@ use super::listing::{list_dir_sync, ListedEntry}; use super::types::{CrawledPackage, CrawlerOptions}; use crate::patch::path_safety; use crate::utils::fs::{is_dir, is_dir_sync, run_blocking}; +use crate::vendor::nuget_config::{settings_chain, SettingsChain}; use crate::vendor::nuget_feed::normalize_nuget_version; #[cfg(test)] @@ -388,23 +389,29 @@ fn nuget_home() -> Option { } } - crate::utils::fs::home_dir().map(|home| home.join(".nuget").join("packages")) + // NuGet's home is `DOTNET_CLI_HOME` off Windows when set (the same + // rule `vendor::nuget_config` reads the user config by). + let cli_home = (!cfg!(windows)) + .then(|| std::env::var_os("DOTNET_CLI_HOME")) + .flatten() + .filter(|v| !v.is_empty()) + .map(PathBuf::from); + cli_home + .or_else(crate::utils::fs::home_dir) + .map(|home| home.join(".nuget").join("packages")) } -/// The global packages folder NuGet uses for a build in `start` (`None`: -/// global mode, the user config alone): `NUGET_PACKAGES`, else the -/// nearest `globalPackagesFolder` setting, else `~/.nuget/packages` +/// The global packages folder NuGet uses for the configs in `chain` +/// ([`settings_chain`]): `NUGET_PACKAGES`, else the nearest +/// `globalPackagesFolder` setting, else the default [`nuget_home`] /// (NuGet's `SettingsUtility.GetGlobalPackagesFolder` order). A relocated /// folder replaces the default one: NuGet never reads the default then, so /// a warm copy left there by another project must not win (#397). -async fn global_packages_folder(start: Option<&Path>) -> Option { +fn global_packages_folder(chain: &SettingsChain) -> Option { if std::env::var_os("NUGET_PACKAGES").is_some_and(|v| !v.is_empty()) { return nuget_home(); } - match crate::vendor::nuget_config::configured_folder(start, "globalPackagesFolder").await { - Some(folder) => Some(folder), - None => nuget_home(), - } + chain.folder("globalPackagesFolder").or_else(nuget_home) } /// What `cwd` itself holds of a .NET build. @@ -522,7 +529,7 @@ async fn package_roots(options: &CrawlerOptions) -> PackageRoots { if options.global || options.global_prefix.is_some() { if let Some(ref custom) = options.global_prefix { roots.paths.push(custom.clone()); - } else if let Some(home) = global_packages_folder(None).await { + } else if let Some(home) = global_packages_folder(&settings_chain(None).await) { if is_dir(&home).await { roots.paths.push(home); } @@ -546,20 +553,32 @@ async fn package_roots(options: &CrawlerOptions) -> PackageRoots { return roots; } + let chain = settings_chain(Some(&options.cwd)).await; + let repository_path = chain.folder("repositoryPath"); + let global = global_packages_folder(&chain); + + // The restores under `cwd`, at any depth. The configured folders are + // package stores, never project trees: the walk skips them, so a large + // in-tree store cannot spend its directory budget. + let skip: Vec = repository_path.iter().chain(&global).cloned().collect(); + let cwd = options.cwd.clone(); + let walk = run_blocking(move || restore_walk(&cwd, &skip)).await; + // 1. The packages.config folder: `repositoryPath` relocates - // `/packages/` (#398). - let legacy = - crate::vendor::nuget_config::configured_folder(Some(&options.cwd), "repositoryPath") - .await - .unwrap_or_else(|| options.cwd.join("packages")); + // `/packages/` (#398). NuGet reads `repositoryPath` only for + // packages.config projects, so a configured one (often a shared + // store set in a parent or user config) counts only when one lives + // under `cwd`. + let legacy = match repository_path { + Some(folder) if walk.packages_config => folder, + _ => options.cwd.join("packages"), + }; if is_dir(&legacy).await && seen.insert(legacy.clone()) { roots.paths.push(legacy.clone()); roots.legacy = Some(legacy); } - // 2. The folders the restores under `cwd` actually used, at any depth. - let cwd = options.cwd.clone(); - let walk = run_blocking(move || restore_walk(&cwd)).await; + // 2. The folders the restores under `cwd` actually used. for p in walk.package_folders { if is_dir(&p).await && seen.insert(p.clone()) { roots.paths.push(p); @@ -567,7 +586,7 @@ async fn package_roots(options: &CrawlerOptions) -> PackageRoots { } // 3. The global packages folder NuGet would restore into. - if let Some(home) = global_packages_folder(Some(&options.cwd)).await { + if let Some(home) = global { if is_dir(&home).await && seen.insert(home.clone()) { roots.paths.push(home); } @@ -636,6 +655,9 @@ struct RestoreWalk { package_folders: Vec, /// The packages every project restored ([`PackageRoots::scope`]). libraries: Option>, + /// A `packages.config` (any casing) was seen: a project under `cwd` + /// uses the packages.config layout. + packages_config: bool, } /// Walk `cwd` for restores: every directory's `obj/project.assets.json` @@ -647,14 +669,19 @@ struct RestoreWalk { /// assets file with a `libraries` object, so a project the scope cannot see /// (not restored yet, a relocated `obj/`, an unreadable directory) never /// loses its packages. Symlinked directories, hidden ones and -/// [`SCOPE_SKIPPED_DIRS`] are not entered; more than [`SCOPE_DIR_BUDGET`] -/// directories also means `None`. -fn restore_walk(cwd: &Path) -> RestoreWalk { +/// [`SCOPE_SKIPPED_DIRS`] are not entered, nor `skip` or a package folder +/// a restore already named (package stores, not project trees); more than +/// [`SCOPE_DIR_BUDGET`] directories also means `None`. +fn restore_walk(cwd: &Path, skip: &[PathBuf]) -> RestoreWalk { let mut walk = RestoreWalk { package_folders: Vec::new(), libraries: Some(Vec::new()), + packages_config: false, }; - let mut pending = vec![cwd.to_path_buf()]; + // Absolute, so `skip` and the restores' (absolute) package folders + // compare against the walked paths. + let root = std::path::absolute(cwd).unwrap_or_else(|_| cwd.to_path_buf()); + let mut pending = vec![root]; let mut listed = 0usize; while let Some(dir) = pending.pop() { listed += 1; @@ -677,11 +704,18 @@ fn restore_walk(cwd: &Path) -> RestoreWalk { }; if kind.is_dir() { has_obj |= name == "obj"; - if !name.starts_with('.') && !SCOPE_SKIPPED_DIRS.contains(&name) { - pending.push(dir.join(name)); + let sub = dir.join(name); + if !name.starts_with('.') + && !SCOPE_SKIPPED_DIRS.contains(&name) + && !skip.contains(&sub) + && !walk.package_folders.contains(&sub) + { + pending.push(sub); } } else if PROJECT_EXTENSIONS.iter().any(|ext| name.ends_with(ext)) { is_project = true; + } else if name.eq_ignore_ascii_case("packages.config") { + walk.packages_config = true; } } if !(is_project || has_obj) { @@ -1063,7 +1097,7 @@ mod tests { let paths = run_blocking({ let cwd = dir.path().to_path_buf(); - move || restore_walk(&cwd) + move || restore_walk(&cwd, &[]) }) .await .package_folders; @@ -2229,13 +2263,14 @@ mod tests { impl NugetEnv { fn new(home: &Path, user_config: Option) -> Self { - let saved = ["HOME", "USERPROFILE", "NUGET_PACKAGES"] + let saved = ["HOME", "USERPROFILE", "NUGET_PACKAGES", "DOTNET_CLI_HOME"] .into_iter() .map(|k| (k, std::env::var_os(k))) .collect(); std::env::set_var("HOME", home); std::env::set_var("USERPROFILE", home); std::env::remove_var("NUGET_PACKAGES"); + std::env::remove_var("DOTNET_CLI_HOME"); crate::vendor::nuget_config::tests_support::USER_CONFIG .with(|c| *c.borrow_mut() = user_config); Self { saved } @@ -2514,4 +2549,45 @@ mod tests { // From the project directory, the parent's config still applies. assert_eq!(applied_copy(&repro.join("App")).await, Some(copy)); } + + /// A `repositoryPath` set for a shared store (here in a parent config) + /// is not a PackageReference project's folder: NuGet reads it only for + /// packages.config, so the project's restore folder still wins and the + /// store is not walked. + #[tokio::test] + #[serial_test::serial] + async fn repository_path_is_ignored_without_a_packages_config() { + let dir = tempfile::tempdir().unwrap(); + let home = dir.path().join("home"); + let _env = NugetEnv::new(&home, None); + let store = dir.path().join("store"); + install_global(&store).await; + write( + &dir.path().join("nuget.config"), + &format!( + r#""#, + store.display() + ), + ) + .await; + let proj = dir.path().join("proj"); + write(&proj.join("App.csproj"), "").await; + let pkgs = proj.join("pkgs"); + let copy = install_global(&pkgs).await; + write_assets( + &proj, + &pkgs, + serde_json::json!({ + "Newtonsoft.Json/13.0.3": { "type": "package", "path": "newtonsoft.json/13.0.3" } + }), + ) + .await; + + let paths = NuGetCrawler + .get_nuget_package_paths(&local(&proj)) + .await + .unwrap(); + assert!(!paths.contains(&store), "{paths:?}"); + assert_eq!(applied_copy(&proj).await, Some(copy)); + } } diff --git a/crates/socket-patch-core/src/vendor/nuget_config.rs b/crates/socket-patch-core/src/vendor/nuget_config.rs index 5c5dc852b..0aa773fd2 100644 --- a/crates/socket-patch-core/src/vendor/nuget_config.rs +++ b/crates/socket-patch-core/src/vendor/nuget_config.rs @@ -171,20 +171,24 @@ pub(crate) async fn inherited_source_keys_traced( /// The configs NuGet merges for a build in `start` (every directory from /// `start` up to the filesystem root, then the user config), NEAREST -/// first, each with the directory its relative paths resolve against. A -/// file that cannot be read or parsed is skipped, as in +/// first, each with the directory its relative paths resolve against +/// ([`settings_chain`]). +pub(crate) struct SettingsChain(Vec<(std::path::PathBuf, crate::formats::nuget::NugetConfig)>); + +/// [`SettingsChain`] for `start`. A relative `start` (the default `--cwd` +/// is `.`) is made absolute first, so its parents' configs are read. A file +/// that cannot be read or parsed is skipped, as in /// [`inherited_source_keys_traced`]. `None` reads the user config alone /// (global mode: no project directory takes part). -async fn settings_chain( - start: Option<&std::path::Path>, -) -> Vec<(std::path::PathBuf, crate::formats::nuget::NugetConfig)> { +pub(crate) async fn settings_chain(start: Option<&std::path::Path>) -> SettingsChain { use crate::formats::nuget::parse_config; async fn read(path: &std::path::Path) -> Option { let text = crate::utils::fs::read_regular_to_string(path).await.ok()?; parse_config(crate::formats::text::strip_bom(&text)) } + let start = start.map(|s| std::path::absolute(s).unwrap_or_else(|_| s.to_path_buf())); let mut chain = Vec::new(); - for dir in start.into_iter().flat_map(std::path::Path::ancestors) { + for dir in start.iter().flat_map(|s| s.ancestors()) { for name in CONFIG_NAMES { let path = dir.join(name); if !crate::utils::fs::file_exists(&path).await { @@ -201,53 +205,111 @@ async fn settings_chain( chain.push((dir.to_path_buf(), cfg)); } } - chain + SettingsChain(chain) } -/// The folder the `config` setting `key` (`globalPackagesFolder`, -/// `repositoryPath`) names for a build in `start` ([`settings_chain`]): -/// the nearest config defining it wins, `%VAR%` references are expanded -/// (NuGet expands them on every OS), and a relative value resolves against -/// the directory of the config that set it. `None` when no config sets it, -/// or its value is empty or names an unset variable. -pub(crate) async fn configured_folder( - start: Option<&std::path::Path>, - key: &str, -) -> Option { - let chain = settings_chain(start).await; - let (idx, value) = crate::formats::nuget::effective_setting(chain.iter().map(|(_, c)| c), key)?; - let value = expand_env_vars(value.trim())?; - if value.is_empty() { - return None; +impl SettingsChain { + /// The folder the `config` setting `key` (`globalPackagesFolder`, + /// `repositoryPath`) names: the nearest config defining it wins, + /// `%VAR%` references are expanded as .NET does (NuGet expands them on + /// every OS; an unset one stays literal), and a relative value resolves + /// against the directory of the config that set it. `None` when no + /// config sets it or its value is empty. + pub(crate) fn folder(&self, key: &str) -> Option { + let (idx, value) = + crate::formats::nuget::effective_setting(self.0.iter().map(|(_, c)| c), key)?; + let value = expand_env_vars(value.trim()); + if value.is_empty() { + return None; + } + let path = std::path::PathBuf::from(value); + Some(if path.is_absolute() { + path + } else { + self.0[idx].0.join(path) + }) } - let path = std::path::PathBuf::from(value); - Some(if path.is_absolute() { - path - } else { - chain[idx].0.join(path) - }) } -/// Expand every `%NAME%` in `value`; `None` when one names an unset (or -/// empty) variable, so a half-expanded path is never probed. -fn expand_env_vars(value: &str) -> Option { +/// Expand every `%NAME%` naming a set variable in `value`, as .NET's +/// `Environment.ExpandEnvironmentVariables` does: an unset name is kept +/// literally, and its closing `%` may open the next reference. +fn expand_env_vars(value: &str) -> String { let mut out = String::with_capacity(value.len()); let mut rest = value; while let Some(start) = rest.find('%') { - let Some(len) = rest[start + 1..].find('%') else { - break; - }; - let name = &rest[start + 1..start + 1 + len]; out.push_str(&rest[..start]); - if name.is_empty() { - out.push('%'); - } else { - out.push_str(&std::env::var(name).ok().filter(|v| !v.is_empty())?); + let after = &rest[start + 1..]; + let Some(len) = after.find('%') else { + out.push_str(&rest[start..]); + return out; + }; + match std::env::var(&after[..len]).ok().filter(|_| len > 0) { + Some(v) => { + out.push_str(&v); + rest = &after[len + 1..]; + } + None => { + out.push('%'); + out.push_str(&after[..len]); + rest = &after[len..]; + } } - rest = &rest[start + len + 2..]; } out.push_str(rest); - Some(out) + out +} + +#[cfg(test)] +mod settings_tests { + /// A set `%VAR%` expands; an unset or empty-named one stays literal, + /// as in .NET. + #[test] + #[serial_test::serial] + fn env_vars_expand_like_dotnet() { + std::env::set_var("SP_NUGET_TEST_ROOT", "/r"); + std::env::remove_var("SP_NUGET_TEST_UNSET"); + assert_eq!( + super::expand_env_vars("%SP_NUGET_TEST_ROOT%/pkgs"), + "/r/pkgs" + ); + assert_eq!( + super::expand_env_vars("%SP_NUGET_TEST_UNSET%/x"), + "%SP_NUGET_TEST_UNSET%/x" + ); + assert_eq!( + super::expand_env_vars("a%SP_NUGET_TEST_UNSET%SP_NUGET_TEST_ROOT%"), + "a%SP_NUGET_TEST_UNSET/r" + ); + assert_eq!(super::expand_env_vars("100%%"), "100%%"); + assert_eq!(super::expand_env_vars("50%"), "50%"); + std::env::remove_var("SP_NUGET_TEST_ROOT"); + } + + /// The default `--cwd .` still reads its parents' configs. + #[tokio::test] + #[serial_test::serial] + async fn a_relative_start_reads_parent_configs() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("nuget.config"), + r#""#, + ) + .unwrap(); + let app = dir.path().join("App"); + std::fs::create_dir(&app).unwrap(); + let prev = std::env::current_dir().unwrap(); + std::env::set_current_dir(&app).unwrap(); + let folder = super::settings_chain(Some(std::path::Path::new("."))) + .await + .folder("repositoryPath"); + std::env::set_current_dir(prev).unwrap(); + let folder = folder.expect("the parent config's repositoryPath"); + assert!( + folder.is_absolute() && folder.ends_with("lib/pkgs"), + "{folder:?}" + ); + } } // ── project walk ── diff --git a/crates/socket-patch-core/tests/crawler_nuget_e2e.rs b/crates/socket-patch-core/tests/crawler_nuget_e2e.rs index 2228e9a8c..3014d3707 100644 --- a/crates/socket-patch-core/tests/crawler_nuget_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_nuget_e2e.rs @@ -707,7 +707,7 @@ async fn get_nuget_package_paths_with_nuget_config_falls_back_to_global() { /// A staged `obj/project.assets.json` with a `packageFolders` map /// must surface those folders alongside the global cache. Covers -/// `discover_paths_from_assets` and `parse_project_assets_package_folders`. +/// the restore walk and `parse_project_assets`. #[tokio::test] #[serial] async fn get_nuget_package_paths_discovers_assets_json_package_folders() { @@ -759,7 +759,7 @@ async fn get_nuget_package_paths_discovers_assets_json_package_folders() { } /// `project.assets.json` exists in a subdirectory (multi-project -/// solution) — `discover_paths_from_assets` walks one level deep. +/// solution) — the restore walk finds it (at any depth). #[tokio::test] #[serial] async fn get_nuget_package_paths_discovers_assets_json_in_subproject() { From b90c9426b588e455dde34ecdb91ddb80b299dfba Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 06:33:00 +0000 Subject: [PATCH 5/5] Skip a store a restore names before walking it The restore walk queued a directory's subdirectories before reading that directory's own project.assets.json, so a store it names (a solution-root RestorePackagesPath of pkgs/) was still walked. A large store could spend the 10,000-directory budget before deeper restores were seen, and a project-shaped folder inside it dropped the restore scope. The walk now queues children after reading the restore, and skips a queued directory that a later restore names. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/crawlers/nuget_crawler.rs | 90 ++++++++++++++----- 1 file changed, 70 insertions(+), 20 deletions(-) diff --git a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs index f799fc8d8..34837c618 100644 --- a/crates/socket-patch-core/src/crawlers/nuget_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/nuget_crawler.rs @@ -684,6 +684,10 @@ fn restore_walk(cwd: &Path, skip: &[PathBuf]) -> RestoreWalk { let mut pending = vec![root]; let mut listed = 0usize; while let Some(dir) = pending.pop() { + // A store a restore named after `dir` was queued. + if walk.package_folders.contains(&dir) { + continue; + } listed += 1; if listed > SCOPE_DIR_BUDGET { walk.libraries = None; @@ -694,6 +698,9 @@ fn restore_walk(cwd: &Path, skip: &[PathBuf]) -> RestoreWalk { walk.libraries = None; } let (mut is_project, mut has_obj) = (false, false); + // Queued only after this directory's own restore is read, so a + // store it names (`RestorePackagesPath` = `pkgs`) is never entered. + let mut subdirs = Vec::new(); for entry in &entries { let Some(name) = entry.name.to_str() else { continue; @@ -708,9 +715,8 @@ fn restore_walk(cwd: &Path, skip: &[PathBuf]) -> RestoreWalk { if !name.starts_with('.') && !SCOPE_SKIPPED_DIRS.contains(&name) && !skip.contains(&sub) - && !walk.package_folders.contains(&sub) { - pending.push(sub); + subdirs.push(sub); } } else if PROJECT_EXTENSIONS.iter().any(|ext| name.ends_with(ext)) { is_project = true; @@ -718,26 +724,30 @@ fn restore_walk(cwd: &Path, skip: &[PathBuf]) -> RestoreWalk { walk.packages_config = true; } } - if !(is_project || has_obj) { - continue; - } - let assets = dir.join("obj").join("project.assets.json"); - let parsed = crate::utils::fs::read_regular_to_string_sync(&assets) - .ok() - .and_then(|content| parse_project_assets(&content)); - let libraries = match parsed { - Some(assets) => { - walk.package_folders.extend(assets.package_folders); - assets.libraries - } - None => None, - }; - if is_project { - match (walk.libraries.as_mut(), libraries) { - (Some(all), Some(own)) => all.extend(own), - _ => walk.libraries = None, + if is_project || has_obj { + let assets = dir.join("obj").join("project.assets.json"); + let parsed = crate::utils::fs::read_regular_to_string_sync(&assets) + .ok() + .and_then(|content| parse_project_assets(&content)); + let libraries = match parsed { + Some(assets) => { + walk.package_folders.extend(assets.package_folders); + assets.libraries + } + None => None, + }; + if is_project { + match (walk.libraries.as_mut(), libraries) { + (Some(all), Some(own)) => all.extend(own), + _ => walk.libraries = None, + } } } + pending.extend( + subdirs + .into_iter() + .filter(|sub| !walk.package_folders.contains(sub)), + ); } walk } @@ -2590,4 +2600,44 @@ mod tests { assert!(!paths.contains(&store), "{paths:?}"); assert_eq!(applied_copy(&proj).await, Some(copy)); } + + /// A store the walked directory's own restore names (a solution-root + /// `RestorePackagesPath` of `pkgs/`, written with NuGet's trailing + /// separator) is not entered: here a project-shaped folder inside it + /// would otherwise drop the scope, and a large store would spend the + /// directory budget before deeper restores are seen. + #[test] + fn restore_walk_does_not_enter_a_store_its_restore_names() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::write(root.join("App.csproj"), "").unwrap(); + let pkgs = root.join("pkgs"); + std::fs::create_dir_all(pkgs.join("some.tool").join("1.0.0").join("content")).unwrap(); + std::fs::write( + pkgs.join("some.tool") + .join("1.0.0") + .join("content") + .join("Template.csproj"), + "", + ) + .unwrap(); + std::fs::create_dir_all(root.join("obj")).unwrap(); + let key = format!("{}{}", pkgs.display(), std::path::MAIN_SEPARATOR); + std::fs::write( + root.join("obj").join("project.assets.json"), + serde_json::to_string(&serde_json::json!({ + "libraries": { "Newtonsoft.Json/13.0.3": { "type": "package" } }, + "packageFolders": { key: {} } + })) + .unwrap(), + ) + .unwrap(); + + let walk = restore_walk(root, &[]); + assert_eq!(walk.package_folders, [pkgs]); + assert_eq!( + walk.libraries, + Some(vec![("Newtonsoft.Json".to_string(), "13.0.3".to_string())]) + ); + } }