Skip to content

Refactor Windows on ARM build script #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

matthew-brett
Copy link
Contributor

Use the standard repo structure for build.

Use the standard repo structure for build.
@matthew-brett matthew-brett marked this pull request as draft April 10, 2025 16:23
@Harishmcw
Copy link
Contributor

Hi Mathew,

Fixes in the Refactored Code:

  1. Fixing Directory Handling for Build Output: (ln No. 26)
pushd "%~dp0\.."
set "ob_out_root=%CD%\local\scipy_openblas"
set "ob_64=%ob_out_root%64"
set "ob_32=%ob_out_root%32"
if exist "%ob_64%" xcopy "%ob_64%" "%ob_32%" /I /Y
set "DEST_DIR=%ob_32%" 
  • Adding pushd "%~dp0.." makes the script more robust—it always sets the working directory to the root of the openblas-libs repo, no matter where the script is invoked from.
  • The original copy command was treating the folder as a file, leading to an error during execution. Switching to xcopy ensures proper copying of the folder and its contents.
  1. Ensuring a Clean Build Environment: (ln. No 46) 
if exist build (rmdir /S /Q build || exit /b 1)
mkdir build || exit /b 1 & cd build || exit /b 1 
  • Prevents stale or conflicting build artifacts from affecting the current build.
  • Helps in achieving a clean and reproducible build every time.
  1. Adding a check before running Python -m build: (ln. No 114)

python -c "import build" 2>NUL || pip install build

  1. Adding the .h extension to the missing header file: (ln. No 100)

if exist lapacke_mangling.h copy /Y lapacke_mangling.h "%DEST_DIR%\include

@matthew-brett
Copy link
Contributor Author

Thanks - could you check I have made those changes as you intended them? By the way, I think you can make pull request to my branch with the changes directly.

@Harishmcw
Copy link
Contributor

Hi Mathew,

I was going through the bat script and noticed the environment variables are currently being set using inline quotes (e.g., set var="value"), which embeds the quotes inside the variable value. So the actual variable value is like:
"C:\some path\local\scipy_openblas64". This can lead to double quotes in unexpected places later in the script.

A safer and more robust approach would be:

set "ob_out_root=%CD%\local\scipy_openblas"
set "ob_64=%ob_out_root%64"
set "ob_32=%ob_out_root%32"
if exist "%ob_64%" xcopy "%ob_64%" "%ob_32%" /I /Y
set "DEST_DIR=%ob_32%" 

This version quotes the values correctly and handles paths with spaces or special characters safely.

Thanks!

@matthew-brett
Copy link
Contributor Author

Current code correct?

@Harishmcw
Copy link
Contributor

Current code correct?

Yep, looks perfect now.👍

@matthew-brett
Copy link
Contributor Author

@Harishmcw - can you think of a way to avoid leaving behind a modified pyproject.toml file?

@Harishmcw
Copy link
Contributor

@Harishmcw - can you think of a way to avoid leaving behind a modified pyproject.toml file?

Hi @matthew-brett ,

To avoid leaving a modified pyproject.toml file, we can create a backup of it.

Add this line after ln No.68:

copy pyproject.toml pyproject.toml.bak

This creates a backup of the original pyproject.toml file as pyproject.toml.bak before any modifications are made. This ensures that we have a copy of the original file.

Add this line after ln No.117:

move /Y pyproject.toml.bak pyproject.toml

This line restores the original pyproject.toml file from the backup (pyproject.toml.bak). The /Y option automatically confirms the overwrite, ensuring that the original file is restored and no modified version is left behind.

Let me know if you need any further clarification or adjustments!

Thanks!

@matthew-brett
Copy link
Contributor Author

How about the current code?

@Harishmcw
Copy link
Contributor

How about the current code?

Yes, the current code works as expected. However, it does leave behind the pyproject_64_32.toml file after the process.

@matthew-brett
Copy link
Contributor Author

How about the current code?

Yes, the current code works as expected. However, it does leave behind the pyproject_64_32.toml file after the process.

I don't see that - I think the move commands take care of that deletion.

@Harishmcw
Copy link
Contributor

How about the current code?

Yes, the current code works as expected. However, it does leave behind the pyproject_64_32.toml file after the process.

I don't see that - I think the move commands take care of that deletion.

Ah, you're right — I missed that move command. Thanks for pointing that out, and sorry for the confusion!

@mattip
Copy link
Collaborator

mattip commented Apr 15, 2025

It seems github has opened the windows11-arm64 runners for public repos like this. Want to add a workflow?

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