Skip to content

Improve requirelock#7216

Merged
waruqi merged 9 commits intodevfrom
requirelock
Jan 14, 2026
Merged

Improve requirelock#7216
waruqi merged 9 commits intodevfrom
requirelock

Conversation

@waruqi
Copy link
Member

@waruqi waruqi commented Jan 13, 2026

  • 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消息,不要用中文,请用英文描述。

@waruqi waruqi added this to the v3.0.7 milestone Jan 13, 2026
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @waruqi, 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 significantly improves the robustness and reliability of Xmake's package locking mechanism. It introduces new, detailed tests to ensure the xmake-requires.lock file is correctly generated and remains stable across builds, preventing unnecessary updates to its modification timestamp. Additionally, it fixes a minor bug related to how repository information is handled during package instantiation.

Highlights

  • New Test Suite for requires_lock: A comprehensive test suite has been added to validate the requires_lock functionality, including a C source file utilizing zlib and a Lua test script that checks lock file generation, content, and stability across rebuilds.
  • Lock File Stability Improvement: The mechanism for writing the xmake-requires.lock file has been enhanced. It now writes to a temporary file first and only copies to the final destination if the content has changed, ensuring the lock file's modification time remains stable when its content is identical.
  • Corrected Repository Option Passing: An issue where the repo option was not correctly passed during package instance creation has been resolved, ensuring proper repository information is used.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 introduces new test cases for the requires_lock functionality, demonstrating its usage with zlib packages and verifying lock file generation and stability. A notable improvement is the implementation of a temporary file mechanism for writing lock files, which enhances robustness by ensuring the file's modification time (mtime) only changes when its content is actually different. However, a potential issue was identified in xmake/core/package/package.lua where the repository object might be incorrectly passed, which could affect how package repositories are handled.

const char* source = "Hello, zlib requires lock test!";
uLong sourceLen = strlen(source) + 1;
uLong destLen = compressBound(sourceLen);
Bytef* dest = (Bytef*)malloc(destLen);
Copy link
Contributor

Choose a reason for hiding this comment

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

high

It's good practice to check the return value of malloc to ensure memory allocation was successful. If malloc returns NULL, dereferencing dest will lead to a crash or undefined behavior.

    Bytef* dest = (Bytef*)malloc(destLen);
    if (!dest) {
        fprintf(stderr, "Memory allocation failed for dest\n");
        return 1;
    }


// decompression test
uLong decompLen = sourceLen;
Bytef* decomp = (Bytef*)malloc(decompLen);
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Similar to the previous malloc call, the return value of malloc for decomp should be checked. Failure to allocate memory here could also lead to a crash.

Suggested change
Bytef* decomp = (Bytef*)malloc(decompLen);
Bytef* decomp = (Bytef*)malloc(decompLen);
if (!decomp) {
fprintf(stderr, "Memory allocation failed for decomp\n");
free(dest);
return 1;
}


-- we should have three zlib entries (version constraint, specific version, shared config)
local zlib_count = count_zlib_entries(plat_entries)
assert(zlib_count == 3, "should find 3 zlib entries in lock file, found: " .. zlib_count)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The assertion assert(zlib_count == 3, ...) uses a magic number 3. While this might be correct for the current test setup, it makes the test less flexible and harder to understand if the number of zlib entries changes in the future. Consider adding a comment explaining why exactly 3 entries are expected, or if possible, derive this number dynamically from the xmake.lua configuration.

@qudix
Copy link
Contributor

qudix commented Jan 13, 2026

this seems to have fixed the empty lock files, I can't say for certain whether it also fixes randomly changing versions and hashes over time.

one thing i noticed is that despite the contents not changing, the date modified does get changed causing git to pick it up:
image

@waruqi
Copy link
Member Author

waruqi commented Jan 14, 2026

this seems to have fixed the empty lock files, I can't say for certain whether it also fixes randomly changing versions and hashes over time.

one thing i noticed is that despite the contents not changing, the date modified does get changed causing git to pick it up:

Did you try the latest commit? it should be fixed.

and test.lua will check it's mtime.

-        io.writefile(project.requireslock(), string.serialize(results, {orderkeys = true}), {encoding = "binary"})
+        -- write to temporary file first, then copy if content is different
+        local lockfile = project.requireslock()
+        local content = string.serialize(results, {orderkeys = true})
+        local tmpfile = os.tmpfile()
+
+        -- write to temporary file
+        io.writefile(tmpfile, content, {encoding = "binary"})
+
+        -- copy to target file only if content is different
+        os.cp(tmpfile, lockfile, {copy_if_different = true})
+        os.rm(tmpfile)

@qudix
Copy link
Contributor

qudix commented Jan 14, 2026

@waruqi
Copy link
Member Author

waruqi commented Jan 14, 2026

https://github.com/libxse/commonlibsse-ng-template

It tried it, it works me. nothing changed

PS C:\Users\wangrunqing\Downloads\commonlibsse-ng-template> xmake f -c
checking for platform ... windows
checking for architecture ... x64
checking for Microsoft C/C++ Compiler (x64) ... ok
checking for Microsoft Visual Studio (x64) version ... 2026
note: install or modify (m) these packages (pass -y to skip confirm)?
in 28f94f40b9abb168bb83cc593f522089.lock:
  -> rapidcsv v8.89 [license:BSD-3-Clause]
in 08e279de9f7b7160f77984c3b3f6b72d.lock:
  -> directxmath 2024.02 [license:MIT]
in 5dcc664bc51111af13d0e5c3c4d3258b.lock:
  -> directxtk 24.2.0 [runtimes:"MD"]
in 014e50bd8d0bd9f640616d2700496751.lock:
  -> spdlog v1.15.3 [runtimes:"MD", header_only:n, wchar:y, std_format:y, license:MIT]
please input: y (y/n/m)

  => install rapidcsv v8.89 .. ok
  => install directxtk 24.2.0 (precompiled) .. ok
  => install directxmath 2024.02 .. ok
  => install spdlog v1.15.3 .. ok
PS C:\Users\wangrunqing\Downloads\commonlibsse-ng-template> git diff
PS C:\Users\wangrunqing\Downloads\commonlibsse-ng-template>
PS C:\Users\wangrunqing\Downloads\commonlibsse-ng-template> xmake project -k vsxmake
checking for debug.x64 ...
checking for Microsoft C/C++ Compiler (x64) ... ok
checking for Microsoft Visual Studio (x64) version ... 2026
note: install or modify (m) these packages (pass -y to skip confirm)?
in 5dcc664bc51111af13d0e5c3c4d3258b.lock:
  -> directxtk 24.2.0 [runtimes:"MDd"]
in 014e50bd8d0bd9f640616d2700496751.lock:
  -> spdlog v1.15.3 [header_only:n, wchar:y, std_format:y, runtimes:"MDd", license:MIT]
please input: y (y/n/m)

  => install spdlog v1.15.3 .. ok
  => download https://github.com/microsoft/DirectXTK/archive/feb2024.zip .. ok
  => install directxtk 24.2.0 .. ok
checking for releasedbg.x64 ...
checking for Microsoft C/C++ Compiler (x64) ... ok
checking for Microsoft Visual Studio (x64) version ... 2026
create ok!
PS C:\Users\wangrunqing\Downloads\commonlibsse-ng-template> git diff

@waruqi
Copy link
Member Author

waruqi commented Jan 14, 2026

mtime is not changed too.

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d-----         2026/1/14     15:12                .xmake
d-----         2026/1/14     15:13                build
d-----         2026/1/14     14:46                lib
d-----         2026/1/14     14:46                src
-a----         2026/1/14     14:46            111 .gitignore
-a----         2026/1/14     14:46            126 .gitmodules
-a----         2026/1/14     14:46           2032 EXCEPTIONS
-a----         2026/1/14     14:46          35149 LICENSE
-a----         2026/1/14     14:46           1343 README.md
-a----         2026/1/14     14:46           1757 xmake-requires.lock
-a----         2026/1/14     14:46            893 xmake.lua

@waruqi waruqi merged commit 6a28220 into dev Jan 14, 2026
70 of 71 checks passed
@waruqi waruqi deleted the requirelock branch January 14, 2026 22:45
@qudix
Copy link
Contributor

qudix commented Jan 15, 2026

it stopped changing once I deleted the .xmake folder and tried again, so it seems fine now

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