-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
SickChill 20240301 test #6007
base: master
Are you sure you want to change the base?
SickChill 20240301 test #6007
Conversation
@BKSteve is it possible to already try this 20240301 test package out? Best, C. |
This is not a significat overhead. Python needs to be built anyway to build packages that depend on python. Since we reuse prebuilt python for such packages, the github build action creates the python package first. If you build sickchill locally, you can benefit from prebuilt python, if you build python spk before sickchill. While development you can rebuild sickchill (make clean and make arch-*) without recreating python. This saves a lot of time when building multiple packages that depend on python, but even for a single package you save time if you have to build a package more than once. We added the reuse of prebuilt dependent packages for packages depending on ffmpeg and packages depending on python. EDIT: |
Any advice on fixing qoriq build failure? How to disable/avoid qoriq? |
ccbb192
to
5164181
Compare
@BKSteve, I tried to run a clean install but got the following error: Install Log
|
@mreid-tt yeah had a little typo in the requirements file for zipp. Now zipp==3.21.0 should work. |
@miigotu is there a new release of sickchill on the way? |
I think I may have messed up by doing a rebase on a foreign branch as noted by @hgy59. I now see this commit has changes across a number of packages as it appears to roll in the changes to the repo since. @hgy59, would this cause a huge issue when squashed and merged? |
@BKSteve do we really need unit testing in a release build? |
@mreid-tt it tells Merge remote-tracking branch 'origin/sickchill-20240202' into sickchill-20240202` |
some findings of my test installation: At package installation the call of All (cross compiled) wheels are downloaded and not installed from *.whl files in the package. For python packages that I maintain (homeassistant, octoprint) I never use
And for the pure python modules (not in the package):
The same solution is used in ffsync (syncstorage-rs) by @mreid-tt. In current sickchill package, we could even call
and create the package without python wheels. I think if the sickchill python package dependencies are complete, it would be enough. But my findings are, that the following are not installed this way
I think it is time for a redesign (or removal) of we should consider two options/versions for wheel installation
/cc @th0ma7 |
No, probably don't need these dev related items. I was and still am somewhat out of my depth in regard spk building. I just took over a while back as nobody was doing it and got it to install with lots of assistance from the team here, especially on recent builds with the DSM 7 changes and deprecation forced by other packages.
Many things are from looking at other packages and bringing them across, grateful for any advice pointing me in the right direction particularly on the question posed. |
@hgy59 this is trickier than you might think... At package generation time a resulting Further, in trying to save storage space online all pure packages where removed from the default packaging unless setting Using this current method, and considering pure python wheels are downloaded from the index, This means for cross-compiled wheels, as joint under the unique Also note that (and to be confirmed), a downloaded |
@BKSteve shall we rebase or start a new PR from scratch? |
During testing I put most of these in requirements.txt as it brought the dependencies sub dependants into the build as they didn't seem to be loading for all DSM HW. All my DSM are x64 so I relied on others to test on different hardware. For x64 devices it would probably just need sickchill== xxx and most dependencies and sub dependencies would be pulled in but some are in But it's the other DSM HW that are the issue to build for and test to get a working package. |
@BKSteve can you please rebase with upstream/master? (or shall @mreid-tt or @hgy59 do this for you?) we should get rid of the commits not related to this PR. EDIT: |
f596fae
to
94e0fd0
Compare
Damn, fun and games with git via pycharm. |
@BKSteve this has still failures at installation:
with the current requirements the build of ARMv7L still works. I have a working version with
Since this package does not support updates via github, I assume that PyGithub/PyJWT are obsolete I could add a commit or provide the files for further discussion. My local version runs as x64-7.1, aarch64-6.2.4 and armv7-7.1. |
Any inputs most appreciated |
- remove obsolete requirements - remove requirements not referenced by sickchill - including requirements for dev package and git installs - update greenlet and add version specific requirements files - use greenlet==3.1.1 for DSM 7 (gcc >= 5) - limit greenlet to 3.1.0 for gcc < 5 - downgrade pyOpenSSL to 23.2.0 (compatible with cryptography==41.0.3) - enable build for ARMv7L - move charset_normalizer to crossenv requirements
Note Mediainfo has been removed, although not a specific SickChill requirement if installed it helps read file data and is loaded through a
What would be best? Pure |
@BKSteve Ok, I understand There are some extra features (optional) in Sickchill
When talking of pymediainfo I suppose we should include the speedups extra feature. Can you tell me what the types and frontend features are for? |
The front end is going to change in the future and is prep phase for future. Thanks for the assistance. I've shared the link to builds for others to test via discord. |
Update to 2024.3.1
Fixes many things.
Updates packages and removes unused.
Aligns requirements-crossenv with latest Python 3.11.5-8
Checklist
all-supported
completed successfullyType of change
Do not merge