dmg pack for common and qt projects #6827

RubMaker · 2025-09-18T14:05:28Z

Before adding new features and new modules, please go to issues to submit the relevant feature description first.
Write good commit messages and use the same coding conventions as the rest of the project.
Please commit code to dev branch and we will merge into master branch in feature
Ensure your edited codes with four spaces instead of TAB.

增加新特性和新模块之前，请先到issues提交相关特性说明，经过讨论评估确认后，再进行相应的代码提交，避免做无用工作。
编写友好可读的提交信息，并使用与工程代码相同的代码规范，代码请用4个空格字符代替tab缩进。
请提交代码到dev分支，如果通过，我们会在特定时间合并到master分支上。
为了规范化提交日志的格式，commit消息，不要用中文，请用英文描述。

gemini-code-assist

Summary of Changes

Hello @RubMaker, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the packaging capabilities by introducing support for creating macOS DMG packages. It includes robust logic to identify Qt projects and automatically bundle their dependencies, ensuring that the generated DMGs are self-contained and ready for distribution. Additionally, it provides extensive configuration options for customizing the visual layout and properties of the DMG, making the packaging process more flexible and user-friendly.

Highlights

DMG Packaging Support: Introduced comprehensive support for creating macOS DMG (Disk Image) packages for applications.
Qt Project Integration: Added intelligent detection for Qt projects and automatic bundling of Qt dependencies using macdeployqt to ensure self-contained application bundles.
Customizable DMG Properties: Enabled extensive customization of DMG appearance and behavior, including window position and size, icon size, application and icon positions, and background images.
New xpack APIs: Expanded the xpack API with new functions to configure DMG-specific settings (set_dmg_window_pos, set_dmg_window_size, set_dmg_icon_size, set_dmg_applications_pos, set_dmg_icon_position) and AppImage settings (set_iconname, set_appimage_tool).
Packaging Type Updates: Updated xpack's internal mappings to correctly classify appimage and dmg as 'binary' input/output kinds and assign their respective file extensions (.AppImage, .dmg).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces support for creating DMG packages on macOS, including specific handling for Qt applications. The implementation is quite comprehensive, with logic to find necessary tools, detect Qt projects, and bundle dependencies using macdeployqt. My review focuses on improving the robustness of the Qt dependency handling and fixing a minor issue with variable scope. Overall, this is a great addition.

gemini-code-assist · 2025-09-18T14:06:57Z

xmake/plugins/pack/dmg/main.lua

+    if is_qt then
+        local macdeployqt = _get_macdeployqt()
+        if macdeployqt then
+            local qt_success = _deploy_qt_dependencies(package, app_source, macdeployqt)


The return value qt_success from _deploy_qt_dependencies is ignored. If deploying Qt dependencies fails, the packaging process continues, which will likely result in a non-functional application in the final DMG. The result of this operation should be checked to ensure the integrity of the package. You might consider failing the packaging process if this step is unsuccessful.

local qt_success = _deploy_qt_dependencies(package, app_source, macdeployqt) if not qt_success then print("Warning: Failed to deploy Qt dependencies, the application may not run correctly.") end

gemini-code-assist · 2025-09-18T14:06:58Z

xmake/plugins/pack/dmg/main.lua

+    if success then
+        local otool_output = os.iorunv("otool", {"-L", main_executable})
+        if otool_output then
+            local external_qt_refs = {}
+            for line in otool_output:gmatch("[^\r\n]+") do
+                -- Look for Qt framework references that are not in the bundle
+                local qt_ref = line:match("(%S*Qt%w+%.framework[^%s]*)")
+                if qt_ref and not qt_ref:find("@executable_path") and not qt_ref:find("@rpath") then
+                    table.insert(external_qt_refs, qt_ref)
+                end
+            end
+        end
+    end


This final verification step using otool appears to be incomplete. The external_qt_refs table is populated with any found external Qt framework references, but it's never used. This could lead to silently producing a broken package if macdeployqt fails to correctly bundle all dependencies. You should use this table to validate the bundling process, for instance by checking if it's non-empty and printing a warning.

Additionally, there are several unused local variables in this function (qt_version on line 210, all_frameworks on line 294, plugin_files on line 304) that could be removed for better code clarity.

gemini-code-assist · 2025-09-18T14:06:58Z

xmake/plugins/pack/dmg/main.lua

+
+    for _, bg_path in ipairs(bg_paths) do
+        if os.isfile(bg_path) then
+            ab_bg_path = path.absolute(bg_path)


The variable ab_bg_path is assigned without being declared with local. This makes it an implicit global variable, which can lead to subtle bugs and pollutes the global namespace. It should be declared as a local variable.

local ab_bg_path = path.absolute(bg_path)

xmake/includes/xpack/xmake.lua

xmake/plugins/pack/dmg/main.lua

waruqi · 2025-09-18T14:28:49Z

xmake/plugins/pack/dmg/main.lua

+    end
+
+    -- Method 2: Check executable for Qt dependencies using otool
+    local app_source, _ = _find_app_bundle(package)


, _ 删了

waruqi · 2025-09-18T14:30:47Z

xmake/plugins/pack/dmg/main.lua

+            local executables = os.files(path.join(macos_dir, "*"))
+            for _, executable in ipairs(executables) do
+                if os.isfile(executable) then
+                    local otool_output = os.iorunv("otool", {"-L", executable})


用 utils.binary.deplibs 模块就能获取到加载依赖。不要直接使用 otool

https://github.com/xmake-io/xmake/blob/dev/xmake/modules/utils/binary/deplibs.lua

waruqi · 2025-09-18T14:30:50Z

xmake/plugins/pack/dmg/main.lua

+    end
+
+    -- Method 3: Check source files for Qt headers/includes
+    local srcfiles, _ = package:sourcefiles()


waruqi · 2025-09-18T14:31:08Z

xmake/plugins/pack/dmg/main.lua

+        local executables = os.files(path.join(macos_dir, "*"))
+        for _, executable in ipairs(executables) do
+            if os.isfile(executable) then
+                local otool_output = os.iorunv("otool", {"-L", executable})


waruqi · 2025-09-18T14:31:12Z

xmake/plugins/pack/dmg/main.lua

+            local executables = os.files(path.join(macos_dir, "*"))
+            for _, executable in ipairs(executables) do
+                if os.isfile(executable) then
+                    local otool_output = os.iorunv("otool", {"-L", executable})


用 utils.binary.deplibs 模块就能获取到加载依赖。不要直接使用 otool

https://github.com/xmake-io/xmake/blob/dev/xmake/modules/utils/binary/deplibs.lua

waruqi · 2025-09-18T14:31:37Z

xmake/includes/xpack/xmake.lua

+        -- set appimage icon name
+        "xpack.set_iconname",
+        -- set appimage tool
+        "xpack.set_appimage_tool",


appimage 放到 appimage 的 pr 里面去，这里 dmg 不需要，删了

waruqi · 2025-09-18T14:31:44Z

xmake/plugins/pack/dmg/main.lua

+        local executables = os.files(path.join(macos_dir, "*"))
+        for _, executable in ipairs(executables) do
+            if os.isfile(executable) then
+                local otool_output = os.iorunv("otool", {"-L", executable})


waruqi · 2025-09-18T14:33:44Z

xmake/plugins/pack/dmg/main.lua

+    end
+
+    -- Method 3: Check for .qml files in project
+    local qml_files = os.files("**.qml")


为啥要执行这个，递归这么搞，层级多的时候，会相当慢，甚至卡住。

如果要检测目录有没有 qml 文件，用 lib.detect.find_file 接口，只要找到一个文件，就会返回，而不是遍历所有

waruqi · 2025-09-18T14:34:15Z

xmake/plugins/pack/dmg/main.lua

+    end
+
+    -- Method 3: Check source files for Qt headers/includes
+    local srcfiles, _ = package:sourcefiles()


waruqi · 2025-09-18T14:37:07Z

xmake/plugins/pack/dmg/main.lua

+end
+
+-- detect if this is a Qt project
+function _is_qt_project(package)


为什么要通过这种方式探测？这会非常慢，而且也不准。。

如果是 qt 工程，肯定会 apply qt 相关的 rules. 所以只需要检测 target:data("qt") 就行了，或者检测 target:rule 是否能获取到 qt env 的 rule

xmake/xmake/rules/qt/env/xmake.lua

Line 31 in 4481279

target:data_set("qt", qt)

waruqi · 2025-09-18T14:39:38Z

xmake/plugins/pack/dmg/main.lua

+end
+
+function _check_qml_usage(package, app_source)
+    -- Method 1: Check for QML-related libraries in links


这里也是，怪怪的，不应该是直接检测 qt qml 相关的 rule 是否存在不就好了么。

xmake/xmake/rules/qt/xmake.lua

Line 194 in 4481279

rule("qt.qmlplugin")

waruqi · 2025-09-18T14:40:20Z

xmake/plugins/pack/dmg/main.lua

+
+    -- create dmg
+    local success = _create_dmg_with_create_dmg(create_dmg, package, staging_dir, dmg_file, appbundle_name, bg_image)
+


空行去了

xmake/plugins/pack/dmg/main.lua

waruqi · 2025-09-18T14:41:39Z

xmake/plugins/pack/xpack.lua

            srpm     = "source",
-            rpm      = "source"
+            rpm      = "source",
+            appimage = "binary",


appimage 相关删了

waruqi · 2025-09-18T14:41:48Z

xmake/plugins/pack/xpack.lua

        srpm     = "source",
-        rpm      = "binary"
+        rpm      = "binary",
+        appimage = "binary",


waruqi · 2025-09-19T14:25:50Z

同样的，先把 qt 支持删，做最基础的 dmg 支持，然后先 merge 进来再说。

qt 的支持最后弄，参考 #6826 (comment)

xmake/includes/xpack/xmake.lua

waruqi · 2025-09-21T07:57:41Z

xmake/modules/detect/tools/find_macdeployqt.lua

+    opt = opt or {}
+
+    -- find program from system PATH
+    local program = find_program(opt.program or "macdeployqt")


这个 qt 的也先删了

waruqi · 2025-09-21T07:58:51Z

xmake/plugins/pack/dmg/main.lua

+            if os.isfile(info_plist) and os.isdir(macos_dir) then
+                return abs_location, appbundle_name
+            else
+                print("      Invalid .app structure")


所有的错误日志，都用 raise

waruqi · 2025-09-21T07:59:28Z

xmake/plugins/pack/dmg/main.lua

+    local staging_dir = path.join(os.tmpdir(), package:name() .. "_dmg_staging")
+    -- clean and create staging directory
+    if os.isdir(staging_dir) then
+        os.vrunv("rm", {"-rf", staging_dir})


删除目录，为啥还要执行命令，os.rm 不就可以删么

waruqi · 2025-09-21T07:59:45Z

xmake/plugins/pack/dmg/main.lua

+        local bg_dest = path.join(staging_dir, path.filename(bg_image))
+        os.vcp(bg_image, bg_dest)
+        if not os.isfile(bg_dest) then
+            print("Warning: Failed to copy background image")


waruqi · 2025-09-21T08:00:06Z

xmake/plugins/pack/dmg/main.lua

+    table.insert(args, staging_dir)
+    -- ensure output directory
+    os.vrunv("mkdir", {"-p", path.directory(dmg_file)})
+    os.vrunv("rm", {"-f", dmg_file})


为啥不用 os.mkdir 和 os.rm

waruqi · 2025-09-21T08:01:03Z

xmake/plugins/pack/dmg/main.lua

+        return true
+    else
+        if errors then
+            print("Error output:", errors)


os.iorunv 运行失败，原本就会抛异常，这里全删了

waruqi · 2025-09-21T08:01:28Z

xmake/plugins/pack/dmg/main.lua

+    local app_source, appbundle_name = _find_app_bundle(package)
+    if not app_source then
+        return false
+    end


适当换行下，不要全挤在一起

gemini-code-assist bot reviewed Sep 18, 2025

View reviewed changes

waruqi reviewed Sep 18, 2025

View reviewed changes

xmake/includes/xpack/xmake.lua Outdated Show resolved Hide resolved

waruqi reviewed Sep 18, 2025

View reviewed changes

xmake/plugins/pack/dmg/main.lua Outdated Show resolved Hide resolved

waruqi reviewed Sep 18, 2025

View reviewed changes

xmake/plugins/pack/dmg/main.lua Outdated Show resolved Hide resolved

waruqi reviewed Sep 18, 2025

View reviewed changes

xmake/plugins/pack/dmg/main.lua Show resolved Hide resolved

waruqi reviewed Sep 18, 2025

View reviewed changes

xmake/plugins/pack/xpack.lua Outdated

srpm = "source",

rpm = "source"

rpm = "source",

appimage = "binary",

Copy link

Member

waruqi Sep 18, 2025

Choose a reason for hiding this comment

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

appimage 相关删了

waruqi reviewed Sep 18, 2025

View reviewed changes

xmake/plugins/pack/xpack.lua Outdated

srpm = "source",

rpm = "binary"

rpm = "binary",

appimage = "binary",

Copy link

Member

waruqi Sep 18, 2025

Choose a reason for hiding this comment

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

这

waruqi reviewed Sep 21, 2025

View reviewed changes

xmake/includes/xpack/xmake.lua Outdated Show resolved Hide resolved

waruqi reviewed Sep 21, 2025

View reviewed changes

RubMaker force-pushed the feat-dmg branch from 9abd8f6 to b91add5 Compare September 29, 2025 15:17

RubMaker force-pushed the feat-dmg branch from b91add5 to 54298b9 Compare September 29, 2025 15:25

RubMaker added 9 commits September 30, 2025 12:28

dmg pack

5fe9194

test version for dmg

705c160

return qt qml detect

2ed6c5e

update some print

aa7c17a

add print

27839c7

returen dmg pack

be2221b

remove macdeploy qt find tool

36ca067

remove qt for dmg pack

089d22a

do some modify

f196909

RubMaker force-pushed the feat-dmg branch from 54298b9 to f196909 Compare September 30, 2025 04:29


		-- create dmg
		local success = _create_dmg_with_create_dmg(create_dmg, package, staging_dir, dmg_file, appbundle_name, bg_image)

Uh oh!

dmg pack for common and qt projects #6827

Are you sure you want to change the base?

dmg pack for common and qt projects #6827

Uh oh!

Conversation

RubMaker commented Sep 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waruqi commented Sep 19, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!