Skip to content

Conversation

@johnnynunez
Copy link
Contributor

@johnnynunez johnnynunez commented Feb 2, 2025

https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/
This pull request includes several updates to the CI workflows, build configurations, and dependencies. The primary focus is on enhancing compatibility, updating dependencies, and improving the build process across different operating systems.

CI Workflow and Build Configuration Updates:

@johnnynunez
Copy link
Contributor Author

cc @nmwsharp

@zjowowen
Copy link

This pull request is helpful. Please merge it or fixing the typo error in core.cpp or gl_engine_egl.cpp in the first time.

@johnnynunez johnnynunez force-pushed the master branch 2 times, most recently from d4dbf60 to d7ca265 Compare February 24, 2025 14:49
@johnnynunez
Copy link
Contributor Author

This pull request is helpful. Please merge it or fixing the typo error in core.cpp or gl_engine_egl.cpp in the first time.

when @nmwsharp fix main builds

@johnnynunez
Copy link
Contributor Author

This pull request is helpful. Please merge it or fixing the typo error in core.cpp or gl_engine_egl.cpp in the first time.

can you merge it? or I have to wait for @nmwsharp ?

@nmwsharp
Copy link
Owner

Thanks for submitting this! And for the pings :)

To confirm, this is to add ARM Linux builds to the wheel set, and other changes to make those builds & tests succeed?

I see a whole lot of changes to the cibuildwheel configs, which is a little scary to me so I will want to test/check carefully. There were already many rounds of testing & tweaking to find the current cibuildwheel config. That's not say the changes aren't appropriate, just warning you that these are the kind of changes I like to check very carefully.

@nmwsharp
Copy link
Owner

I've approved CI runs, and I'll look in to the main branch CI failures to sort out that distraction.

@johnnynunez
Copy link
Contributor Author

I've approved CI runs, and I'll look in to the main branch CI failures to sort out that distraction.

Cibuildwheel now support natively arm ci

@nmwsharp
Copy link
Owner

I've fixed the build issues on the main branch!

@johnnynunez
Copy link
Contributor Author

johnnynunez commented Mar 28, 2025

@nmwsharp I simplify the code. Now is building with your changes too.
Done
image

@nmwsharp
Copy link
Owner

Thanks! The diff seems to contain many changes, added packages, etc. It also drops support for Python 3.7 & 3.8. I realize these are end-of-life, but I'd still rather build for them if possible. Are all of these changes really necessary for ARM builds?

(Also, the diff is kinda hard to read because it looks like it adds a space to the end of every line or something?)

@johnnynunez johnnynunez reopened this Mar 28, 2025
@johnnynunez
Copy link
Contributor Author

Thanks! The diff seems to contain many changes, added packages, etc. It also drops support for Python 3.7 & 3.8. I realize these are end-of-life, but I'd still rather build for them if possible. Are all of these changes really necessary for ARM builds?

(Also, the diff is kinda hard to read because it looks like it adds a space to the end of every line or something?)

I revert this, and also I revet commits, I don't now why git diff before not worked. Now you can see the differences

@johnnynunez
Copy link
Contributor Author

@nmwsharp basically, I update tests with last stable python version 3.12, and I add last cibuildwheel and adapt packages that exists in both architectures x86 and arm in linux, finally, I added arm runners

@nmwsharp
Copy link
Owner

nmwsharp commented Mar 31, 2025

Thanks for the updates!

I'm pretty sure there are still packages added to the CI lists which are not necessary. For instance I see GLFW, but Polyscope itself already includes GLFW as a submodule. And I also see Eigen, but this repository already includes Eigen as a submodule. Also, why was it necessary to add gcc/LLVM to the install lists?

This may seem pedantic, if the builds are succeeding now, but having multiple versions of dependencies around in CI pipelines can be an annoying source of tricky bugs in the future.

@johnnynunez
Copy link
Contributor Author

Thanks for the updates!

I'm pretty sure there are still packages added to the CI lists which are not necessary. For instance I see GLFW, but Polyscope itself already includes GLFW as a submodule. And I also see Eigen, but this repository already includes Eigen as a submodule. Also, why was it necessary to add gcc/LLVM to the install lists?

This may seem pedantic, if the builds are succeeding now, but having multiple versions of dependencies around in CI pipelines can be an annoying source of tricky bugs in the future.

done

@nmwsharp nmwsharp merged commit 9370788 into nmwsharp:master Apr 1, 2025
@nmwsharp
Copy link
Owner

nmwsharp commented Apr 1, 2025

Just merged! We still haven't really tested the builds, but I for some reason have had trouble getting builds to trigger from PRs, so it will be easier to debug that from the main branch.

Thanks for your changes! As you may have noticed a pushed a few more small cleanup commits. As one additional change I removed the manually-set image names; I want to understand/verify problems with the default image before using overridden ones.

@nmwsharp
Copy link
Owner

nmwsharp commented Apr 2, 2025

Looks like the build succeeded!

The wheels are here: https://github.com/nmwsharp/polyscope-py/actions/runs/14208200092 (artifact links at the bottom). If you have access to a suitable ARM machine feel free to test and verify they work as expected. I'll deploy a new version to pypi soon.

@johnnynunez
Copy link
Contributor Author

Looks like the build succeeded!

The wheels are here: https://github.com/nmwsharp/polyscope-py/actions/runs/14208200092 (artifact links at the bottom). If you have access to a suitable ARM machine feel free to test and verify they work as expected. I'll deploy a new version to pypi soon.

Yes it works. I have running 3dgrut on jetson agx

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