-
Notifications
You must be signed in to change notification settings - Fork 61
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
Packaging improvements #96
Comments
And you might also be interested in the configuration of mne-lsl, We can't ditch the internal
|
Hi @mscheltienne , I tried to explain it in #95 but I can see how that got lost with all the other spam this repo was producing yesterday thanks to me. In short, I want the win_amd64 and win32 builds to include liblsl (lsl.dll) because Windows users have a tougher time installing liblsl on the PATH. However, for ALL OTHER PLATFORMS, I don't bundle liblsl because it's relatively simple to build/install -- the strategy instead is to provide a useful error message when liblsl can't be found. Back when we were bundling the binaries with pylsl, there was always a platform or 2 users had that weren't covered and it wasn't worth the effort to support them all. A couple questions for you:
|
I missed #95 entirely 🙈 Alright that makes sense. For For macOS code signing.. good question and I don't have a definitive answer. I hope we are taking this into account (I'll have to check on a macOS laptop on Monday), my guess is that either |
*The tag is |
Hi @cboulay! I see that you pushed quite a few nice changes. I have a couple of questions and tips for the packaging. I now see:
pylsl/pyproject.toml
Lines 67 to 68 in fb0ae2e
Accompagnied with attempts to re-target wheels on windows 32 and 64. Unless mistaken, you are not building and packaging
liblsl
withinpylsl
, thus you should have a source distribution and a single binary distributionpylsl-**-py3-none-any.whl
. Overall, you should be able to get rid ofsetup.py
entirely and since you reverted tosetuptools
, specify the followingpyproject.toml
:The text was updated successfully, but these errors were encountered: