Skip to content

Conversation

@pewpewnor
Copy link
Contributor

@pewpewnor pewpewnor commented Nov 12, 2025

A potential solution fix for #8605

Note: this version fix is only for imgui-sfml version 3.0.0, other versions might still be broken...

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @pewpewnor, I'm Gemini Code Assist1! 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 addresses a dependency issue by refining how the imgui library is declared as a dependency for imgui-sfml. It introduces version-specific constraints for imgui when imgui-sfml is at version v3.0, ensuring compatibility and preventing potential build failures or runtime issues that might arise from incompatible imgui versions.

Highlights

  • Conditional ImGui Dependency: The imgui dependency is now conditionally added based on the imgui-sfml package version, moving away from a blanket dependency.
  • Version-Specific ImGui Constraint: For imgui-sfml version v3.0, the imgui dependency is specifically constrained to versions 1.91.1 through 1.91.9 to ensure compatibility.
  • Dynamic Dependency Generation: The xmake.lua generation logic within the on_install hook has been updated to dynamically include the version-specific imgui requirement.
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.

Footnotes

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

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes the imgui dependency version for imgui-sfml v3.0 by conditionally adding version constraints. The changes are consistent across the on_load and on_install functions. My review includes a suggestion to refactor the code to improve maintainability by removing a duplicated version string and to make the logic more concise.

Comment on lines 39 to 42
local imgui_version = ""
if package:version():eq("v3.0") then
imgui_version = " >=1.91.1 <=1.91.9"
end
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This logic for determining the imgui version string is duplicated from the on_load function (lines 23-27). To improve maintainability, you could define the version constraint string as a variable in the package scope and reuse it in both places.

Additionally, this block can be written more concisely using a ternary-like expression in Lua.

        local imgui_version = package:version():eq("v3.0") and " >=1.91.1 <=1.91.9" or ""

Copy link
Contributor

Choose a reason for hiding this comment

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

This code can be removed, because we can use policy package.sync_requires_to_deps
xmake-io/xmake#5745 (comment)

Copy link
Contributor Author

@pewpewnor pewpewnor Nov 13, 2025

Choose a reason for hiding this comment

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

This code can be removed, because we can use policy package.sync_requires_to_deps xmake-io/xmake#5745 (comment)

Can you show me how? I tried setting set_policy("package.sync_requires_to_deps", true) but it doesn’t seem to work as add_requires("imgui") still resolves to the newest imgui version for some reason.

However, I found that refactoring it to:

local imgui_dep = package:dep("imgui")
local imgui_version = imgui_dep and " " .. imgui_dep:version_str() or ""

works and I added it to the commit.

Copy link
Member

Choose a reason for hiding this comment

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

tried setting set_policy("package.sync_requires_to_deps", true) but it doesn’t seem to work as add_requires("imgui") still resolves to the newest imgui version for some reason.

it should be fixed. xmake-io/xmake#7030

Copy link
Contributor Author

@pewpewnor pewpewnor Nov 18, 2025

Choose a reason for hiding this comment

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

tried setting set_policy("package.sync_requires_to_deps", true) but it doesn’t seem to work as add_requires("imgui") still resolves to the newest imgui version for some reason.

it should be fixed. xmake-io/xmake#7030

Using the latest dev version of xmake v3.0.4+dev.91e10df41, it could not resolve package:add("deps", "imgui >=1.91.1<=1.91.9") properly without the space in between the version constraints.
I also tried set_policy("package.sync_requires_to_deps", true) with spaces between constraints again and it still did not work, add_requires("imgui") still resolves to the latest version?

/usr/bin/git clone https://github.com/ocornut/imgui.git -b >=1.91.1<=1.91.9 --depth 1 --recursive --shallow-submodules -c core.fsmonitor=false source.tmp/imgui
Cloning into 'source.tmp/imgui'...
warning: Could not find remote branch >=1.91.1<=1.91.9 to clone.
fatal: Remote branch >=1.91.1<=1.91.9 not found in upstream origin
error: @programdir/core/sandbox/modules/os.lua:378: execv(/usr/bin/git clone https://github.com/ocornut/imgui.git -b >=1.91.1<=1.91.9 --depth 1 --recursive --shallow-submodules -c core.fsmonitor=false source.tmp/imgui) failed(128)
stack traceback:
    [C]: in function 'error'
    [@programdir/core/base/os.lua:1125]:
    [@programdir/core/sandbox/modules/os.lua:378]:
    [@programdir/core/sandbox/modules/os.lua:291]: in function 'vrunv'
    [@programdir/modules/devel/git/clone.lua:127]:
    [...modules/private/action/require/impl/actions/download.lua:114]: in function '_checkout'
    [...modules/private/action/require/impl/actions/download.lua:392]:

  => clone https://github.com/ocornut/imgui.git >=1.91.1<=1.91.9 .. failed

we can also download these packages manually:
  - https://github.com/ocornut/imgui.git
to the local search directories:
  - imgui
and we can run `xmake g --pkg_searchdirs=/xxx` to set the search directories.
error: @programdir/core/main.lua:274: @programdir/modules/async/runjobs.lua:394: ...modules/private/action/require/impl/actions/download.lua:456:
stack traceback:
    [C]: in function 'error'
    [@programdir/core/base/os.lua:1125]:
    [...modules/private/action/require/impl/actions/download.lua:456]: in function 'catch'
    [@programdir/core/sandbox/modules/try.lua:123]: in function 'try'
    [...modules/private/action/require/impl/actions/download.lua:367]:
    [...modules/private/action/require/impl/install_packages.lua:503]: in function 'job_func'
    [@programdir/modules/async/runjobs.lua:207]:

stack traceback:
        [C]: in function 'error'
        @programdir/core/base/os.lua:1125: in function 'os.raiselevel'
        (...tail calls...)
        @programdir/core/main.lua:274: in upvalue 'cotask'
        @programdir/core/base/scheduler.lua:514: in function <@programdir/core/base/scheduler.lua:507>
error: execv(/home/braum/.local/bin/xmake require -f -y --build -v -D --shallow --extra={configs={shared=false}} imgui-sfml) failed(255)

Copy link
Contributor Author

@pewpewnor pewpewnor Nov 19, 2025

Choose a reason for hiding this comment

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

Yes it does work for me, so does the user need to add:

add_requires("imgui >=1.91.1 <=1.91.9")
set_policy("package.sync_requires_to_deps", true)

themselves everytime they want to add_requires("imgui-sfml") or is this PR still needed so that the user doesn't have to do that?

I think the original code review issue here is that adding version constraints to the add_requires("imgui") for the second time during the on_install hook is said to be not needed since set_policy("package.sync_requires_to_deps", true) can be used instead.
But from what I can tell, we still need to add the version constraints again regardless since it is independent of the package:add("deps", "imgui >=1.91.1 <=1.91.9") in the code for defining the target build, therefore switching the logic with set_policy("package.sync_requires_to_deps", true) in the package definition code will still have no effect.

Copy link
Member

Choose a reason for hiding this comment

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

I think you can limit the version range in package add_deps, and xmake supports automatic version resolution to select the most suitable version. However, there's no need to pass the version to port/xmake.lua.

Copy link
Contributor Author

@pewpewnor pewpewnor Nov 19, 2025

Choose a reason for hiding this comment

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

If I try to remove the version constraints from the package's xmake.lua, it will still try to use the latest version of imgui when building the imgui-sfml target since that runs in a different isolation from what I can tell. It does not care about the previous add_deps version constraints at all, the same reason why in the original code (before any of my commits) checks the linux and shared again (twice) for requiring sfml.

Copy link
Member

Choose a reason for hiding this comment

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

It will pass all deps configs. https://github.com/xmake-io/xmake/blob/660450b6adb49c5a8b792a0c844a414d0a1b7379/xmake/modules/package/tools/xmake.lua#L437

But it seems to have stopped working.

you can try this patch. xmake-io/xmake#7041

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, it's working now.
I've updated the PR.

@pewpewnor pewpewnor changed the title imgui-sfml: fix imgui dependency version (#8605) imgui-sfml: fix imgui dependency version Nov 12, 2025
@waruqi waruqi merged commit 76ad497 into xmake-io:dev Nov 20, 2025
61 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