Skip to content
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 DSM 6, 7 #4901

Closed
wants to merge 10 commits into from
Closed

Sickchill DSM 6, 7 #4901

wants to merge 10 commits into from

Conversation

BKSteve
Copy link
Contributor

@BKSteve BKSteve commented Oct 9, 2021

To get SickChill a functional SPK in SynoCommunity
Relates to
#4856 #4870 #4867 #4743 #4835

The build is not perfect and has issues due to Python38 build causing cffi package build errors for a number of architectures, along with some rust build issues from that, there is also a lxml issue with man pages throwing an error in the build process.
To overcome most of the Python38 issues (other than cffi) the cross makefile lines 89 & 90 were changed to use newest versions of other packages required to build "setuptools>=58.0.0" "wheel>=0.36.2" "cffi>=1.14.5". Yes going >= was what i chose.
I'm sure there's a way to do this without affecting everybody else using cross/python38 and ask for input from others.

To build the spk is a 3 step process make all-supported then spkclean followed by all-supported provides the 11/11 architectures and DSM 6&7 builds which are fully functional, load and work.
All the builds and testing are here https://github.com/SickChill/SickChill/issues/7504 if you wish to check anything out.

This pull request has makefile numbered as version 3 for SynoCommunity sequence which is the testing 20210729-17.spk packages in 7504.

Checklist

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

@BKSteve BKSteve closed this Oct 9, 2021
@BKSteve BKSteve reopened this Oct 9, 2021
Copy link
Contributor

@th0ma7 th0ma7 left a comment

Choose a reason for hiding this comment

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

That cffi thing still bugs me, there must be a way to avoid this compile issue... I tried a few things but wasn't able to identify the root cause yet.

Comment on lines -17 to -23
PRE_CONFIGURE_TARGET = dante_pre_configure

include ../../mk/spksrc.cross-cc.mk

.PHONY: dante_pre_configure
dante_pre_configure:
$(RUN) autoreconf -i -f -v
Copy link
Contributor

Choose a reason for hiding this comment

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

This addition along with the patch allows it to build on DSM7. Otherwise it does the following:

$ make arch-armv7-7.0
...
checking getaddrinfo() error symbols... configure: error: in `/home/spksrc/BKSteve/spksrc/cross/dante-sockd/work-armv7-7.0/dante-1.4.2':
configure: error: error: getaddrinfo() error value count too low
See `config.log' for more details
make[2]: *** [../../mk/spksrc.configure.mk:50: configure_target] Error 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's very interesting and somewhat odd.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed but it's the only workaround I was able to find online... Feel free to figure a better option, at least on master it now build ok with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm open to any solutions, I'm still at the bottom of the S curve with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have the patch for dante-sockd and I can't get that error message when building armv7-7.0 like you show.
If make armv7 then I get the cffi error and second build after spkclean is good.
I'm a little confused now.

Copy link
Contributor

Choose a reason for hiding this comment

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

My guess is that your tree wasn't rebased against master as this update to version 1.4.3 is the last commit I pushed in and therefore your tree is removing this last commit on master.

The error I'm referring to is happening on all DSM7 builds and was caught on github-action from #4797.

Copy link
Contributor

Choose a reason for hiding this comment

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

So you could just discard all the dante changes and use what's on master.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please all dante related changes as this is most probably due to your branch not being rebased against master as this was merged part of PR #4898 and yours removes it entirely and it isn't related to Sickchill at all.

spk/sickchill/src/service-setup.sh Show resolved Hide resolved
@BKSteve
Copy link
Contributor Author

BKSteve commented Oct 16, 2021

I tried a build with the Python 3.8.12 PR and still had cffi issues with 88f5281 most others were able to compile first build

@th0ma7
Copy link
Contributor

th0ma7 commented Oct 16, 2021

Presuming you mean 88f6281 arch, any other? Looking at my latest logs I couldn't find anything related to a build issue with cffi anywhere in my build-*.log files. All looked good from a python update PR standpoint.

Maybe its within Sickchill Makefile ?

@th0ma7
Copy link
Contributor

th0ma7 commented Oct 16, 2021

Also, you haven't rebased against master...

@@ -86,8 +86,8 @@ python38_post_install: $(WORK_DIR)/python-cc.mk
@$(RUN) $(PYTHON_NATIVE) -m crossenv $(STAGING_INSTALL_PREFIX)/bin/python$(PKG_VERS_MAJOR_MINOR) $(WORK_DIR)/crossenv/
. $(WORK_DIR)/crossenv/bin/activate && $(RUN) wget https://bootstrap.pypa.io/get-pip.py -O - | build-python
. $(WORK_DIR)/crossenv/bin/activate && $(RUN) wget https://bootstrap.pypa.io/get-pip.py -O - | python
. $(WORK_DIR)/crossenv/bin/activate && $(RUN) build-pip install "setuptools==44.1.0" "wheel==0.36.2" "cffi==1.14.5"
Copy link
Contributor

Choose a reason for hiding this comment

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

If I were you I would really avoid using >= mechanisms. It would make the build non repeatables.
Similarly it would be worth updating all cross/* equivalents to the same versions to be on the safe side.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this is superseeded by #4902 and should be removed from your PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the >= was an ugly work around during the testing and the errors being thrown that's why I mentioned in first post.

Now 3.8.12 is in master I'll update and rebase instead of using the combination of clone of th0ma7 3.8.12 branch from his repo along with my sickchill copied to that clone.

@BKSteve BKSteve closed this Oct 17, 2021
@BKSteve BKSteve deleted the Sickchill-6-7 branch October 17, 2021 05:23
@BKSteve
Copy link
Contributor Author

BKSteve commented Oct 17, 2021

Still active, just deleted and recreated branch.

Here's my log of 88f6281 build where it looks like poetry is trying getting cffi from 35 down until is gets to 3.3.2 for success.
Line 17680 is the first Error:
build-88f6281.zip

But does this constitute an actual issue? As a spk was built, so it should install and load.

Note, I only have x64 DSM so have to get others to test them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants