Skip to content

feat(apt-clean): skip immutable clean and merge cache sizes in print-json mode#446

Draft
electricface wants to merge 1 commit into
linuxdeepin:masterfrom
electricface:swt/skip-clean-when-print-json
Draft

feat(apt-clean): skip immutable clean and merge cache sizes in print-json mode#446
electricface wants to merge 1 commit into
linuxdeepin:masterfrom
electricface:swt/skip-clean-when-print-json

Conversation

@electricface

Copy link
Copy Markdown
Member

When running with --print-json, lastore-apt-clean no longer executes
deepin-immutable-ctl upgrade clean. Instead it queries cache sizes via
--print-cache-sizes and appends immutable debtree cache info to the
JSON output.

  • Add binImmutableCtl constant to replace hardcoded command name
  • Add queryImmutableCacheSizes() and supporting types
  • Merge immutable debtree cache into archivesInfos output
  • Guard JSON marshaling with options.printJSON check

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @electricface, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface

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

deepin pr auto review

★ 总体评分:65分

■ 【总体评价】

代码尝试增加不可变系统缓存查询逻辑并清理废弃代码,但存在导致无法编译的致命语法错误
逻辑正确但因移除必要导入包且遗留调试代码导致无法编译扣35分

■ 【详细分析】

  • 1.语法逻辑(存在致命错误)✕
    具体分析内容:在diff头部移除了"path"包的导入,但在main函数中(cfg := config.NewConfig(path.Join("/var/lib/lastore", "config.json")))仍然使用了path.Join,直接导致编译失败。此外,在if cfg.UseIncrementalUpdate()内部引入了调试遗留代码if true {,不仅导致原有的增量更新条件判断被完全绕过,而且该if true {块缺少对应的闭合大括号},引发严重的括号不匹配编译错误。
    潜在问题:编译彻底失败无法构建;增量更新条件判断失效,在非不可变系统上错误执行清理或查询逻辑
    建议:恢复"path"包的导入;删除if true {调试代码并补齐或修正对应的作用域大括号
  • 2.代码质量(存在严重问题)✕
    具体分析内容:代码中遗留了明显的调试代码if true {,属于典型的未清理提交物。虽然清理了大量历史注释掉的废弃代码(如testAgainDebInfoList相关逻辑),提升了可读性,但新引入的致命语法问题严重拉低了整体质量。新增的immutableCacheOutput结构体及queryImmutableCacheSizes函数注释清晰、定义规范,但被外围的语法错误掩盖。
    潜在问题:调试代码混入生产分支;代码提交前缺乏基本的编译验证
    建议:在提交前必须执行go build验证;建立代码提交前的静态检查或CI流水线拦截机制
  • 3.代码性能(无性能问题)✓
    具体分析内容:新增的缓存文件遍历逻辑中,使用了make([]*archiveInfo, 0, len(files))预先分配了切片容量,避免了动态扩容带来的内存分配开销。通过外部命令获取JSON并解析的方式在调用频次极低(清理任务)的场景下是合理的。
    建议:保持当前的内存预分配习惯
  • 4.代码安全(存在0个安全漏洞)✓
    漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
    代码中调用的exec.Command(binImmutableCtl, "upgrade", "clean", "--print-cache-sizes")使用了硬编码的常量binImmutableCtl,未拼接任何外部用户输入,不存在命令注入风险。JSON解析使用了标准库json.Unmarshal且未开启未知字段忽略(默认行为安全)。文件路径/persistent/ostree/debtree/cache为硬编码常量,无路径遍历风险。
  • 建议:保持对外部命令参数的硬编码处理方式,避免未来引入变量拼接

■ 【改进建议代码示例】

diff --git a/src/lastore-apt-clean/main.go b/src/lastore-apt-clean/main.go
index 4470209e4..corrected_hash 100644
--- a/src/lastore-apt-clean/main.go
+++ b/src/lastore-apt-clean/main.go
@@ -14,6 +14,7 @@ import (
 	"io"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"syscall"
 	"time"
@@ -79,18 +80,17 @@ func main() {
 	findBins()
 
 	// Handle immutable system cache if incremental update is enabled
-	var immutableCache *immutableCacheOutput
 	cfg := config.NewConfig(path.Join("/var/lib/lastore", "config.json"))
+	var immutableCache *immutableCacheOutput
 	if cfg.UseIncrementalUpdate() {
-	if true {
 		if options.printJSON {
 			var err error
 			immutableCache, err = queryImmutableCacheSizes()
 			if err != nil {
 				logger.Debugf("failed to query immutable cache sizes: %v", err)
 			}
 		} else {
 			err := exec.Command(binImmutableCtl, "upgrade", "clean").Run()
 			if err != nil {
 				logger.Debugf("failed to clean immutable cache: %v", err)
 			}
 		}
+	}
 	}
 

@deepin-ci-robot

Copy link
Copy Markdown

@electricface: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
github-pr-review-ci 3deb3c6 link true /test github-pr-review-ci

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@electricface electricface marked this pull request as draft July 2, 2026 13:48
…json mode

When running with --print-json, lastore-apt-clean no longer executes
deepin-immutable-ctl upgrade clean. Instead it queries cache sizes via
--print-cache-sizes and appends immutable debtree cache info to the
JSON output.

- Add binImmutableCtl constant to replace hardcoded command name
- Add queryImmutableCacheSizes() and supporting types
- Merge immutable debtree cache into archivesInfos output
- Guard JSON marshaling with options.printJSON check
@electricface electricface force-pushed the swt/skip-clean-when-print-json branch from 3deb3c6 to b9a9f3f Compare July 2, 2026 13:51
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