PoseLib
PoseLib copied to clipboard
Pypi packaging
Resolves #72
Added build for multiple platforms. Also some fixes for platform specific issues.
Mac complained about "aligned-allocation" so compiling without it. Windows broke because the MSVC flag was missing in the pybind cmakelists.
Currently no integration with the test suite, should add. Also wheels seem to be sometimes broken, so we need to test these on some different systems before pushing pypi.
@pablospe Currently it will build on push to master, which I think is reasonable. I guess for other CI stuff we want to run on PRs, especially the test suite. Not sure how extensive we want to test, the current builds are for a wide range of setups, but that takes a very long time to run. Perhaps limiting the testing to just some ubuntu/python combo should be enough.
In order to have a Github Action, you will need to set-up one first by clicking here: https://github.com/PoseLib/PoseLib/actions/new
Then copy the files content of this PR to the new one, and close this PR to avoid confusion.
@vlarsson Is there any news about this PR?
I will have a look at this next week after the 3DV camera ready deadline.
So this should upload to pypi when we make new releases? or I am not sure I am understanding the build-pypi.yaml.
you will probably need to create a new github action, see my previous message: https://github.com/PoseLib/PoseLib/pull/73#issuecomment-1741845945
@Parskatt You might need to create a Github Action first, by clicking here: https://github.com/PoseLib/PoseLib/actions/new
Then copy the files content of this PR to the new one. The github actions from build-pypi.yaml
doesn't trigger otherwise, you can see it doesn't run in the "Checks" tab.
Sorry, I did not mean to cause this review. Was just gonna fix stuff over at my fork. Perhaps we can disable the autoreview? I'll get back to you later.
@pablospe Ok, but the action won't run without the changes to rest in the PR, if I push the action first it will start running on pushes to master. Wouldn't that be kind of annoying? Maybe I'm misinterpreting you.
Also some general discussion:
- I had to set BUILD_SHARED_LIBS manually off, since it seemed to mess with compatability.
- Same for -march=native, had to remove it due to issues on mac @vlarsson . This is a bigger change. I ran the solver benchmarks, and I saw no diff in performance on the machine I tried. It could of course be worse in certain cases. I'm a super cmake noob, but it was also somewhat annoying that that benchmark binary ends up in a pile of build files, does files I have no idea where they would end up on the runners.
- Also had to set -fno-aligned-allocation on mac to get stuff to compile.
@pablospe Ok, but the action won't run without the changes to rest in the PR,
I was suggesting to create another PR where the actions are active, that's all, as now the build-pypi.yaml
is not running automatically, but the PR content should be the same, of course (only different PR). One question, how are you testing this? Your own fork/branch with manual triggering of the action?
If you see here:
only one
github Actions
is running and your .yaml is ignored. This is the reason I was suggesting to start a new PR where the action is working and you move the content (probably merging with the branch on this PR).
And one also wants to run it on master because it takes long time to run, and because you don't want to push artifacts to pypi from untested and non-approved branch.
if I push the action first it will start running on pushes to master. Wouldn't that be kind of annoying? Maybe I'm misinterpreting you.
Not sure what you mean. The action will run in the current branch on the PR, later you will need to filter and say the publishing of the artifact should run only in master, so when you publish a package you know it is from master, but this has to be done explicitly (it is not the default). Probably we are talking about something different?
Also some general discussion:
- I had to set BUILD_SHARED_LIBS manually off, since it seemed to mess with compatability.
This is fine, just wondering how it works with the python bindings? Doesn't it needs a .so? (at least in linux)
- Same for -march=native, had to remove it due to issues on mac @vlarsson . This is a bigger change. I ran the solver benchmarks, and I saw no diff in performance on the machine I tried.
You can also check the benchmark result when the action is running: https://github.com/PoseLib/PoseLib/actions/runs/8214094373/job/22466214680?pr=73#step:7:72
-
Click here: https://github.com/PoseLib/PoseLib/actions/new
-
and then:
-
then you put the content/files from this PR.
@pablospe The issue is that we're building wheels, and the binaries don't end up in any convienent place as far as I can tell. In the basic CI that you already made, its easy to set the install_dir and simply run, but when building wheels I don't know where these targets end up.
Regarding testing the wheels, yes, they are already set up and running on my fork. I guess I can set up a new PR with your approach, so that the action runs there
@pablospe I followed your strategy above (see https://github.com/PoseLib/PoseLib/pull/95) , but this still only runs the already existing action (main.yml). I don't think its possible to run new CI in a PR.
Now it is running, see the changes (you need to revert these before merging to PR, now it is for testing until you get it running): https://github.com/PoseLib/PoseLib/pull/95/commits/7ac7e9ff368577e423423444579d65ad783d1ca8
So, it wasn't triggering because in your PR/branch because you added:
on:
push:
branches:
- master
For testing you can comment it out, then when things are ready you can put it again so it only runs on master for the following PRs.
should we close this PR? And move to the new one you created?