Skip to content

fix: exclude systemd files from merge output#1617

Merged
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
myml:fix-depend-systemd
Mar 17, 2026
Merged

fix: exclude systemd files from merge output#1617
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
myml:fix-depend-systemd

Conversation

@myml

@myml myml commented Mar 17, 2026

Copy link
Copy Markdown
Collaborator
  1. Modified mergeOutput function to accept an 'excludes' parameter for filtering unwanted directories during file merging
  2. Added logic to skip systemd-related files (lib/systemd, share/ systemd) during build stage
  3. This prevents unnecessary systemd service files from being included in the final package output
  4. Updated test to verify the exclusion mechanism works correctly

Log: Systemd service files are no longer included in application packages by default

Influence:

  1. Test package building to ensure systemd files are excluded from output
  2. Verify that other critical directories (bin/, lib/, sbin/) are still properly merged
  3. Test with various source directory structures to ensure exclusion logic handles edge cases
  4. Verify that existing packages still build correctly after this change
  5. Check that .wh.* files continue to be excluded as expected

fix: 从合并输出中排除 systemd 文件

  1. 修改 mergeOutput 函数,添加 'excludes' 参数用于在文件合并时过滤不需要 的目录
  2. 在构建阶段添加逻辑跳过 systemd 相关文件(lib/systemd, share/systemd)
  3. 这防止不必要的 systemd 服务文件被包含在最终软件包输出中
  4. 更新测试以验证排除机制正常工作

Log: Systemd 服务文件默认不再包含在应用程序包中

Influence:

  1. 测试软件包构建,确保 systemd 文件被正确排除
  2. 验证其他关键目录(bin/, lib/, sbin/)仍然正确合并
  3. 使用不同的源目录结构测试,确保排除逻辑能处理边界情况
  4. 验证现有软件包在此更改后仍然能正确构建
  5. 检查 .wh.* 文件是否按预期继续被排除

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 refines the package building process by introducing a robust exclusion mechanism within the mergeOutput function. The primary goal is to prevent unnecessary systemd service files from being bundled into application packages, leading to leaner and more focused distributions. This change enhances control over package content and improves build efficiency by filtering out specified directories.

Highlights

  • Function Signature Update: The mergeOutput function was modified to accept an additional excludes parameter, allowing for filtering unwanted directories during file merging.
  • Systemd File Exclusion: Logic was added to skip systemd-related files (specifically lib/systemd and share/systemd) during the build stage, preventing their inclusion in the final package output.
  • Test Coverage: Existing tests were updated to verify that the new exclusion mechanism works correctly, ensuring systemd files are indeed omitted from the merged output.
Changelog
  • libs/linglong/src/linglong/builder/linglong_builder.cpp
    • Updated the mergeOutput function signature to include a new excludes parameter.
    • Implemented logic within mergeOutput to filter out paths that match any of the provided exclusion patterns.
    • Modified the buildStagePreCommit function to utilize the new excludes parameter, specifically excluding lib/systemd and share/systemd directories.
  • libs/linglong/src/linglong/builder/linglong_builder.h
    • Updated the declaration of the mergeOutput function to reflect the addition of the excludes parameter.
  • libs/linglong/tests/ll-tests/src/linglong/builder/linglong_builder_test.cpp
    • Added test setup to create a lib/systemd directory and a dbus.service file within it.
    • Modified the mergeOutput test call to pass lib/systemd as an excluded path.
    • Added an assertion to confirm that the lib/systemd/dbus.service file is not present in the merged output, verifying the exclusion logic.
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.

@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

本次 PR 正确地实现了在 mergeOutput 过程中排除特定路径(特别是 systemd 相关文件)的功能。对 mergeOutput 函数、其声明以及相应测试用例的修改是符合逻辑的,并解决了所述问题。我有一个建议,可以提高路径匹配逻辑的健壮性,以防止因部分路径匹配而导致的潜在问题。

Comment on lines +177 to +179
if (common::strings::starts_with(path.string(), exclude)) {
return false;
}

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.

medium

当前的路径排除逻辑 common::strings::starts_with(path.string(), exclude) 存在两个潜在问题:

  1. 精确性不足:它会错误地匹配前缀相同的路径。例如,若排除规则为 "lib/foo",路径 "lib/foobar/some.file" 也会被错误地排除。
  2. 平台依赖path.string() 在不同平台(如 Windows)上可能返回带反斜杠的路径,导致与使用正斜杠的 exclude 规则匹配失败。

建议改用 path.generic_string() 并检查完整的目录匹配,以提高健壮性和平台兼容性。

if (auto path_str = path.generic_string(); path_str == exclude || common::strings::starts_with(path_str, exclude + "/")) {
    return false;
}

1. Modified mergeOutput function to accept an 'excludes' parameter for
filtering unwanted directories during file merging
2. Added logic to skip systemd-related files (lib/systemd, share/
systemd) during build stage
3. This prevents unnecessary systemd service files from being included
in the final package output
4. Updated test to verify the exclusion mechanism works correctly

Log: Systemd service files are no longer included in application
packages by default

Influence:
1. Test package building to ensure systemd files are excluded from
output
2. Verify that other critical directories (bin/, lib/, sbin/) are still
properly merged
3. Test with various source directory structures to ensure exclusion
logic handles edge cases
4. Verify that existing packages still build correctly after this change
5. Check that .wh.* files continue to be excluded as expected

fix: 从合并输出中排除 systemd 文件

1. 修改 mergeOutput 函数,添加 'excludes' 参数用于在文件合并时过滤不需要
的目录
2. 在构建阶段添加逻辑跳过 systemd 相关文件(lib/systemd, share/systemd)
3. 这防止不必要的 systemd 服务文件被包含在最终软件包输出中
4. 更新测试以验证排除机制正常工作

Log: Systemd 服务文件默认不再包含在应用程序包中

Influence:
1. 测试软件包构建,确保 systemd 文件被正确排除
2. 验证其他关键目录(bin/, lib/, sbin/)仍然正确合并
3. 使用不同的源目录结构测试,确保排除逻辑能处理边界情况
4. 验证现有软件包在此更改后仍然能正确构建
5. 检查 .wh.* 文件是否按预期继续被排除
@myml myml force-pushed the fix-depend-systemd branch from 371c4bf to 6a62af2 Compare March 17, 2026 02:25
@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

这段代码的修改主要是为 mergeOutput 函数增加了一个排除列表(excludes)功能,以便在合并输出时能够排除特定路径(如 systemd 相关文件)。以下是对这段代码的审查意见:

1. 语法逻辑

  • 修改正确:在 linglong_builder.cpplinglong_builder.h 中,mergeOutput 函数的签名和调用点均已正确更新,增加了 const std::vector<std::string> &excludes 参数。
  • Lambda 捕获:在 mergeOutput 内部定义的 matcher lambda 表达式正确捕获了新增的 excludes 变量 [&dir, &targets, &excludes],逻辑上没有问题。
  • 测试覆盖:在 linglong_builder_test.cpp 中,测试用例 mergeOutput 增加了对排除路径的验证(创建了 lib/systemd 目录和文件,并断言其不存在于目标目录中),逻辑验证是完整的。

2. 代码质量

  • 参数顺序:将 excludes 放在 targets 之后作为最后一个参数是合理的,保持了向后兼容性(如果是重载)或清晰的参数扩展顺序。
  • 代码可读性:在 matcher 中,先检查 excludes 再检查 targets 的逻辑顺序是清晰的,符合"优先排除"的直觉。
  • 测试注释:测试代码中增加了创建 lib/systemd 的步骤,虽然简单,但配合最后的断言,足以说明测试意图。

3. 代码性能

  • 循环嵌套matcher 函数在遍历文件系统路径时被调用,内部包含两个 for 循环(一个遍历 excludes,一个遍历 targets)。
    • 潜在瓶颈:如果 excludestargets 列表非常大,或者文件系统中的文件数量非常多,这 $O(N \times M)$ 的复杂度(N是文件数,M是规则数)可能会成为性能瓶颈。
    • 建议:虽然目前看来列表长度都很短(如 {"lib/systemd", "share/systemd"}),性能不是问题。但为了代码的健壮性,如果未来规则数量可能增加,建议考虑使用更高效的数据结构(如 std::unordered_set)存储匹配前缀,或者使用 Trie 树结构进行前缀匹配,将匹配复杂度降低。
  • 字符串构造path.string()path.filename().string() 会产生临时字符串对象。在性能敏感场景下,可以尝试直接比较 path 对象或使用 path.native(),但这取决于 common::strings::starts_with 的实现。

4. 代码安全

  • 路径遍历风险:代码使用了 common::strings::starts_with(path.string(), exclude) 进行简单的字符串前缀匹配。
    • 风险:如果 excludepath 包含 .. 或符号链接,简单的字符串匹配可能无法准确反映预期的文件系统层级关系。例如,排除 lib/secret 可能会意外匹配到 lib_backup/secret(如果路径处理不当)。
    • 建议:虽然当前的使用场景(构建系统合并输出)通常受控,但为了更安全,建议对 exclude 规则和 path 进行规范化处理,确保比较的是规范化的绝对路径或相对于根目录的规范路径。
  • 版权年份更新:测试文件中的版权年份从 2025 更新到了 2026,这通常是正确的(如果是为明年做准备),但请确认这是否符合项目的时间规划。

5. 其他建议

  • 硬编码字符串:在 buildStagePreCommit 中调用时,excludes 列表是硬编码的 {"lib/systemd", "share/systemd"}
    • 建议:如果这些排除规则可能在其他地方复用,或者需要根据配置动态调整,建议将其提取为常量或配置项,而不是硬编码在函数调用中。

总结

这段代码修改在功能上是正确的,逻辑清晰,并且补充了必要的测试用例。主要关注点在于性能优化(如果规则数量增加)和路径匹配的安全性(规范化处理)。在当前场景下,代码是可以接受的。

@codecov

codecov Bot commented Mar 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...linglong/src/linglong/builder/linglong_builder.cpp 50.00% 2 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
...s/linglong/src/linglong/builder/linglong_builder.h 0.00% <ø> (ø)
...linglong/src/linglong/builder/linglong_builder.cpp 2.47% <50.00%> (+0.14%) ⬆️
🚀 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, myml

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 1b35461 into OpenAtom-Linyaps:master Mar 17, 2026
15 of 17 checks passed
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