Skip to content

add requires api checker#7584

Open
Shiffted wants to merge 2 commits into
xmake-io:devfrom
Shiffted:requires_checker
Open

add requires api checker#7584
Shiffted wants to merge 2 commits into
xmake-io:devfrom
Shiffted:requires_checker

Conversation

@Shiffted

@Shiffted Shiffted commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

#7378

Adds checker for add_requires() like discussed in #7394

This does not check configs from package deps, e.g.:

package("foo")
    add_deps("libsdl2", {configs = {invalid_conf = true}})
    on_install(function (package) end)
package_end()

add_requires("foo")

will not be checked.

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

Copy link
Copy Markdown
Contributor

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 introduces new API checkers (api.requires.package and api.requires.configs) to validate package names and configurations in add_requires and add_requireconfs, along with corresponding tests and refactoring of package loading logic. The reviewer identified several issues, including a potential side-effect where table.join2 could mutate the project's internal state in-place, missing defensive checks for opt and requireinfo that could cause runtime errors, and an issue where requireconf is skipped entirely if its source information is missing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread xmake/modules/private/check/checkers/api/api_checker.lua Outdated
Comment thread xmake/modules/private/check/checkers/api/api_checker.lua Outdated
Comment thread xmake/modules/private/check/checkers/api/api_checker.lua Outdated
Comment thread xmake/modules/private/action/require/impl/package.lua
@Shiffted Shiffted force-pushed the requires_checker branch from f498485 to 6dfa02b Compare June 5, 2026 12:26
@Shiffted Shiffted marked this pull request as draft June 5, 2026 12:35
@Shiffted Shiffted force-pushed the requires_checker branch 2 times, most recently from 8321b20 to 0c203f6 Compare June 5, 2026 13:00
@Shiffted Shiffted marked this pull request as ready for review June 5, 2026 13:00
@waruqi

waruqi commented Jun 6, 2026

Copy link
Copy Markdown
Member

To minimize changes, I don't have time to review large patches.

Comment thread xmake/modules/private/check/checker.lua Outdated
@Shiffted Shiffted marked this pull request as draft June 7, 2026 17:29
@Shiffted Shiffted force-pushed the requires_checker branch from 0c203f6 to 3d24d0b Compare June 8, 2026 18:42
@Shiffted Shiffted force-pushed the requires_checker branch from 3d24d0b to de04689 Compare June 8, 2026 18:54
@Shiffted

Shiffted commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

I changed the implementation to minimize changes and added a checker only for add_requires(). I will add the add_requireconfs() checker in a separate PR.

This now also allows values checker to skip instances. The only other previously existing values checker always returns a non-nil table, so existing checkers are unaffected.


我修改了实现方案,以尽量减少改动,并且仅为 add_requires() 添加了检查器。我将在另一个 PR 中添加 add_requireconfs() 的检查器。

现在,values 检查器也可以跳过实例了。此前唯一存在的另一个 values 检查器始终返回一个非空表,因此现有的检查器不受影响。

@Shiffted Shiffted marked this pull request as ready for review June 8, 2026 18:54
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.

2 participants