fix: exclude systemd files from merge output#1617
Conversation
Summary of ChangesHello, 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 Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
| if (common::strings::starts_with(path.string(), exclude)) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
当前的路径排除逻辑 common::strings::starts_with(path.string(), exclude) 存在两个潜在问题:
- 精确性不足:它会错误地匹配前缀相同的路径。例如,若排除规则为
"lib/foo",路径"lib/foobar/some.file"也会被错误地排除。 - 平台依赖:
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.* 文件是否按预期继续被排除
371c4bf to
6a62af2
Compare
deepin pr auto review这段代码的修改主要是为 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
5. 其他建议
总结这段代码修改在功能上是正确的,逻辑清晰,并且补充了必要的测试用例。主要关注点在于性能优化(如果规则数量增加)和路径匹配的安全性(规范化处理)。在当前场景下,代码是可以接受的。 |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Log: Systemd service files are no longer included in application packages by default
Influence:
fix: 从合并输出中排除 systemd 文件
Log: Systemd 服务文件默认不再包含在应用程序包中
Influence: