Skip to content

fix: initialize lastore config to clean incremental update cache#324

Merged
zhaohuiw42 merged 1 commit intolinuxdeepin:masterfrom
zhaohuiw42:master
Mar 7, 2026
Merged

fix: initialize lastore config to clean incremental update cache#324
zhaohuiw42 merged 1 commit intolinuxdeepin:masterfrom
zhaohuiw42:master

Conversation

@zhaohuiw42
Copy link
Copy Markdown
Contributor

lastore-apt-clean did not initialize lastore config before running, so incremental update settings were not applied correctly and incremental update cache could not be cleaned.

Task: https://pms.uniontech.com/task-view-386845.html

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 7, 2026

CLA Assistant Lite bot:
提交邮箱中包含我们的合作伙伴,但您似乎并非合作伙伴的成员或对接人,请联系相关对接人将您添加至组织之中,或由其重新发起 Pull Request。
The commit email domain belongs to one of our partners, but it seems you are not yet a member of the current organization, please contact the contact person to add you to the organization or let them submit the Pull Request.

You can retrigger this bot by commenting recheck in this Pull Request

qiuzhiqian
qiuzhiqian previously approved these changes Mar 7, 2026
lastore-apt-clean did not initialize lastore config before running,
so incremental update settings were not applied correctly and
incremental update cache could not be cleaned.

Task: https://pms.uniontech.com/task-view-386845.html
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

代码审查报告

总体评价

这段代码主要进行了以下改动:

  1. config.go 中添加了 DisableConsoleLogging() 函数
  2. lastore-apt-clean/main.go 中移除了命令行参数 incremental-update,改为从配置文件读取
  3. lastore-daemon/common.go 中移除了传递 --incremental-update 参数的逻辑

整体来看,代码逻辑清晰,但存在一些可以改进的地方。

详细审查意见

1. 语法逻辑问题

config.go

  • 无明显语法问题

lastore-apt-clean/main.go

  • 导入了 path 包但只使用了一次 path.Join,建议统一使用 filepath.Join 以保持一致性
  • main() 函数中直接硬编码了配置文件路径 "/var/lib/lastore/config.json",建议提取为常量

lastore-daemon/common.go

  • 移除了 . 导入方式(import "github.com/linuxdeepin/lastore-daemon/src/internal/config"),这是好的改进

2. 代码质量建议

config.go

// DisableConsoleLogging keeps config logs away from stdout for commands that
// need machine-readable output.
func DisableConsoleLogging() {
    logger.RemoveBackendConsole()
}

建议添加注释说明这个函数是线程安全的,或者确保其实现是线程安全的。

lastore-apt-clean/main.go

cfg := config.NewConfig(path.Join("/var/lib/lastore", "config.json"))
if cfg.IncrementalUpdate {
    err := exec.Command("deepin-immutable-ctl", "upgrade", "clean").Run()
    if err != nil {
        logger.Debugf("failed to clean upgrade cache: %v", err)
    }
}

建议:

  1. 将配置文件路径提取为常量
  2. 添加错误处理,当配置文件读取失败时应该有适当的处理
  3. 考虑添加日志记录,记录是否启用了增量更新

改进建议:

const (
    configDir = "/var/lib/lastore"
    configFile = "config.json"
)

// 在main函数中
configPath := filepath.Join(configDir, configFile)
cfg, err := config.NewConfig(configPath)
if err != nil {
    logger.Warningf("Failed to load config from %s: %v", configPath, err)
    // 根据业务逻辑决定是否继续执行
}

lastore-daemon/common.go

func getArchiveInfo() (string, error) {
    out, err := exec.Command("/usr/bin/lastore-apt-clean", "-print-json").Output()
    if err != nil {
        return "", err
    }
    return string(out), nil
}

建议:

  1. 将命令路径提取为常量
  2. 考虑添加超时控制
  3. 考虑添加更详细的错误日志

改进建议:

const (
    lastoreAptCleanCmd = "/usr/bin/lastore-apt-clean"
)

func getArchiveInfo() (string, error) {
    cmd := exec.Command(lastoreAptCleanCmd, "-print-json")
    // 设置超时
    ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
    defer cancel()
    
    out, err := cmd.CombinedOutput()
    if err != nil {
        logger.Warningf("Failed to get archive info: %v, output: %s", err, string(out))
        return "", err
    }
    return string(out), nil
}

3. 代码性能建议

  1. 配置文件读取:在 lastore-apt-clean/main.go 中每次运行都会读取配置文件,如果这个程序会被频繁调用,可以考虑缓存配置或者使用配置热加载机制。

  2. 命令执行:在 lastore-daemon/common.go 中,getArchiveInfo()getNeedCleanCacheSize() 都会执行外部命令,如果这些函数被频繁调用,可能会影响性能。建议:

    • 考虑缓存结果
    • 或者添加缓存失效机制
    • 或者使用更高效的 IPC 方式替代命令行调用

4. 代码安全建议

  1. 路径注入风险:在 lastore-apt-clean/main.go 中使用了 path.Join 构建路径,虽然当前是硬编码路径,但如果将来改为从用户输入获取,需要注意路径注入风险。建议始终使用 filepath.Join 并验证路径。

  2. 命令注入风险:在执行外部命令时,确保参数不会被注入。当前代码使用的是 exec.Command 的多参数形式,这是安全的做法,建议保持这种风格。

  3. 权限检查:在执行 deepin-immutable-ctl 命令前,建议检查当前用户是否有足够的权限执行该操作。

  4. 配置文件安全:确保配置文件 /var/lib/lastore/config.json 的权限设置正确,防止非授权用户修改。

总结

这段代码整体质量良好,主要改进点在于:

  1. 提取硬编码的路径为常量
  2. 添加更完善的错误处理和日志记录
  3. 考虑性能优化,如缓存配置和命令执行结果
  4. 加强安全性检查

建议在合并前进行充分的测试,特别是与增量更新相关的功能。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qiuzhiqian, zhaohuiw42

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

@zhaohuiw42 zhaohuiw42 merged commit 4384657 into linuxdeepin:master Mar 7, 2026
13 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