Skip to content

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Dec 25, 2025

  1. Added GDK initialization in accounts module to ensure proper image handling functionality
  2. Removed GTK initialization from session daemon main function to reduce unnecessary dependencies
  3. Updated CGO pkg-config to remove gtk+-3.0 dependency from session daemon
  4. Added error handling for GDK initialization failure with warning log

Log: Fixed potential image processing issues in user account management

Influence:

  1. Test user account avatar loading and display
  2. Verify user account creation and management functionality
  3. Test session startup without GTK dependency
  4. Check for any image-related errors in system logs
  5. Verify backward compatibility with existing user accounts
  6. Test system performance impact after removing GTK initialization

fix: 在账户模块中初始化GDK并移除GTK初始化

  1. 在账户模块中添加GDK初始化以确保正确的图像处理功能
  2. 从会话守护进程主函数中移除GTK初始化以减少不必要的依赖
  3. 更新CGO pkg-config配置,从会话守护进程中移除gtk+-3.0依赖
  4. 添加GDK初始化失败的错误处理并记录警告日志

Log: 修复了用户账户管理中潜在的图像处理问题

Influence:

  1. 测试用户账户头像加载和显示功能
  2. 验证用户账户创建和管理功能
  3. 测试无GTK依赖的会话启动
  4. 检查系统日志中是否有图像相关的错误
  5. 验证与现有用户账户的向后兼容性
  6. 测试移除GTK初始化后的系统性能影响

PMS: BUG-316783

Summary by Sourcery

Initialize GDK in the accounts module and remove GTK initialization from the session daemon to simplify dependencies and improve image handling robustness.

New Features:

  • Initialize GDK in the accounts module during startup to support image handling for accounts.

Bug Fixes:

  • Prevent potential image processing issues in account management by ensuring GDK is initialized before use.

Enhancements:

  • Log a warning when GDK initialization fails instead of silently continuing.
  • Remove GTK initialization from the session daemon main entrypoint to avoid an unnecessary GUI dependency.

@sourcery-ai
Copy link

sourcery-ai bot commented Dec 25, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Initializes GDK in the accounts module for image handling while removing legacy GTK initialization and dependency from the session daemon, adding warning-level error handling around GDK setup.

Sequence diagram for updated accounts module initialization with GDK

sequenceDiagram
    participant Runtime as Go_runtime
    participant Accounts as accounts1_package
    participant GdkPixbuf as gdkpixbuf
    participant Logger as logger
    participant Loader as loader

    Runtime->>Accounts: load_package
    activate Accounts
    Accounts->>GdkPixbuf: InitGdk()
    alt GDK initialization fails
        GdkPixbuf-->>Accounts: error
        Accounts->>Logger: Warning(Failed to initialize GDK, err)
    else GDK initialization succeeds
        GdkPixbuf-->>Accounts: nil
    end
    Accounts->>Loader: Register(NewDaemon())
    deactivate Accounts
Loading

Class diagram for updated accounts module initialization and session daemon main

classDiagram
    class accounts1_package {
        <<package>>
        +init()
        +getAccountsManager() *Manager
    }

    class gdkpixbuf {
        <<library>>
        +InitGdk() error
    }

    class logger {
        <<package>>
        +Warning(args ...interfaceAny)
    }

    class loader {
        <<package>>
        +Register(daemon Daemon)
    }

    class Daemon {
        <<interface>>
    }

    class Manager {
    }

    class session_daemon_main {
        <<binary>>
        +main()
    }

    accounts1_package --> gdkpixbuf : uses
    accounts1_package --> logger : logs_with
    accounts1_package --> loader : registers_Daemon
    loader --> Daemon : depends_on
    accounts1_package --> Manager : returns

    session_daemon_main --> accounts1_package : uses

    class C {
        <<c_library_stub>>
        -init()  %% removed_from_main
    }

    session_daemon_main ..> C : init_removed
Loading

File-Level Changes

Change Details Files
Initialize GDK in the accounts/accounts manager startup path and add warning-level error handling for GDK init failures.
  • Call gdkpixbuf.InitGdk() in the accounts module init() function before registering the daemon.
  • Log a warning message if GDK initialization fails instead of aborting startup.
  • Keep the existing loader.Register(NewDaemon()) call so that account management startup behavior is otherwise unchanged.
accounts1/accounts.go
Remove GTK C-init usage and gtk+-3.0 dependency from the session daemon startup path.
  • Remove the C.init() GTK initialization call from the dde-session-daemon main() function so the daemon no longer depends on GTK at runtime.
  • Rely on the updated CGO pkg-config configuration (outside this diff) to drop gtk+-3.0 from the session daemon build-time dependencies while keeping other behavior intact.
  • Preserve the rest of the session daemon startup sequence (proxy.SetupProxy() and NewSessionDaemon(logger)) unchanged.
bin/dde-session-daemon/main.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Consider whether gdkpixbuf.InitGdk() failing should cause a hard failure instead of just a warning, or at least propagate the error up so callers can decide how critical missing image support is for their flow.
  • Because init() runs at import time, please double‑check that logger is fully initialized before this package is imported in all binaries, or guard the logging with a nil check to avoid potential panics during early startup.
  • If InitGdk() is not idempotent, it might be safer to ensure it is only called once (e.g., via sync.Once) in case this package is imported by multiple binaries or reinitialized in tests.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider whether `gdkpixbuf.InitGdk()` failing should cause a hard failure instead of just a warning, or at least propagate the error up so callers can decide how critical missing image support is for their flow.
- Because `init()` runs at import time, please double‑check that `logger` is fully initialized before this package is imported in all binaries, or guard the logging with a nil check to avoid potential panics during early startup.
- If `InitGdk()` is not idempotent, it might be safer to ensure it is only called once (e.g., via `sync.Once`) in case this package is imported by multiple binaries or reinitialized in tests.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@mhduiy
Copy link
Contributor Author

mhduiy commented Dec 25, 2025

调用 gtk+ 的函数前,需要使用gtk_init进行初始化,参考文档:https://docs.gtk.org/gtk3/func.init.html

根据搜索和AI分析,部分 dde-daemon 的接口已经移动到其他项目比如 appearance, xsettings 等,目前 dde-daemon 及其依赖并没有直接使用gtk+的接口,部分使用gdk的地方,比如account应该使用gdk的初始化

@mhduiy
Copy link
Contributor Author

mhduiy commented Dec 25, 2025

一些待删除的调用 gtk+ 接口的代码

1. Added GDK initialization in accounts module to ensure proper image
handling functionality
2. Removed GTK initialization from session daemon main function to
reduce unnecessary dependencies
3. Updated CGO pkg-config to remove gtk+-3.0 dependency from session
daemon
4. Added error handling for GDK initialization failure with warning log

Log: Fixed potential image processing issues in user account management

Influence:
1. Test user account avatar loading and display
2. Verify user account creation and management functionality
3. Test session startup without GTK dependency
4. Check for any image-related errors in system logs
5. Verify backward compatibility with existing user accounts
6. Test system performance impact after removing GTK initialization

fix: 在账户模块中初始化GDK并移除GTK初始化

1. 在账户模块中添加GDK初始化以确保正确的图像处理功能
2. 从会话守护进程主函数中移除GTK初始化以减少不必要的依赖
3. 更新CGO pkg-config配置,从会话守护进程中移除gtk+-3.0依赖
4. 添加GDK初始化失败的错误处理并记录警告日志

Log: 修复了用户账户管理中潜在的图像处理问题

Influence:
1. 测试用户账户头像加载和显示功能
2. 验证用户账户创建和管理功能
3. 测试无GTK依赖的会话启动
4. 检查系统日志中是否有图像相关的错误
5. 验证与现有用户账户的向后兼容性
6. 测试移除GTK初始化后的系统性能影响

PMS: BUG-316783
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这两个文件的diff进行审查:

  1. accounts/accounts.go 的变更:
+	err := gdkpixbuf.InitGdk()
+	if err != nil {
+		logger.Warning("Failed to initialize GDK:", err)
+	}

改进建议:

  1. 代码质量:这个修改看起来是为了初始化 GDK,但错误处理可能需要更严格。建议:

    • 如果 GDK 初始化失败,是否应该阻止程序继续运行?如果这是一个关键依赖,应该用 logger.Fatal 而不是 Warning
    • 可以添加更详细的错误信息,比如初始化失败的具体原因
  2. 安全性:这里没有明显的安全问题,但建议:

    • 确保只有必要的权限被授予
    • 验证初始化的来源是否可信
  3. bin/dde-session-daemon/main.go 的变更:

-//#cgo pkg-config: x11 gtk+-3.0
+//#cgo pkg-config: x11
-//#include <gtk/gtk.h>
-//void init(){XInitThreads();gtk_init(0,0);}
+//void init(){XInitThreads();}

改进建议:

  1. 代码质量:

    • 这个修改移除了 GTK 的依赖,只保留了 X11。这个改动是合理的,如果 GTK 功能确实不需要的话
    • 建议添加注释说明为什么移除 GTK 依赖,以及这个改动的影响范围
  2. 性能:

    • 移除 GTK 依赖可以减少程序的启动时间和内存占用,这是一个好的改进
  3. 安全性:

    • 减少依赖项可以减少潜在的攻击面,这是一个好的安全实践

总体建议:

  1. 在 accounts/accounts.go 中,建议评估 GDK 初始化失败的严重性,可能需要更严格的错误处理
  2. 在 main.go 中,建议添加注释说明移除 GTK 依赖的原因
  3. 建议添加单元测试来验证这些改动不会影响现有功能
  4. 考虑添加文档说明这些改动对系统行为的影响

这些改动看起来是合理的,主要是为了优化依赖和初始化过程。不过需要确保这些改动不会影响系统的核心功能。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

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

@mhduiy mhduiy merged commit 28abc12 into linuxdeepin:master Dec 25, 2025
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