Skip to content

chore: change pk policy from packagekit to appwiz's script#37

Merged
BLumia merged 2 commits into
linuxdeepin:masterfrom
BLumia:release-0.1.14
Apr 16, 2025
Merged

chore: change pk policy from packagekit to appwiz's script#37
BLumia merged 2 commits into
linuxdeepin:masterfrom
BLumia:release-0.1.14

Conversation

@BLumia

@BLumia BLumia commented Apr 15, 2025

Copy link
Copy Markdown
Member

调整卸载脚本的polkit规则,使得符合磐石环境的需求.

PMS: BUG-303279

Summary by Sourcery

Modify the application uninstaller script and polkit rules to support non-root users uninstalling packages using pkexec

Bug Fixes:

  • Improve package uninstallation mechanism to work correctly in Panshi (磐石) environment

Enhancements:

  • Add root user check to dynamically choose between direct apt and pkexec-based uninstallation

Chores:

  • Update uninstallation process to use pkexec for non-root users

BLumia added 2 commits April 15, 2025 19:52
调整卸载脚本的polkit规则,使得符合磐石环境的需求.

PMS: BUG-303279
Log:
Changelog:

  * chore: change polkit rule from packagekit to appwiz's script
    (PMS: BUG-303279)

Log:
@BLumia
BLumia requested review from robertkill and yixinshark April 15, 2025 12:03
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

代码审查意见:

  1. 安全性:

    • dde-appwiz-uninstaller.sh脚本中,直接使用pkexec命令可能会带来安全风险,建议使用sudo命令来代替,并确保脚本执行的用户具有相应的权限。
    • Launcher1Compat::uninstallPackageByScript函数中,使用pkexec来启动dde-appwiz-uninstaller.sh脚本,这可能会绕过一些安全限制,需要确保这样做是安全的。
  2. 代码质量:

    • dde-appwiz-uninstaller.sh脚本中,apt_command变量的值应根据用户是否为root来动态设置,但这里没有检查pkexec命令是否可用,可能会导致脚本在非root用户环境中失败。
    • dde-appwiz-uninstaller.sh脚本中的apt_command变量设置应该放在一个函数中,以便于维护和复用。
  3. 性能:

    • process.waitForFinished();调用可能会导致阻塞,如果dde-appwiz-uninstaller.sh脚本执行时间较长,可能会影响用户体验。可以考虑使用非阻塞的方式或者异步处理。
  4. 错误处理:

    • dde-appwiz-uninstaller.sh脚本中,没有对pkexec命令的执行结果进行错误处理,如果pkexec命令不可用或者执行失败,应该有相应的错误处理逻辑。
  5. 代码风格:

    • dde-appwiz-uninstaller.sh脚本中的注释应该使用中文,以保持代码的一致性和可读性。
  6. 日志记录:

    • Launcher1Compat::uninstallPackageByScript函数中,应该记录更多的日志信息,比如process.readAllStandardError()的输出,以便于调试和问题追踪。

综上所述,建议对上述问题进行修改和优化,以提高代码的安全性和稳定性,同时保持良好的代码质量和可维护性。

@github-actions

Copy link
Copy Markdown

TAG Bot

TAG: 0.1.14
EXISTED: no
DISTRIBUTION: unstable

@sourcery-ai

sourcery-ai Bot commented Apr 15, 2025

Copy link
Copy Markdown

Reviewer's Guide by Sourcery

This pull request modifies the uninstallation process to align with the requirements of the 'Panshi' environment. It changes the policy for the uninstallation script from PackageKit to a custom script using pkexec. The uninstallation script now checks if the user is root before using pkexec.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Modified the uninstallation script to use pkexec only when the user is not root.
  • Added a check to determine if the script is running as root.
  • Conditionally set the apt_command variable to either apt purge -y (if root) or pkexec apt purge -y (if not root).
  • Execute the uninstallation command using the apt_command variable.
scripts/dde-appwiz-uninstaller.sh
Modified the uninstallPackageByScript function to execute the uninstallation script using pkexec.
  • Changed the process.start call to use pkexec to execute the uninstallation script.
dbus/launcher1compat.cpp
Removed the polkit rule file.
  • Removed the org.deepin.dde.application-wizard.rules file.
polkit-1/rules.d/org.deepin.dde.application-wizard.rules

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!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

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

@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.

Hey @BLumia - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider using QProcess::execute instead of process.start and process.waitForFinished for simpler code.
  • It looks like you're calling pkexec twice in some cases - is that intentional?
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

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.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, yixinshark

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

@BLumia
BLumia merged commit 343ab76 into linuxdeepin:master Apr 16, 2025
@BLumia
BLumia deleted the release-0.1.14 branch April 16, 2025 02:32
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