openblas: refactor with cmake #8445

choyy · 2025-10-21T12:16:26Z

No description provided.

gemini-code-assist · 2025-10-21T12:16:41Z

Summary of Changes

Hello @choyy, 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 refactors the openblas package definition within xmake.lua by migrating its build system to CMake. This change simplifies the build and installation process across various platforms, replacing complex, platform-specific Makefile logic with a unified CMake-driven approach. The update also standardizes source URL management and introduces a richer set of configurable build options for OpenBLAS.

Highlights

Build System Migration: The OpenBLAS build process has been migrated from a custom Makefile-based system to CMake, simplifying cross-platform compilation and configuration.
Unified Source Management: OpenBLAS source URLs and version definitions have been consolidated, removing platform-specific download logic and adding support for version v0.3.30.
Enhanced Configuration Options: A comprehensive set of add_configs has been introduced for OpenBLAS, allowing granular control over features like LAPACK, CBLAS, dynamic architecture support, threading, and OpenMP during the build.

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 is an excellent refactoring of the OpenBLAS package, moving from a complex Makefile-based build to a unified CMake-based approach. This greatly simplifies the package script, improves maintainability, and expands build options for users. My review includes a couple of suggestions to enhance the robustness of the new configuration options by programmatically handling their inter-dependencies. This will help prevent invalid build configurations and improve the overall user experience.

packages/o/openblas/xmake.lua

waruqi · 2025-10-22T09:25:53Z

remove old versions, it will break some user projects.

choyy · 2025-10-22T14:44:30Z

remove old versions, it will break some user projects.

Successful at all versions:

Android (Windows) / build (windows-latest, arm64-v8a, r22, 30) (pull_request)
Android / build (ubuntu-latest, arm64-v8a, r22, 30) (pull_request)
Windows / build (windows-2025, shared, x64, MD) (pull_request)
Windows / build (windows-2025, shared, x64, MT) (pull_request)
Windows / build (windows-2025, shared, x86, MD) (pull_request)
Windows / build (windows-2025, shared, x86, MT) (pull_request)
Windows / build (windows-2025, static, x64, MD) (pull_request)
Windows / build (windows-2025, static, x64, MT) (pull_request)
Windows / build (windows-2025, static, x86, MD) (pull_request)
Windows / build (windows-2025, static, x86, MT) (pull_request)

others failed at certain version. Though I suggest keeping it like this for now.

waruqi · 2025-10-23T01:33:11Z

packages/o/openblas/xmake.lua


-    if is_plat("windows") then
-        if is_arch("x64", "x86_64") then
-            add_urls("https://github.com/OpenMathLib/OpenBLAS/releases/download/v$(version)/OpenBLAS-$(version)-x64.zip")


If you keep the previous version and just add the new version, it will not trigger all version checks. In addition, why remove the win binary package?

Keep the binary and source-build? How to choose when add package?

use binary for win

waruqi · 2025-10-23T01:34:14Z

packages/o/openblas/xmake.lua

-            end
-        end
-        make.build(package, configs)
-        make.make(package, table.join("install", "PREFIX=" .. package:installdir():gsub("\\", "/"), configs))


Lower versions should not support cmake, so we need to choose the build tool based on the package version.

waruqi · 2025-10-23T01:35:03Z

remove old versions, it will break some user projects.

Successful at all versions:

Android (Windows) / build (windows-latest, arm64-v8a, r22, 30) (pull_request)
Android / build (ubuntu-latest, arm64-v8a, r22, 30) (pull_request)
Windows / build (windows-2025, shared, x64, MD) (pull_request)
Windows / build (windows-2025, shared, x64, MT) (pull_request)
Windows / build (windows-2025, shared, x86, MD) (pull_request)
Windows / build (windows-2025, shared, x86, MT) (pull_request)
Windows / build (windows-2025, static, x64, MD) (pull_request)
Windows / build (windows-2025, static, x64, MT) (pull_request)
Windows / build (windows-2025, static, x86, MD) (pull_request)
Windows / build (windows-2025, static, x86, MT) (pull_request)

others failed at certain version. Though I suggest keeping it like this for now.

Only when all ci passes can it be merged

