Skip to content

fix: Use module options when exporting UAB#1595

Merged
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_export_uab
Feb 27, 2026
Merged

fix: Use module options when exporting UAB#1595
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_export_uab

Conversation

@reddevillg

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: reddevillg <reddevillg@gmail.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @reddevillg, I'm Gemini Code Assist1! 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 addresses a specific issue in the UAB export process. It refines how module-specific layers are handled during distributed-only exports, ensuring that all relevant modules are correctly processed and included in the final UAB package. This change ensures the integrity and completeness of UABs generated under these conditions.

Highlights

  • UAB Export Logic: Modified the exportUAB function to correctly iterate through exportOpts.modules when distributedOnly is true, ensuring that getLayerDir and appendLayer are called for each specified module, rather than just once.

🧠 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.

Changelog
  • libs/linglong/src/linglong/builder/linglong_builder.cpp
    • Corrected the UAB export logic to properly iterate and append layers for all specified modules in distributed-only mode.
Activity
  • No human activity has been recorded on this pull request yet.
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.

Footnotes

  1. 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.

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

这段代码主要是修改了 exportUAB 函数中处理分层导出的逻辑,将原本只处理单个层的逻辑改为遍历 exportOpts.modules 来处理多个层。以下是对这段代码的审查意见:

1. 语法逻辑

  • 问题:变量作用域和命名冲突

    • for 循环内部定义了 auto ret,而在循环外部又定义了一个同名的 auto ret。虽然这在 C++ 中是合法的(外部的 ret 会遮蔽内部的),但这种写法容易引起混淆,降低了代码的可读性。
  • 改进建议

    • 建议将循环内的 ret 重命名为 layerRetappendRet,以明确其用途。
    • 或者,如果 appendLayer 的错误处理逻辑允许,可以在循环内直接返回错误,避免重复定义。

2. 代码质量

  • 问题:重复的错误处理代码

    • if (!layerDir)if (!ret) 的错误处理逻辑在循环内重复出现,虽然这是必要的,但可以通过提取公共逻辑来减少重复。
  • 改进建议

    • 如果 appendLayer 的错误处理逻辑一致,可以考虑使用宏或辅助函数来简化错误处理。

3. 代码性能

  • 问题:潜在的 I/O 性能问题

    • packager.appendLayer(*layerDir) 在循环内被多次调用,如果 exportOpts.modules 包含大量模块,可能会频繁触发 I/O 操作,影响性能。
  • 改进建议

    • 如果 appendLayer 的 I/O 操作可以批量处理,建议优化 packager 的实现,减少 I/O 开销。
    • 如果无法优化,可以在循环内添加日志,记录每次 appendLayer 的耗时,便于后续性能分析。

4. 代码安全

  • 问题:输入验证不足

    • exportOpts.modules 的内容没有被验证,如果 modules 包含无效或恶意的模块名,可能会导致 getLayerDir 返回错误,甚至引发安全问题。
  • 改进建议

    • 在循环前验证 exportOpts.modules 的内容,确保其合法性和安全性。
    • 例如,可以检查模块名是否符合预期的格式,或者是否在允许的列表中。

5. 其他建议

  • 日志记录
    • 在循环内添加日志,记录当前正在处理的模块名,便于调试和问题排查。
    • 例如:
      for (const auto &module : exportOpts.modules) {
          LINGLONG_LOG("Processing module: " << module);
          auto layerDir = this->repo.getLayerDir(*curRef, module);
          // ...
      }

改进后的代码示例

if (distributedOnly) {
    for (const auto &module : exportOpts.modules) {
        auto layerDir = this->repo.getLayerDir(*curRef, module);
        if (!layerDir) {
            return LINGLONG_ERR(layerDir);
        }

        auto appendRet = packager.appendLayer(*layerDir);
        if (!appendRet) {
            return LINGLONG_ERR(appendRet);
        }
    }

    auto packRet = packager.pack(uabFile, true);
    if (!packRet) {
        return LINGLONG_ERR(packRet);
    }
}

总结

这段代码的功能是正确的,但在变量命名、错误处理和性能优化方面有改进空间。通过调整变量作用域、减少重复代码和增强输入验证,可以提升代码的可读性、安全性和性能。

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a bug in the UAB export functionality for distributed-only mode. Previously, the export process would only include the default 'binary' layer, disregarding any modules specified in the export options. The fix introduces a loop that correctly iterates through the modules provided in exportOpts.modules, appending each corresponding layer to the packager. After all specified module layers have been added, the pack function is called once to create the UAB file. This change ensures that the exported UAB correctly contains all requested modules. The implementation is sound and effectively resolves the issue.

@codecov

codecov Bot commented Feb 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...linglong/src/linglong/builder/linglong_builder.cpp 0.00% 8 Missing ⚠️
Files with missing lines Coverage Δ
...linglong/src/linglong/builder/linglong_builder.cpp 2.33% <0.00%> (-0.01%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11, reddevillg

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dengbo11 dengbo11 merged commit 419de9a into OpenAtom-Linyaps:master Feb 27, 2026
15 of 16 checks passed
@reddevillg reddevillg deleted the fix_export_uab branch March 17, 2026 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants