SickChill: fix broken package and update

[BKSteve]

Motivation: Fix ownership on the DSM6 package so SickChill can update successfully without having to ssh and chown the env folder
Linked issues: #4703 and many on SickChill https://github.com/SickChill/SickChill/discussions/7364 https://github.com/SickChill/SickChill/issues/7316 #7234 #7249 #7265 #7293 #7295 #7321 #7327 #7347 #7410 #7411

This is the first fix to get the ownership of the /env folder under sc user rather than root so that updates of SickChill can access and utilize the folder for updated python packages. Therefore it's numbered as 20210329-2 as it uses the same original version of SickChill in the cross environment with only the source location changed so it loads.

Yes this still has the #4538 item 1) as python is referred to using \usr\local\python38. But as this is an intermediate small fix only for DSM6 and in prep for the move to a combined DSM6 and DSM7 package this will be tackled soon.

If others wish to try this version here's a link to the spk. sickchill_noarch-dsm6_20210329-2.spk

Checklist

• Build rule all-supported completed successfully
• Package upgrade completed successfully
• New installation of package completed successfully

[publicarray]

Thanks the Installation and upgrade succeeds and fixes the package. Unfortunately running the internal updater breaks the package again. Can this be fixed? It might be something the sickchill developers have to do.

FYI installation logs this error but does not appear to impact the package.

grep --ignore-case error /var/log/packages/sickchill.log
ERROR: SQLAlchemy-1.3.20-cp37-none-any.whl is not a supported wheel on this platform.

[miigotu]

Some changes to SC are related to this, and the next release/push to master from develop fixes updating. But we absolutely need fixes in the Synocommunity package for it to ever work. I wish your team would open up a little on changes to this specific package, because it only effects users of SickChill, and we absolutely know what we need for it to be able to run on syno, reliably. If we aren't creating a security issue or some sort of system wide critical error please just let some changes in. If you want a huge change set at once, let us know. We can do a bunch of small PRs with easy to review changes, or a major PR that makes it work for dsm7 and also fix the installer and updater issues at the same time. Just tell us what we need to do to get something merged, otherwise we need to go back to releasing our own spk's, which I don't want to do really.

[publicarray]

I just thought this would be an easy PR to merge and fix the package and would obviously prefer if the updater worked.
But can publish it like this to at least fix the installation of the published package.

I haven't looked at #4743 yet, It looks like it doesn't compile yet.

If you want a huge change set at once, let us know. We can do a bunch of small PRs with easy to review changes, or a major PR that makes it work for dsm7 and also fix the installer and updater issues at the same time

Ideal PR size is hard to judge. Usually 1 PR per SPK package (can include all of: updates/features/cross packages/very small core framework changes) and 1 PR for large changes needed in the framework. If changes to dsm7 are significant enough it might be best to separate it out to a separate PR.

But we absolutely need fixes in the Synocommunity package for it to ever work. I wish your team would open up a little on changes to this specific package, because it only effects users of SickChill, and we absolutely know what we need for it to be able to run on syno, reliably. If we aren't creating a security issue or some sort of system wide critical error please just let some changes in.

I'm not sure if I understand, Maybe it helps to explain my/our general process. @hgy59 feel free to correct me

All packages must compile on all architectures that report themselves as supported. (e.g. pass CI [sometimes there is an exemption e.g. when a known broken package courses an error in the CI, this is from when the CI did not exist]). We can help get you there, but our tiny team is stretched pretty thin. Then we look over the code (I think this where everyone has a slightly different view on things e.g. I don't have much knowledge of Python with the framework, so I usually don't review them, but I know DSM requirements well), Lastly we build package and do some tests (in my case in a virtual environment) of basic functionally and look in log files for errors. Then we merge and publish (master is stable and is what is published)

IMHO this PR failed the update test which I think is part of basic functionality testing. The package is otherwise functional. But since this package is already broken I'm happy to merge this.

As to my question then

Can this be fixed?

I Just wanted to know what the plan with the updater is. The answer would determine if there are any changes I would recommend adding to this PR. If for instance it would not work in any way we would have to notify users that the updater is not working. So should I wait until develop is merged then publish this?

[publicarray]

Never mind. I included the patch here

[publicarray]

@hgy59 I'm sorry but can you delete the dsm7 version. Using the CI to publish no-arch packages was not a good idea of mine...

[BKSteve]

DSM 6 spk while we wait for sc release.
sickchill_noarch-dsm6_20210729-2.spk - deleted link
Use link below.

[publicarray]

You can use this link too.

https://packages.synocommunity.com/sickchill/2/sickchill.v2.f7321%5Bnoarch%5D.spk

[BKSteve]

Hi guys, the attempt to put the patch on the file init_helpers.py hasn't been successful people are getting failures.
They used the linked file in post above.

@miigotu can you push the fix to master please.

[publicarray]

Hi guys, the attempt to put the patch on the file init_helpers.py hasn't been successful people are getting failures.
They used the linked file in post above.

Please be specific what fails? I know the patch does not fix the updater (the updater replaces the file), but it did fix the build which I assumed this was about

[BKSteve]

No, the fix for init_helpers.py is required when SickChill runs after installation.
It is when the SickChill package is running and runs the pip string to start downloading and installing all the components required for the main package to run. it is not required to compile.

I was under the misconception that it actually modified the code in the file being built into the package.
Anyway this package fixes the env ownership issue and I requested the init_helpers fixed version be pushed into master so the cross compile version will need to be updated once this is done.

[miigotu]

As soon as I can get to my laptop, I will make a release of SC, but this patch should apply on a new install. I'm only concerned if patching here breaks the updater in SC. Ideally, we should remove the patch and update the commit hash included in the package. (This is why it was not pinned to a specific release originally, it was a git pull from the master branch (which is always the latest release) just like sickbeard-custom.) Any errors were easily fixed on SC end without a new PR here.

[miigotu]

Thanks for merging this though, it's a step in the right direction and what is most important is to see progress. Otherwise it just gets frustrating having perpetual requested changes on PRs that actually work.

[BKSteve]

The patch inside the package doesn't seem to have any adverse effects, I removed the patch and built the package. Installed it, swapped the init_helpers file and it starts and with some basic testing functions.
Just hanging for the new package to drop.

I also built DSM7 version and did a test and installs.
So once the updated SickChill is available I'll do a full test on both 6 and 7 and create a new pull request linked to this one.
As we've already moved to 20210729 I don't believe git is a requirement anymore, so I'll test that too.

[miigotu]

The patch inside the package doesn't seem to have any adverse effects, I removed the patch and built the package. Installed it, swapped the init_helpers file and it starts and with some basic testing functions.
Just hanging for the new package to drop.

I also built DSM7 version and did a test and installs.
So once the updated SickChill is available I'll do a full test on both 6 and 7 and create a new pull request linked to this one.
As we've already moved to 20210729 I don't believe git is a requirement anymore, so I'll test that too.

Yeah I've been working on it, there were some issues with current develop and I'm not about to be rushed into another disaster release to partially help only one ecosystem. Maybe tomorrow it will be sorted out for testing.

[miigotu]

The patch inside the package doesn't seem to have any adverse effects, I removed the patch and built the package. Installed it, swapped the init_helpers file and it starts and with some basic testing functions.
Just hanging for the new package to drop.

I also built DSM7 version and did a test and installs.
So once the updated SickChill is available I'll do a full test on both 6 and 7 and create a new pull request linked to this one.
As we've already moved to 20210729 I don't believe git is a requirement anymore, so I'll test that too.

Yeah I've been working on it, there were some issues with current develop and I'm not about to rush into another disaster release to partially help only one ecosystem. Maybe tomorrow it will be sorted out for testing.

[BKSteve]

The patch inside the package doesn't seem to have any adverse effects, I removed the patch and built the package. Installed it, swapped the init_helpers file and it starts and with some basic testing functions.
Just hanging for the new package to drop.

I also built DSM7 version and did a test and installs.
So once the updated SickChill is available I'll do a full test on both 6 and 7 and create a new pull request linked to this one.
As we've already moved to 20210729 I don't believe git is a requirement anymore, so I'll test that too.

Based on the #4856 issue thread it's obvious the online package is duff.
Please advise what actions are required to rectify the failing online package?

Also in my own compilation of the package for DSM7 it is also functional. Please consider releasing the DSM7 version too when the current version is corrected.

[publicarray]

I've already unpublished it yesterday, so It already stops showing up as an update in the package center. The website will still show it though

[BKSteve]

The 20210329-1 version also doesn't load now due to #4861

[BKSteve]

As I'm working on a fix for all the issues that have become apparent with moving to Python 38 package and particularly for ARM based architectures I have the code and requirements for 38 but for some reason
The install on a DS216j gives this error
ERROR: tornado-6.1-cp37-none-any.whl is not a supported wheel on this platform.
which implies that it's building a 3.7 wheel, so how can I force the build environment to build a cp38 wheel or fix this error on ARM based machines.
grateful for any help @publicarray and others

Edit: additional from build spk.

Collecting tornado==6.1
  Downloading tornado-6.1.tar.gz (497 kB)
...
  Building wheel for tornado (setup.py): started
  Building wheel for tornado (setup.py): finished with status 'done'
  Created wheel for tornado: filename=tornado-6.1-cp37-cp37m-linux_x86_64.whl size=414483 sha256=8705a07c6363477174bceb6d1473adf870f4abbbd2f6cf76e2ea794607bacf24
  Stored in directory: /tmp/pip-ephem-wheel-cache-ssmx83xm/wheels/02/62/2c/f52c662d8ae374c4abda0c13ce432fb58eb1c75281a27b406c

[BKSteve]

Reverted version 20210329-1 doesn't seem load same requirements.
#4861 (comment)