star-hengxing · 2025-10-23T08:25:27Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the OpenBLAS package to use CMake for source builds, particularly for versions 0.3.30 and newer. It introduces a comprehensive set of new configuration options to control the CMake build and adds several pre-build checks to improve the user experience. The changes are generally good, but I've identified a critical issue in an on_install selector that will break all source builds. I've also noted a couple of medium-severity issues regarding future maintenance and a potentially breaking change in default configurations. Please address the critical issue to ensure the package remains buildable.

gemini-code-assist · 2025-10-23T08:28:11Z

packages/o/openblas/xmake.lua

-            local fortran = find_tool("gfortran")
-            if fortran then
-                table.insert(configs, "FC=" .. fortran.program)
+    on_install("windows|!x64 and windows|!x86", function (package)


The selector for this on_install block, "windows|!x64 and windows|!x86", is invalid and will prevent source builds on any platform. It appears the intention is to handle all source builds not covered by the pre-built Windows binary installation.

To fix this, you should use a generic on_install block. This will act as a fallback for any platform/architecture combination that doesn't match the more specific on_install("windows|x64", "windows|x86", ...) block, which is the correct pattern for this scenario.

on_install(function (package)

gemini-code-assist · 2025-10-23T08:28:11Z

packages/o/openblas/xmake.lua

+        add_configs("openmp",            {description = "Compile with OpenMP enabled.",                                                                                                     default = false,  type = "boolean"})
+        add_configs("fortran",           {description = "Compile with fortran enabled.",                                                                                                    default = false,  type = "boolean"})


The default values for the openmp and fortran configurations have been changed from being enabled by default on most platforms (not is_plat("macosx")) to being disabled by default (false). This is a potentially breaking change for users who were relying on the previous behavior for versions older than 0.3.30. While this might be intentional to align with OpenBLAS's own default configuration, it's a significant change that might surprise users.

gemini-code-assist · 2025-10-23T08:28:11Z

packages/o/openblas/xmake.lua

        os.cp(path.join("bin", "libopenblas.dll"), package:installdir("bin"))
        os.cp("include", package:installdir())
-        if package:version():ge("0.3.28") then
+        if package:version():eq("0.3.28") then


Using package:version():eq("0.3.28") is less future-proof than ge("0.3.28"). If a future version of the pre-built OpenBLAS for Windows (e.g., 0.3.31) follows the same file layout as 0.3.28 (with libopenblas.lib at the root of the archive), this condition will fail, and the installation will break. Using ge would be more robust, assuming this new layout is the standard going forward for pre-built binaries.

if package:version():ge("0.3.28") then

choyy added 5 commits October 20, 2025 17:43

openblas: update to 0.3.30

f2f6bcc

fix macosx

5f278b2

update

46ff760

update

d183252

build with cmake

bc27e19

gemini-code-assist bot reviewed Oct 21, 2025

View reviewed changes

packages/o/openblas/xmake.lua Outdated Show resolved Hide resolved

choyy added 7 commits October 21, 2025 20:50

fix

edf0496

fix

bc1d611

fix

2063564

fix

d133f76

fix

a50327c

fix

19f4cd6

fix

45a9c23

choyy force-pushed the blas branch from b4e9796 to 45a9c23 Compare October 22, 2025 07:06

add old versions

1172c0c

waruqi reviewed Oct 23, 2025

View reviewed changes

choyy added 2 commits October 23, 2025 14:03

update

b30a497

update

146504c

gemini-code-assist bot reviewed Oct 23, 2025

View reviewed changes

fix

1fd60ea

waruqi merged commit 7736f21 into xmake-io:dev Oct 24, 2025
61 checks passed

choyy deleted the blas branch October 25, 2025 11:43

		add_configs("openmp", {description = "Compile with OpenMP enabled.", default = false, type = "boolean"})
		add_configs("fortran", {description = "Compile with fortran enabled.", default = false, type = "boolean"})

Uh oh!

Uh oh!

openblas: refactor with cmake #8445

openblas: refactor with cmake #8445

Uh oh!

Conversation

choyy commented Oct 21, 2025

Uh oh!

gemini-code-assist bot commented Oct 21, 2025

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!

Uh oh!

waruqi commented Oct 22, 2025

Uh oh!

choyy commented Oct 22, 2025

Uh oh!

waruqi Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

choyy Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

waruqi Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

waruqi Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

waruqi commented Oct 23, 2025

Uh oh!

star-hengxing commented Oct 23, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants