针对#issues6086导出整合包时内存溢出修复#6173
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors both McbbsModpackExportTask and ModrinthModpackExportTask to stream JSON output directly using JsonWriter instead of constructing in-memory manifest objects. The review feedback highlights critical issues, including potential resource leaks from unclosed Files.walk streams and a potential NullPointerException when handling nullable Java versions. Additionally, the reviewer suggests refactoring repetitive loader version checks into loops to improve code readability and maintainability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors both McbbsModpackExportTask and ModrinthModpackExportTask to write modpack manifests directly to temporary files using JsonWriter instead of serializing in-memory manifest objects. Feedback on these changes highlights several critical serialization bugs in the MCBBS export task, such as incorrect JSON keys (e.g., origins instead of origin, pluralized launch arguments, and supportedJavaVersions instead of supportJava), an incorrect type discriminator value, and missing default settings. Additionally, the reviewer notes that throwing a RuntimeException on remote file processing failures in the Modrinth export task could prematurely abort the export, and suggests refactoring ifPresent lambdas to standard loops or conditionals to avoid boilerplate try-catch blocks and RuntimeException wrapping.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the MCBBS and Modrinth modpack export tasks to stream JSON generation using JsonWriter to a temporary file, reducing memory overhead compared to the previous in-memory GSON serialization. However, several issues were identified in the review: using Files.walk instead of Files.walkFileTree introduces a severe performance regression by traversing blacklisted directories; writing explicit null values for optional fields like fileApi and summary may cause compatibility issues with strict parsers; and writing an empty origin array in the MCBBS manifest discards user-configured origin data.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors both McbbsModpackExportTask and ModrinthModpackExportTask to stream manifest and index JSON generation directly to a temporary file using JsonWriter instead of constructing large in-memory manifest objects. It also utilizes Files.walkFileTree for directory traversal. A critical issue was identified in ModrinthModpackExportTask where tryGetRemoteFile can throw an IOException that is no longer caught within the file visitor, which could cause the entire export task to fail on a single file error instead of logging a warning and continuing.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the MCBBS and Modrinth modpack export tasks to stream JSON metadata directly to temporary files using JsonWriter instead of building full in-memory manifest objects, which optimizes memory usage. It also replaces stream-based file walking with Files.walkFileTree. Feedback on these changes includes addressing a potential NullPointerException in McbbsModpackExportTask when iterating over tokenized launch and Java arguments, and optimizing the whitelist lookup in ModrinthModpackExportTask by converting the whitelist List to a HashSet to avoid O(N) performance overhead during directory traversal.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the MCBBS and Modrinth modpack export tasks to stream JSON manifests directly to temporary files using JsonWriter instead of building large in-memory objects. It also updates directory traversal to use Files.walkFileTree. The review feedback highlights critical issues in both export tasks where directory traversal (preVisitDirectory and zip.putDirectory) prematurely skips subtrees when a whitelist is present, because directory paths themselves are not in the whitelist. The reviewer recommends allowing traversal of non-blacklisted directories and applying the whitelist filter only to files.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors McbbsModpackExportTask and ModrinthModpackExportTask to stream their respective manifest files (mcbbs.packmeta and modrinth.index.json) directly to temporary files using JsonWriter instead of holding the entire manifest structures in memory. This is achieved by utilizing Files.walkFileTree to write files dynamically. A critical issue was identified in ModrinthModpackExportTask where enabling a disabled mod renames the file on disk but does not update the file reference, which can lead to a NoSuchFileException during subsequent operations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
|
你好！感谢你提交的关于修复大型整合包导出时 不过，我注意到你在评论区多次重复输入了 目前我已经根据你提供的补丁进行了分析，你的修复方案在逻辑上是正确的，使用了 如果后续有其他问题或需要进一步的反馈，请随时告知。 |
|
核心维护者们好，PR 中 CI 检查已全部通过。为了将 #6086 这个影响用户的 Bug 修复，这边的所有修改均已完成。请问是否还有其他地方需要调整？谢谢！ |
等一等，glavo这几天在赶其他功能的进度，过一阵应该会来审pr |
他是不是没看到？ |
|
我来审审 |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the MCBBS and Modrinth modpack export tasks to stream manifest and index JSON data directly to temporary files using JsonWriter, preventing potential OutOfMemoryError issues on large modpacks. It also introduces translation keys for Java version warnings and manual updates, and ensures that temporarily enabled mods are properly restored to their disabled state after export. The feedback suggests removing redundant and potentially leaky deleteOnExit() calls on the temporary files, as they are already safely deleted in finally blocks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the MCBBS and Modrinth modpack export tasks to stream manifest and index JSON files directly to temporary files using JsonWriter, significantly reducing memory usage and preventing OutOfMemoryError on large modpacks. It also adds translation keys for unsupported Java versions and manual updates. The review feedback points out a potential issue in ModrinthModpackExportTask where modifying the directory structure (enabling disabled mods) during Files.walkFileTree traversal can cause undefined behavior or exceptions, and suggests collecting the file paths first before processing them.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces localized boot properties and refactors both McbbsModpackExportTask and ModrinthModpackExportTask to stream manifest and index JSON files directly to temporary files using JsonWriter. This optimization avoids holding large amounts of file metadata in memory, preventing potential OutOfMemoryError issues on large modpacks. Additionally, the export task now temporarily enables disabled mods and restores them afterward. The review feedback suggests defensive handling of a potential NullPointerException when retrieving the whitelist, and recommends utilizing the ModManager API (enableMod and disableMod) rather than manual file operations to manage and restore the state of disabled mods consistently.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request refactors the MCBBS and Modrinth modpack export tasks to stream manifest and index JSON files directly to temporary files using JsonWriter, preventing OutOfMemoryErrors on large modpacks. It also adds localization files for the boot module. The review feedback points out two critical issues: first, the new translation files are incorrectly placed in the 'bin/main/' directory, which is prone to being deleted during clean builds; second, using relative paths when enabling or disabling mods in ModrinthModpackExportTask could fail if the working directory differs from the game directory, so absolute paths should be used instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
代码写多了啥啥都容易错😶 |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds bootloader localization files and refactors the MCBBS and Modrinth modpack export tasks to stream manifest and index JSON files directly to temporary files using JsonWriter, which prevents OutOfMemoryErrors on large modpacks. Feedback on the Modrinth export task identifies a critical issue where temporarily enabled mods are restored only in the finally block, leading to incorrect packaging and duplication in the final zip; a fix is suggested to restore these mods before zipping.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
a? |
|
让我审吗? |
|
ClaudeCode启动!
|
我代码能力稍弱，还请指教 |
|
对了 上次审查吃掉我的上下文: |
实在抱歉……我java实在不太行，偶尔还要查AI |
Minecraft269
left a comment
Minecraft269
left a comment
There was a problem hiding this comment.
审查概要
🤖 审查模型: DeepSeekV4
本次 PR 将整合包导出的内存模型从「全量加载到内存再序列化」改为「JsonWriter 流式写入临时文件」，从根本上解决了大型整合包的 OOM 问题。设计方向正确，流式写入是解决此类问题的标准方案。
总体评价
- ✅ 核心方案正确 —
JsonWriter流式写入是合适的选择 - ✅ Javadoc 清晰说明了设计权衡
- ✅
ModrinthModpackExportTask修复了 disabled mod 的路径问题（enableMod现在使用绝对路径） - ✅
whitelistnull 安全性改进 ⚠️ 有几处需要注意的问题（详见行内评论）- 🚨 1 个需要立即修复的问题
变更概要
| 文件 | 变更 | 审查重点 |
|---|---|---|
McbbsModpackExportTask.java |
+174/-59 | 流式 JSON + 两遍遍历 |
ModrinthModpackExportTask.java |
+168/-71 | 流式 JSON + disabled mod 恢复 |
| 4 个 boot 语言文件 | 新增 | i18n 内容完整性 |
git (空文件) |
意外新增 | 🚨 应移除 |
行内评论汇总
| 严重度 | 文件 | 问题 |
|---|---|---|
| 🚨 Critical | git |
意外提交的空文件，必须移除 |
ModrinthModpackExportTask.java:196 |
路径分隔符不一致导致 Windows 上 remoteFilePaths 匹配失效 |
|
McbbsModpackExportTask.java:118 |
info.getUrl() 等字段缺少 null 检查 |
|
| ℹ️ Info | boot_es.properties:19 |
缺少 boot.manual_update 和 boot.message.error 键 |
| ℹ️ Info | ModrinthModpackExportTask.java:170 |
processedPaths 去重在单目录遍历中冗余 |
| ℹ️ Info | McbbsModpackExportTask.java:144 |
Files.isRegularFile() 检查被移除，符号链接行为变更 |
额外发现（已有代码，非本次引入）
ModrinthModpackExportTask.tryGetRemoteFile() 中 (int) fileSize 的警告信息写的是 "precision may be lost"，但实际上 (int) 转换对于 >2.1GB 的文件是整数溢出，会产生无意义的错误值，不是精度丢失。建议改为 long 类型或溢出时跳过该文件。（此问题在 PR 之前已存在，建议单独修复）
| # along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
| # | ||
|
|
||
| boot.unsupported_java_version=HMCL requiere Java 17 o posterior para ejecutarse, pero sigue admitiendo el inicio de juegos con Java 6~16.\nPor favor, instala la última versión de Java e intenta ejecutar HMCL de nuevo.\nPuedes conservar tu versión antigua de Java. HMCL puede detectar y gestionar múltiples instalaciones de Java,\ny seleccionará automáticamente la versión de Java adecuada para tu juego. |
There was a problem hiding this comment.
ℹ️ 缺少 boot.manual_update 和 boot.message.error 键
其他三个语言文件（boot.properties, boot_zh.properties, boot_zh_Hant.properties）都包含这三个键：
boot.unsupported_java_versionboot.manual_updateboot.message.error
但 boot_es.properties（西班牙语）仅包含 boot.unsupported_java_version，缺少后两个。如果是刻意为之（尚未翻译），建议添加带英文 fallback 的占位符；如果是遗漏，请补全。
boot.manual_update=HMCL no puede completar las actualizaciones automáticas en el entorno actual. Por favor, descarga la última versión de HMCL manualmente.\n¿Deseas ir a la página de descarga?
boot.message.error=Error|
|
||
| writer.name("formatVersion").value(1); | ||
| writer.name("game").value("minecraft"); | ||
| writer.name("versionId").value(info.getVersion()); |
There was a problem hiding this comment.
ℹ️ processedPaths 去重逻辑在单目录遍历中是冗余的
在单次 Files.walkFileTree() 遍历中，visitFile() 不会被重复调用同一个文件。processedPaths 的检查仅在以下情况下有用：
- 同一个文件通过不同的路径（hardlink/symlink）被访问
- 或者将来有人在此循环中增加了其他遍历逻辑
这是无害的防御性编程，但增加了不必要的 HashSet 内存开销和 contains/add 操作。如果 resourceDirs（mods, resourcepacks, shaderpacks）之间没有重叠，可以考虑移除以简化代码。
|
这次审查好像没用Subagent 审查质量可能会低一点 本次审查使用的上下文: |
|
非常抱歉因为频繁推送给大家带来了困扰。所有审查意见已经修复完毕，后续我会注意减少不必要的更新。感谢各位的耐心 |
4 participants
问题：导出大型整合包时，HMCL 因
OutOfMemoryError卡死（见 #6086）。原因：
McbbsModpackExportTask和ModrinthModpackExportTask一次性将所有文件信息加载到内存生成 JSON，导致内存峰值过高修复：改用
JsonWriter流式写入临时文件，避免内存积累，并消除ModrinthModpackExportTask中重复的 API 调用和 I/O测试：已完成编译测试，本地通过且功能正常
关联：Fixes #6086