WIP: gnodev native loader#5604
Draft
gfanton wants to merge 50 commits intognolang:masterfrom
Draft
Conversation
Implements pkgdownload.PackageFetcher backed by an in-memory map of MemPackages. Replaces the ad-hoc mock resolvers in downstream tools. Assisted-By: Claude (claude-opus-4-7)
- Add compile-time interface assertion (matches examplespkgfetcher). - Doc FetchPackage as implementing PackageFetcher (Effective Go). - Note duplicate-Path "last wins" semantic on NewInMemoryFetcher. Assisted-By: Claude (claude-opus-4-7)
Preparation for replacing the resolver-chain loader with a single Loader type that delegates to gnovm's native loader. Assisted-By: Claude (claude-opus-4-7)
Temporary name; renamed to Package in Phase D after legacy Package is removed. Backs filesystem-resolved packages with Dir + Kind and mock/RPC packages with an embedded MemPackage. Assisted-By: Claude (claude-opus-4-7)
Walks up from start dir looking for gnowork.toml or gnomod.toml. Keeps gnodev independent of unexported gnovm workspace-discovery internals. Assisted-By: Claude (claude-opus-4-7)
Per code-review. Also drops the now-redundant "// path/file.go" header lines at the top of both files. Assisted-By: Claude (claude-opus-4-7)
Side-by-side with the legacy Loader interface: new LoaderImpl struct delegates to gnovm.Load for bulk workspace loading and tracks packages in an index for future Resolve/Reload methods. Assisted-By: Claude (claude-opus-4-7)
Adds the fast path: RLock-only lookup in the in-memory index, with a double-checked write-lock cold path ready to host FS and RPC lookups in subsequent commits. Introduces ErrPackageNotFound. Assisted-By: Claude (claude-opus-4-7)
Scans each ExtraRoot (and GNOROOT/examples when Examples=true) for a gnomod.toml matching the requested import path. Walks each root at most once and caches the import-path -> dir map on the LoaderImpl. Assisted-By: Claude (claude-opus-4-7)
After FS walk misses, Resolve falls back to the configured pkgdownload.PackageFetcher. A successful fetch yields a KindRemote package that is memoized in the index and added to tracked paths. Assisted-By: Claude (claude-opus-4-7)
Reload re-runs gnovm.Load for the configured workspace and re-Resolves every tracked path so packages discovered through ExtraRoots or the fetcher are refreshed. Dropping tracked entries from the index and invalidating per-root scan caches ensures on-disk changes are picked up. Assisted-By: Claude (claude-opus-4-7)
Eagerly loads workspace + every ExtraRoot + GNOROOT/examples. Extra roots are materialized via per-root index + Resolve to bypass gnovm.Load's "pattern must be inside workspace" constraint. Assisted-By: Claude (claude-opus-4-7)
- rpcLookup renamed (was rpcLookupLocked): name no longer implies a lock contract since l.fetcher is immutable after NewLoaderImpl. - scanRoot logs truly unparseable gnomod.toml at debug (still silent on "no mod file", which is normal during a walk). - TestLoader_Resolve_MissReturnsNotFound now passes an empty InMemoryFetcher so the RPC fallback fails fast instead of reaching real gno.land (previous run: ~0.78s → now sub-millisecond). Assisted-By: Claude (claude-opus-4-7)
-no-examples skips $GNOROOT/examples entirely. -extra-root is repeatable; each value is added to packages.Config.ExtraRoots. Not yet wired to the loader — Phase C later tasks consume these. Assisted-By: Claude (claude-opus-4-7)
node.go no longer imports packages.Loader. Caller supplies the initial pkg list plus a reload function. generateTxs and ListPkgs migrated to *NewPackage; pkg.MemPackage.Path → pkg.ImportPath. Build is intentionally broken at this commit — callers are fixed in the next two commits. Assisted-By: Claude (claude-opus-4-7)
Drops the -resolver chain in favor of workspace detection + -no-examples/-extra-root flags. Workspace-absent case warns and continues in discovery mode unless -no-examples + no roots leaves nothing to load, which is fatal. Lazy mode hydrates only the workspace at startup and relies on the proxy + Reload to fill in the rest; eager mode (-lazy-loader=false) calls LoadAll for both the initial pkg set and subsequent reloads, which is the staging subcommand's default. guessPath moves out of setup_loader.go (slated for deletion in Phase D) into a standalone paths.go so the helper survives. Assisted-By: Claude (claude-opus-4-7)
Staging eager-loads workspace + examples + extra-roots via LoadAll. The eager path is selected in app.Setup by lazyLoader=false, which is already the default for staging. Drops the legacy resolver-chain seeding that no longer has a consumer. Assisted-By: Claude (claude-opus-4-7)
Dir (was Location), KindFS (was PackageKindFS). Legacy pkg/dev tests (node_test.go, node_state_test.go) gated behind the phase_e_tests build tag: they depend on the removed cfg.Loader field and the MockResolver/BaseLoader chain. Phase E rewrites them against NodeConfig.InitialPkgs + a stub Reload. Assisted-By: Claude (claude-opus-4-7)
BREAKING: -resolver and -lazy-loader are removed from the user-facing flag surface. Use -extra-root to add package roots; lazy mode is the default (what today's gnodev does). `gnodev staging` continues to eager-load via an internal staging flag. Internal AppConfig.lazyLoader renamed to staging with inverse sense: staging=true eager-loads everything (LoadAll), staging=false lazy-loads (LoadWorkspace + proxy-driven Reload). Assisted-By: Claude (claude-opus-4-7)
Legacy resolver system (Local/Remote/Root/Mock + chain/middleware). Replaced by the native Loader's single-path resolution in Phase B. Also drop the last live caller of NewLocalResolver in command_local.go: the resolver was only used for its path-validity side effect, replaced by a direct gnolang.ReadMemPackage check. Assisted-By: Claude (claude-opus-4-7)
BaseLoader/GlobLoader replaced by LoaderImpl which delegates to gnovm.Load for pattern expansion. Assisted-By: Claude (claude-opus-4-7)
…interface Final sweep of code replaced by the native Loader + NewPackage. Leaves contribs/gnodev/pkg/packages/ with only the new Loader/Package types, Config, FindWorkspace, and their tests. Assisted-By: Claude (claude-opus-4-7)
Temporary names introduced in Phase B to avoid collision with the legacy types; those are now deleted. Final rename makes the public surface match the ADR and allows the new Loader to be imported under its natural name. Also updates gnodev's LongHelp to drop the removed -resolver flag documentation. Assisted-By: Claude (claude-opus-4-7)
Removes the phase_e_tests build tag. Legacy MockResolver/BaseLoader seeding replaced with a pkgsFromMem test helper that builds []*packages.Package via the new Loader + InMemoryFetcher. The testing Reload closure reads the node's live paths slice directly (not via Paths(), which would deadlock under the write lock held by Reset/rebuildNodeFromState) and rebuilds a fresh Loader on each call so in-place MemPackage.Files mutations are observed. Assisted-By: Claude (claude-opus-4-7)
Adds TestGnodev_Workspace_EagerLoad: a workspace root containing a gnowork.toml and one package, verifies Loader.LoadWorkspace returns that package. Assisted-By: Claude (claude-opus-4-7)
Adds TestGnodev_NoWorkspace_DiscoveryMode: with Workspace="" + ExtraRoots, verifies LoadWorkspace returns nil,nil and that Resolve still succeeds against the extra root. Assisted-By: Claude (claude-opus-4-7)
…extra-root Adds TestGnodev_NoWorkspace_NoExamples_ConfigError: when the cwd is outside any workspace, -no-examples is set, and no -extra-root is provided, app.Setup must fail fast with a "nothing to load" error per the ADR error model. Assisted-By: Claude (claude-opus-4-7)
Adds TestGnodev_Staging_EagerAll: verifies Loader.LoadAll returns both the workspace package and the extra-root package when Examples is disabled (to keep the test hermetic). Assisted-By: Claude (claude-opus-4-7)
Adds TestGnodev_Reload_AfterProxyHit: simulates a proxy hit by calling Resolve on an extra-root path after LoadWorkspace, then verifies Loader.Reload returns both the workspace package and the tracked extra-root package. Assisted-By: Claude (claude-opus-4-7)
Architecture Decision Record describing the replacement of gnodev's custom package loader/resolver subsystem with the gnovm native loader (PR gnolang#4957). Captures the design, alternatives considered, and consequences that guided the implementation. Status flipped to Accepted after all implementation phases (InMemoryFetcher upstream, new Loader type, wiring, legacy deletion, integration tests) landed and passed local verification. Assisted-By: Claude (claude-opus-4-7)
The field was written at 3 call sites but never read inside pkg/dev. The watcher seeds from ds.devNode.ListPkgs() (reads n.pkgs populated by Reset -> Reload), not from InitialPkgs. Startup parse errors still surface because NewDevNode calls Reset which calls Reload. Assisted-By: Claude (claude-opus-4-7)
Findings aggregated from code-reuse, code-quality, and efficiency sub-reviews on the branch: - Replace stringSliceVar with commands.StringArr (already in the repo). Deletes contribs/gnodev/flags.go entirely. - Replace extractPackageName's inline go/parser call with gnolang.PackageNameFromFileBody (same behavior, fewer imports). - Cache gnomod.ModCachePath() on the Loader instead of re-resolving it on every package classification. - scanRoot now skips hidden dirs (.git, etc.), node_modules, and _build — avoids descending into VCS/build trees on user-supplied extra roots. Matches files by name (gnomod.toml) instead of stat-ing every directory. - scanRoot now surfaces top-level WalkDir errors at Warn. - Drop the stray "// path/file.go" header comment left at the top of workspace.go. Assisted-By: Claude (claude-opus-4-7)
…staging help loadWithPatterns took an RLock to snapshot l.fetcher, but l.fetcher is set once in New() and never mutated — same treatment as l.cfg, which is read lock-free throughout the loader. Remove the unnecessary lock and note the invariant in a comment. Also update the staging command's LongHelp: "lazy-load" is no longer a user-facing concept in the native-loader design (ADR removes the -lazy-loader flag). Replace with the ADR's own phrasing describing what staging eager-loads. Assisted-By: Claude (claude-opus-4-7)
Running "gnodev" in a loose directory that has .gno files but no gnomod.toml/gnowork.toml ancestor previously added the CWD's path to cfg.paths without adding the directory to the loader's extra roots. The node then referenced a path it couldn't resolve. This restores the pre-PR "just run it anywhere" UX: when ReadMemPackage succeeds on CWD and no workspace is detected, CWD is appended to cfg.extraRoots so Loader.Resolve can find it. Assisted-By: Claude (claude-opus-4-7)
Two production bugs uncovered by running gnodev in examples/gno.land/r/gnoland/boards2/v1: 1. Native-stdlib imports (e.g. "chain", "chain/runtime") left PkgList.Sort() unable to find the "dep" because gnovm.Load skips native stdlibs during dep traversal but leaves them in each package's Imports slice. Fix: new stripStdlibs helper drops stdlib packages from the returned list and stdlib imports from each remaining pkg's import list, mirroring the convention already used by gno.land/pkg/gnoland/genesis.go (via ReadPkgListFromDir). 2. ToMemPackage used MPUserAll, which includes *_test.gno and *_filetest.gno files. Chain-side type-check then fails when a test file imports a package whose tests reference packages not deployed yet (seen with bptree's tree_test.gno importing avl). gnodev is a dev-time tool; deployed packages don't need test files. Switch to MPUserProd / MPStdlibProd. Adds TestLoader_LoadWorkspace_WithStdlibImport as a regression test for (1): a workspace package importing "chain" no longer errors. Assisted-By: Claude (claude-opus-4-7)
Reload was resetting l.rootIdx on every call, forcing every watcher event to re-walk every extra root. With large extra-roots this is multi-second per save. Directories are stable mid-session — picking up new dirs requires a gnodev restart, which is the documented behavior. Update ADR root-scan-caching section to match. Assisted-By: Claude (claude-opus-4-7)
Previously a `var p Package` or forgotten `Package{...}` literal would
silently report Kind == KindFS, letting the watcher try to watch a
nonexistent dir. Adding KindUnknown as the iota=0 sentinel makes such
bugs trip loudly at read time.
Assisted-By: Claude (claude-opus-4-7)
Closes the migration gap left by removing the old -resolver flag. The override applies only to packages outside the workspace: workspace pkgs hit fsLookup first via gnovm.Load, only unresolvable paths reach the rpc fetcher which consumes RemoteOverrides. Repeatable. Validated at parse time (domain and rpc both required). Assisted-By: Claude (claude-opus-4-7)
Replaces the buried slog warn with a pre-slog stderr banner. The user gets a distinct, actionable startup message explaining what discovery mode is and how to add local packages, instead of a single line lost in the log stream. Lazy examples still load in discovery mode (no behavior change there). Assisted-By: Claude (claude-opus-4-7)
Without this check, running gnodev with -no-examples on a workspace that imports gno.land/r/demo/* silently boots, then panics at first query. The new pre-flight scans workspace package imports, attempts to resolve each gno.land/* path via the loader, and warns (non-fatal — user may be stubbing) for each unresolvable one with a clear hint. Assisted-By: Claude (claude-opus-4-7)
Staging mode walks every extra root + examples; on real systems this takes several seconds with no output, looking hung. Add INFO logs at the start and end of each root (and the workspace) with package counts. Assisted-By: Claude (claude-opus-4-7)
All prior loader tests used trivial 'package foo' stubs with zero imports. Real realms expose code paths that those don't — stripStdlibs on native imports, MPUserProd on test-file handling. This test loads boards2/v1 from $GNOROOT/examples and asserts the realm loads, the realm package is in the result, ToMemPackage succeeds, and no test files leak through. Skips cleanly if examples aren't available (running outside monorepo). Assisted-By: Claude (claude-opus-4-7)
CheckMissingExampleImports previously called l.Resolve, which falls through
to the rpc fetcher on FS miss and writes to the loader's index/tracked sets.
Two consequences:
- Blocking RPC at startup: with -no-examples and several broken imports,
gnodev startup synchronously hits https://rpc.<domain>:443 per missing
import, blocking on TCP connects and timeouts.
- State pollution: a diagnostic helper must not mutate the system it
observes; index entries seeded by a startup probe outlive the check.
Add Loader.LookupFS as the read-only sibling of Resolve: walks any not-yet-
cached root, consults rootIdx, never touches the fetcher, never writes to
index or tracked. Switch CheckMissingExampleImports to use it.
Assisted-By: Claude (claude-opus-4-7)
Reload preserves rootIdx across calls (deliberately, so directories don't get re-walked on every reload). Document that the flip side is staleness: if an extra-root dir is deleted mid-session, Resolve will keep returning the cached dir until gnodev restarts. Assisted-By: Claude (claude-opus-4-7)
The String() method iterated the map directly, producing non-deterministic output across calls. flag.Value implementations are read by the flag package to print defaults and may be logged for diagnostics, so the output should be stable. Sort keys lexicographically before joining and add a regression test. Assisted-By: Claude (claude-opus-4-7)
The per-root progress log in LoadAll built its progress field via
fmt.Sprintf("%d/%d", ...), which packs two values into one opaque string.
Slog handlers can't filter or extract the components separately. Emit n
and of as distinct integer fields so log consumers can index on either.
Strengthen the existing test to assert the new structured shape (n=1,
of=1) rather than only the message and root path.
Assisted-By: Claude (claude-opus-4-7)
…rts non-mutation Lock in the contract that the -no-examples diagnostic helper does not mutate l.index or l.tracked, and that LookupFS never invokes the rpc fetcher. Without these, a future revert to l.Resolve(imp) would silently restore the blocking-RPC + state-pollution bug fixed in 2f21494. Assisted-By: Claude (claude-opus-4-7)
When run in a directory without gnomod.toml, gnodev derives a fallback import path of gno.land/r/dev/<basename>. The previous regex preserved hyphens, uppercase letters, and digit-leading names — all rejected by gno's Re_name validator — causing ReadMemPackage to panic at startup. Sanitize the basename to lowercase letters/digits/underscore only and ensure it starts with a letter. Fall back to "app" for inputs with no letters. Assisted-By: Claude (claude-opus-4-7)
github-project-automation
Bot
added this to
💪 Bounties & Worx and 🧙♂️Gno.land development
Collaborator
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Replaces the help-text section for -resolver / -lazy-loader (removed in this PR) with -extra-root, -no-examples, and -remote-override. The staging mode description is updated to reflect eager workspace + extra-roots + examples loading. Produced by `make generate` in contribs/gnodev/. Assisted-By: Claude (claude-opus-4-7)
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
2 participants
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.
TODO
replace and close #4957