autosetup: resolve hoisted node_modules packages, and say which kind of unresolved import failed - #168
Open
shellygr wants to merge 2 commits into
Open
autosetup: resolve hoisted node_modules packages, and say which kind of unresolved import failed#168shellygr wants to merge 2 commits into
shellygr wants to merge 2 commits into
Conversation
… imports npm/yarn hoist a dependency to the highest node_modules that satisfies every consumer, so a sub-project's own node_modules/<pkg> frequently does not exist while the repo root's does. solc has no such resolver: the packages list must name a directory that exists. A bare `node_modules/...` remapping target is now looked for in base_dir first and then in each ancestor up to the run root — node's own order, bounded so the emitted path stays inside the tree certoraRun uploads. A target that resolves under base_dir always wins, so the walk can only change an entry whose target does not exist on disk. lib/ and dependencies/ targets are never walked: forge and soldeer do not hoist and a sibling project's lib/<name> is routinely a different pin. The run root now reaches the build-system managers as its own argument. It was being passed as the build-config dir, so for a monorepo sub-project — the only case where the two differ — both the remapping-context rebasing and the new walk would have been no-ops. utils/import_diagnostics.py classifies each `ParserError: Source "S" not found` against the packages the conf carried. solc names the source unit after remapping, so a target-prefix match on S decides whether a remapping fired and one is_dir() decides the rest: the package target is missing, the file inside an installed package is missing (rebuilding the list provably cannot help), the import is unmapped, or a project file is absent. The classification never gates a workaround — it names the class in the log, in the loop's giving-up message, and in the terminal compilation error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three fixes on the unresolved-import work, all in the same seam. last_import_diagnostics was only ever written on an output that had a source-not-found error, so a run whose imports were fixed and then died of something unrelated still carried the earlier classification — which setup_prover appends verbatim to CompilationAnalysisError and the no-progress message prints. Every exit from the workaround loop now refreshes the field from the output it is returning on, clearing it when that output has no source-not-found at all. The classifier contradicted the resolver for an installed package whose remapped subdirectory is absent: it tested is_dir() on the full remapped target only, so the case resolve_node_modules_target reports as `subpath_missing` (it found the package directory) was described as "the dependency is not installed there and was not found in any ancestor node_modules". That class now has its own kind, decided by the node_modules/<pkg> root above the target. _ancestor_roots took its step count from resolved paths while composing the candidates textually. When a base_dir reaches the run root through a symlink the two disagree, and the walk either climbs above the run root — emitting a package path outside the tree certoraRun uploads — or stops short of it and misses a hoisted package. The walk is now textual end to end and terminates on the run root itself. Tests: the ancestor-beats-nearer-package case now builds a real package directory (@pkg/artifacts is two segments, so the old fixture created no package at all and never reached the branch it named), plus coverage for subpath_missing, both symlink shapes, the package-root split, and an unrelated terminal failure after an import fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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.
Two related defects, both measured on a 569-contract corpus sweep
1. Hoisted packages are invisible.
build_packages_from_remapping_sourcesresolves anode_modules-derived target againstbase_dironly. npm/yarn workspaces hoist dependencies to anancestor
node_modules, and node's own resolution walks up the directory chain — ours did not. Amonorepo whose project dir is
<repo>/pkg-atherefore fails to resolve a package installed at<repo>/node_modules/..., and compilation dies withParserError: Source "node_modules/…" not foundfor a dependency that is on disk.
Of 54 rows classified
missing-deps/submodulein that sweep, 19 belong to projects whose JS installgenuinely failed and 14 to projects with no build step at all — but 21 had
node_modulespresent.That last group is what this fixes.
The walk starts at
base_dirand stops at the run root; it never escapes it. A package that resolvesunder
base_dirtoday resolves to exactly the same path — pinned by a test, since that is theproperty that keeps flat projects (the overwhelming majority) unchanged.
2. Unresolved imports all looked alike. Whether the remapping target directory is absent, no
remapping covers the import at all, or the package is installed but the remapped subdirectory inside
it is missing, the failure read the same and the reactive workaround treated them the same. New
utils/import_diagnostics.pyclassifies what the compiler output plus the filesystem can actuallydecide, and the classification is attached to the failure that produced it — cleared, not carried,
when the next attempt fails for an unrelated reason.
Tests
tests/test_import_diagnostics.py, plus additions totest_remappings.py,test_compilation_workarounds.pyand a newtest_build_system_run_root.py: flat-project byteequality, the hoisted case, the walk stopping at the run root, each diagnostic kind, and that a
diagnosis does not survive into an unrelated failure.
pytest -m "not expensive": 769 passed, 9 skipped.pyright: 0 errors.Composes with #164's context rebasing (same project-dir-vs-run-root family).