Skip to content

pip build#2145

Merged
casperdcl merged 14 commits into
masterfrom
skbuild
Jun 10, 2025
Merged

pip build#2145
casperdcl merged 14 commits into
masterfrom
skbuild

Conversation

@casperdcl

@casperdcl casperdcl commented Apr 23, 2025

Copy link
Copy Markdown
Member

testing

  1. cleanup any old installation (pip uninstall cil ; rm $CONDA_PREFIX/lib/*cilacc.*)
  2. follow the updated README

open follow-up issues

@casperdcl casperdcl self-assigned this Apr 23, 2025
Comment thread src/Core/CMakeLists.txt
Comment thread pyproject.toml
@casperdcl casperdcl force-pushed the skbuild branch 2 times, most recently from 2907599 to 71bb164 Compare April 23, 2025 13:33

@purepani purepani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There are sort of 3 separate tasks that need to be done in some order:

  • switch to nanobind or pybind11 or similar
  • simplify IPP library discovery
  • switch to scikit-build

I was attacking it from the perspective that it would be easier to do 1 and 2 before 3(and I probably unnecessarily mixed up CI changes into there). This PR attacks it by doing 2 and 3 together.
How hard would it be to just simplify IPP library discovery in 1 PR? That seems like the simplest unit to change here, at least to me.

Comment thread .github/workflows/build.yml
Comment thread pyproject.toml
@purepani

purepani commented Apr 23, 2025

Copy link
Copy Markdown
Collaborator

Oh I just realized - are you not doing the build with scikit-build-core yet?
The reason I changed the name for the folder was because I believe scikit-build-core needs the last component of the file name to be the same as the library(otherwise the build doesn't work).

Perhaps making that tiny change in a separate PR in anticipation is a good idea to minimize conflict issues.

Edit: OH I see what's happening; the python package is being built by cmake itself, whereas I just had the c code being built by cmake, which is needed since you're not building a c-extension but a normal shared library

@purepani

Copy link
Copy Markdown
Collaborator

After attempting it myself, it might just be easier to do all 3 at once(at least for me). I don't personally have the cmake knowledge to do the steps separately, and using scikit-build-core makes it way simpler.

@casperdcl

Copy link
Copy Markdown
Member Author

Thanks @purepani.

simplify IPP library discovery in 1 PR

Good point; it's now in #2148

And yes, separate PRs make it easier for review/debugging/approvals.

@github-project-automation github-project-automation Bot moved this to Todo in CIL work Apr 24, 2025
@casperdcl casperdcl moved this from Todo to In Progress in CIL work Apr 24, 2025
@purepani purepani mentioned this pull request Apr 24, 2025
@casperdcl casperdcl force-pushed the skbuild branch 2 times, most recently from 6c8ece0 to 6f78f05 Compare April 30, 2025 15:10
@casperdcl casperdcl requested review from gfardell and paskino April 30, 2025 15:18
@casperdcl casperdcl moved this from In Progress to Priority review in CIL work Apr 30, 2025
@casperdcl casperdcl marked this pull request as ready for review April 30, 2025 15:23
@casperdcl casperdcl requested a review from a team as a code owner April 30, 2025 15:23
Comment thread src/Core/CMakeLists.txt
@paskino

paskino commented May 8, 2025

Copy link
Copy Markdown
Contributor

currently trying to test conda build but failing, potentially because my conda is old-ish?

@casperdcl casperdcl mentioned this pull request May 9, 2025
9 tasks
Comment thread recipe/bld.bat Outdated

@gfardell gfardell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All seems happy on windows

@casperdcl casperdcl merged commit 8c889a1 into master Jun 10, 2025
11 checks passed
@casperdcl casperdcl deleted the skbuild branch June 10, 2025 11:00
@github-project-automation github-project-automation Bot moved this from Priority review to Done in CIL work Jun 10, 2025
@casperdcl casperdcl mentioned this pull request Aug 11, 2025
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants