feat(modules): add modules for module resolution #61

MeteorsLiu · 2026-01-05T03:46:44Z

Implement classfileCache for loading and caching formulas with lazy downloading from remote repositories
Add mvsReqs implementing MVS (Minimal Version Selection) algorithm for dependency version resolution
Support custom version comparators per module
Include comprehensive tests for version comparison, upgrades, and build list generation

- Implement classfileCache for loading and caching formulas with lazy downloading from remote repositories - Add mvsReqs implementing MVS (Minimal Version Selection) algorithm for dependency version resolution - Support custom version comparators per module - Include comprehensive tests for version comparison, upgrades, and build list generation

gemini-code-assist · 2026-01-05T03:47:10Z

Summary of Changes

Hello @MeteorsLiu, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request delivers a foundational module resolution framework. It enables dynamic loading and caching of module 'formulas,' resolves dependencies using the MVS algorithm, and supports custom version comparison rules. This system is designed to provide a flexible and efficient way to manage and integrate external modules, complete with mechanisms for remote fetching and local caching.

Highlights

Module Resolution System: Introduced a new module resolution system capable of loading and caching 'formulas' (likely module definitions or configurations) and their dependencies.
Minimal Version Selection (MVS): Implemented the Minimal Version Selection (MVS) algorithm for robust dependency resolution, ensuring that the highest compatible version of each module is selected.
Custom Version Comparators: Added support for custom version comparators, allowing modules to define their own logic for comparing versions, which can be dynamically loaded.
Formula Caching and Lazy Downloading: Developed a classfileCache to manage the loading, caching, and lazy downloading of formulas from remote repositories, improving performance and efficiency.
Comprehensive Testing: Included extensive unit tests covering module initialization, version comparison logic, dependency resolution, and directory management, ensuring the reliability of the new system.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a comprehensive module resolution system, including dependency resolution with MVS, formula caching, and custom version comparators. The implementation is quite extensive and covers many aspects of module management. My review has identified a few critical issues, such as potential panics from unhandled errors and unsafe type assertions, which should be addressed. I've also noted some areas for improvement in terms of error handling, code clarity, and testing robustness. Overall, this is a significant feature addition, and with the suggested fixes, it will be a solid foundation for module management.

gemini-code-assist · 2026-01-05T03:49:28Z

internal/modules/deps.go

+	}
+	repoFS := repo.At(mainMod.Version, sourceCacheDir)
+	proj := &classfile.Project{
+		FileFS: repoFS.(fs.ReadFileFS),


The unchecked type assertion repoFS.(fs.ReadFileFS) can cause a panic if the fs.FS instance returned by repo.At does not also implement fs.ReadFileFS. This could crash the application.

proj := &classfile.Project{ FileFS: repoFS.(fs.ReadFileFS), }

gemini-code-assist · 2026-01-05T03:49:28Z

internal/modules/load.go

+		}
+		compare, err := cache.comparatorOf(p)
+		if err != nil {
+			panic(err)


Calling panic(err) here is dangerous, as it will crash the entire application for what could be a recoverable error (e.g., a network issue when fetching a comparator). Errors from cache.comparatorOf(p) should be handled gracefully. Since the cmp function signature doesn't allow returning an error, you might need to reconsider the design, perhaps by pre-loading all necessary comparators before starting the MVS process, or by modifying the MVS requirement interface to support fallible comparisons.

gemini-code-assist · 2026-01-05T03:49:28Z

internal/modules/cache.go

+	var comp func(v1 module.Version, v2 module.Version) int
+
+	for _, searchPath := range seachPaths {
+		comp, err = loadComparator(searchPath)


The loadComparator function expects a file path to a *_cmp.gox file, but here it's being called with searchPath, which is a directory. This will cause loadComparator to fail. The logic should be updated to search for a comparator file (e.g., *_cmp.gox) within each searchPath directory and then pass the found file path to loadComparator.

gemini-code-assist · 2026-01-05T03:49:28Z

internal/modules/comparator.go

+
+	val.Interface().(interface{ Main() }).Main()
+
+	return valueOf(class, "fCompareVer").(func(v1, v2 module.Version) int), nil


Using unsafe and reflection to access the unexported field fCompareVer is highly fragile. This creates a strong dependency on the internal implementation details of the GoPlus compiler, which could change in future versions and break this code. It would be more robust to rely on a public, stable API provided by the GoPlus toolchain to access compiled functions.

gemini-code-assist · 2026-01-05T03:49:28Z

internal/modules/mock_test.go

+	if err := os.MkdirAll(destDir, 0755); err != nil {
+		return err
+	}
+	return os.CopyFS(destDir, os.DirFS(srcDir))


The function os.CopyFS does not exist in the standard os package, which will cause a compilation error in the tests. You may need to implement a helper function to copy the contents of an fs.FS to a destination directory or use a function from a third-party library. This issue also appears in cache_test.go.

gemini-code-assist · 2026-01-05T03:49:28Z

internal/modules/cache.go

+	// 		├── libpng_cmp.gox
+	// 		└── versions.json
+	// so modId is the sub directory of a module
+	return m.formulaRepo.Sync(context.TODO(), "main", modId, moduleDir)


Using context.TODO() is generally discouraged in production code. The context should be passed down from the caller to allow for proper cancellation and deadline propagation. Please consider updating lazyDownloadFormula and its callers (comparatorOf) to accept a context.Context.

Suggested change

return m.formulaRepo.Sync(context.TODO(), "main", modId, moduleDir)

return m.formulaRepo.Sync(ctx, "main", modId, moduleDir)

gemini-code-assist · 2026-01-05T03:49:29Z

internal/modules/cache.go

+	if err != nil {
+		return nil, err
+	}
+	seachPaths := append([]string{moduleDir}, m.searchPaths...)


There is a typo in the variable name seachPaths. It should be searchPaths. This typo also appears on lines 87, 133, and 135.

Suggested change

seachPaths := append([]string{moduleDir}, m.searchPaths...)

searchPaths := append([]string{moduleDir}, m.searchPaths...)

gemini-code-assist · 2026-01-05T03:49:29Z

internal/modules/deps.go

+			if idx < 0 {
+				// It seems safe to drop deps here, because we resolve deps recursively and finally we will find that dep.
+				continue


Silently dropping a dependency by using continue when its version cannot be resolved can hide configuration errors and lead to unexpected behavior. It would be better to at least log a warning to inform the user that a dependency specified in onRequire was not found in versions.json and is being ignored.

Replace all references to module.Version.ID with module.Version.Path across the modules package. This change improves code clarity by using a more semantically accurate field name, as the field contains module paths (e.g., "github.com/user/repo") rather than generic identifiers. Changes include: - Updated cache.go: comparatorOf, formulaOf, findMaxFromVer functions - Updated all test files to use Path field in module.Version literals - Updated error messages to reference mod.Path instead of mod.ID

gemini-code-assist bot reviewed Jan 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(modules): add modules for module resolution #61

feat(modules): add modules for module resolution #61

Uh oh!

MeteorsLiu commented Jan 5, 2026

Uh oh!

gemini-code-assist bot commented Jan 5, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jan 5, 2026

Uh oh!

gemini-code-assist bot Jan 5, 2026

Uh oh!

gemini-code-assist bot Jan 5, 2026

Uh oh!

gemini-code-assist bot Jan 5, 2026

Uh oh!

gemini-code-assist bot Jan 5, 2026

Uh oh!

gemini-code-assist bot Jan 5, 2026

Uh oh!

gemini-code-assist bot Jan 5, 2026

Uh oh!

gemini-code-assist bot Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant


		val.Interface().(interface{ Main() }).Main()

		return valueOf(class, "fCompareVer").(func(v1, v2 module.Version) int), nil

	return m.formulaRepo.Sync(context.TODO(), "main", modId, moduleDir)
	return m.formulaRepo.Sync(ctx, "main", modId, moduleDir)

	seachPaths := append([]string{moduleDir}, m.searchPaths...)
	searchPaths := append([]string{moduleDir}, m.searchPaths...)

feat(modules): add modules for module resolution #61

Are you sure you want to change the base?

feat(modules): add modules for module resolution #61

Uh oh!

Conversation

MeteorsLiu commented Jan 5, 2026

Uh oh!

gemini-code-assist bot commented Jan 5, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant