-
Notifications
You must be signed in to change notification settings - Fork 106
fix: initialize GDK in accounts module and remove GTK init #985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideInitializes 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 GDKsequenceDiagram
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
Class diagram for updated accounts module initialization and session daemon mainclassDiagram
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
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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 thatloggeris 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., viasync.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.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
调用 gtk+ 的函数前,需要使用gtk_init进行初始化,参考文档:https://docs.gtk.org/gtk3/func.init.html 根据搜索和AI分析,部分 dde-daemon 的接口已经移动到其他项目比如 appearance, xsettings 等,目前 dde-daemon 及其依赖并没有直接使用gtk+的接口,部分使用gdk的地方,比如account应该使用gdk的初始化 |
|
一些待删除的调用 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 pr auto review我来对这两个文件的diff进行审查:
+ err := gdkpixbuf.InitGdk()
+ if err != nil {
+ logger.Warning("Failed to initialize GDK:", err)
+ }改进建议:
-//#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();}改进建议:
总体建议:
这些改动看起来是合理的,主要是为了优化依赖和初始化过程。不过需要确保这些改动不会影响系统的核心功能。 |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Log: Fixed potential image processing issues in user account management
Influence:
fix: 在账户模块中初始化GDK并移除GTK初始化
Log: 修复了用户账户管理中潜在的图像处理问题
Influence:
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:
Bug Fixes:
Enhancements: