Skip to content

feat(ll-builder): add clean subcommand to remove build artifacts - #1789

Open
dengbo11 wants to merge 1 commit into
OpenAtom-Linyaps:masterfrom
dengbo11:add-build-clean
Open

feat(ll-builder): add clean subcommand to remove build artifacts#1789
dengbo11 wants to merge 1 commit into
OpenAtom-Linyaps:masterfrom
dengbo11:add-build-clean

Conversation

@dengbo11

@dengbo11 dengbo11 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator
  • Add CleanCommandOptions and register ll-builder clean with optional -f/--file
  • Validate project location through getProjectYAMLPath before cleaning
  • Implement cmdCleanBuildArtifacts to fix overlay workdir permissions via makeDirectoryTreeRemovable and remove the linglong/ directory

Log: Add a clean subcommand to ll-builder for removing the build-generated linglong directory and handling overlay workdir permission issues

- Add CleanCommandOptions and register `ll-builder clean` with optional `-f/--file`
- Validate project location through getProjectYAMLPath before cleaning
- Implement cmdCleanBuildArtifacts to fix overlay workdir permissions via makeDirectoryTreeRemovable and remove the linglong/ directory

Log: Add a clean subcommand to ll-builder for removing the build-generated linglong directory and handling overlay workdir permission issues
@dengbo11
dengbo11 requested a review from reddevillg August 6, 2026 13:52
@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11

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

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码完整实现了ll-builder的clean子命令功能,链路清晰且无安全漏洞
逻辑正确、代码规范、性能良好,符合优秀标准仅作微调扣5分

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

代码从CLI参数解析到核心删除逻辑的完整链路均无语法与逻辑错误。handleClean中正确处理了current_path可能失败的情况,cmdCleanBuildArtifacts中正确处理了目录不存在、权限修改失败、删除失败等边界条件
潜在问题:无
建议:无

  • 2.代码质量(良好)✓

新增的结构体、函数命名严格遵循项目既有风格(如handleExtract与ExtractCommandOptions),注释精简有效,错误信息使用fmt格式化且包含具体路径和系统错误码,便于排查
潜在问题:无
建议:无

  • 3.代码性能(无性能问题)✓

清理操作使用std::filesystem::remove_all,底层直接调用系统API进行递归删除,无多余的系统调用或内存分配,符合工具类操作的性能预期
潜在问题:无
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码在本地CLI环境下执行,未引入命令注入、路径遍历越权等漏洞。std::filesystem::remove_all严格遵循C++17标准,对符号链接仅删除链接本身而不跟随,有效防止了符号链接攻击

  • 建议:可增加对workingDir的weakly_canonical规范化处理,防止路径中包含..导致的逻辑混淆,提升防御性编程水平

■ 【改进建议代码示例】

diff --git a/libs/linglong/src/linglong/builder/linglong_builder.cpp b/libs/linglong/src/linglong/builder/linglong_builder.cpp
index 23b98bb1d..123456789 100644
--- a/libs/linglong/src/linglong/builder/linglong_builder.cpp
+++ b/libs/linglong/src/linglong/builder/linglong_builder.cpp
@@ -2288,10 +2288,15 @@ std::string Builder::layerExportFilename(const linglong::package::Reference &ref
                        ref.arch.toString(),
                        module);
 }
+
 utils::error::Result<void> cmdCleanBuildArtifacts(const std::filesystem::path &workingDir) noexcept
 {
     LINGLONG_TRACE("clean build artifacts");
 
-    auto linglongDir = workingDir / "linglong";
     std::error_code ec;
+    // 规范化路径,消除..等相对路径成分,防止路径混淆
+    auto canonicalWorkingDir = std::filesystem::weakly_canonical(workingDir, ec);
+    if (ec) {
+        return LINGLONG_ERR(fmt::format("failed to canonicalize workingDir: {}", ec.message()));
+    }
+    auto linglongDir = canonicalWorkingDir / "linglong";
     if (!std::filesystem::exists(linglongDir, ec)) {
         return LINGLONG_OK;

@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
apps/ll-builder/src/main.cpp 0.00% 23 Missing ⚠️
...linglong/src/linglong/builder/linglong_builder.cpp 0.00% 13 Missing ⚠️
Files with missing lines Coverage Δ
...s/linglong/src/linglong/builder/linglong_builder.h 100.00% <ø> (ø)
...linglong/src/linglong/builder/linglong_builder.cpp 9.72% <0.00%> (-0.11%) ⬇️
apps/ll-builder/src/main.cpp 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants