Skip to content

fix(config): load symlinked dependency configs#15871

Draft
mablr wants to merge 3 commits into
masterfrom
mablr/oss-582-discover-symlinked-dependency-configs
Draft

fix(config): load symlinked dependency configs#15871
mablr wants to merge 3 commits into
masterfrom
mablr/oss-582-discover-symlinked-dependency-configs

Conversation

@mablr

@mablr mablr commented Jul 23, 2026

Copy link
Copy Markdown
Member

WIP
Closes OSS-582

Resolve dependency aliases before discovering nested foundry.toml files, while preserving lexical aliases for generated remappings. Traverse declared nested libraries once per canonical config and stop project-root or dependency cycles.

@mattsse mattsse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The standard dependency source remapping is dropped by this condition. This regresses can_detect_lib_foundry_toml, can_prioritise_closer_lib_remappings, and can_prioritise_project_remappings: dependencies using src, contracts, or lib no longer get a package remapping when the directory does not exist yet, and closest-remapping precedence can change. Please synthesize the declared source remapping for every nested config, not only nonstandard source directories. Removing this condition makes all three tests pass locally.

Always synthesize the source remapping declared by nested dependency configs. Auto-detection cannot recover standard source directories when they have not been created yet.

@mattsse mattsse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The external suite still has two deterministic remapping regressions; details and fix suggestions inline.

// Nested configs are traversed here, so load their declared remappings without
// recursively installing another remappings provider.
let figment =
Config::with_root(&entry.canonical).to_figment(FigmentProviders::Cast);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Using FigmentProviders::Cast skips RemappingsProvider entirely, so nested remappings.txt files are dropped. This is why snekmate loses the @openzeppelin/contracts/ mapping from lib/properties/remappings.txt. Keep traversal non-recursive but still merge each dependency’s remappings.txt, for example by installing a provider with auto-detection disabled and no library paths, or by parsing and merging that file explicitly. Please add focused coverage for this case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 from my pass

remappings.push(r);
// Synthesize the declared source remapping for every lexical alias. The source
// directory may not exist yet, so `Remapping::find_many` cannot always detect it.
if let Some(name) = entry.path.file_name().and_then(|name| name.to_str()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Always pointing the package alias at its configured source directory breaks dependencies whose imports intentionally include that directory, such as forge-std/src/...; prb_math now resolves these as node_modules/forge-std/src/src/.... Only synthesize src, contracts, or lib when the declared directory is missing, and otherwise let Remapping::find_many preserve the package-root mapping. Custom source directories should continue to be synthesized.

@mablr
mablr marked this pull request as draft July 24, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants