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

Deluge Generic Service approach #3153

Merged
merged 1 commit into from
Mar 1, 2018
Merged

Conversation

Safihre
Copy link
Contributor

@Safihre Safihre commented Feb 15, 2018

Deluge works fine, also with the Python 2.7.14 package. This is unfortunate, since the problems reported after that Python update must then be arch specific. I wouldn't know how to test that 😢

I tried to update cross/libtorrent and cross/boost but it gave some problems that I couldn't resolve so far.
Package added to the list in #3138.

Checklist

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

@ymartin59
Copy link
Contributor

Good job. One detail: french translations ;)
I am uncomfortable about python failure. It may be useful to collect impacted models and architectures. If identified I can exclude them from update.

@Safihre
Copy link
Contributor Author

Safihre commented Feb 15, 2018

Probably all non-x86/64 🤒 There is no way to emulate those in a virtual machine of some kind, is there?
Maybe I can find a cheap second hand one.

@ymartin59
Copy link
Contributor

Mine is evansport, 32 bits but still x86. Running DSM intel based system on virtual machine is not easy... so I fear it is far more complex to get it run on qemu or other platform emulator

@Safihre
Copy link
Contributor Author

Safihre commented Feb 17, 2018

@ymartin59 I updated the Generic Service to include set_unix_permissions because also here in Deluge (just like Sonarr/Radarr) I needed to set unix permissions. In this case on the env directory, as was reported in #3138.

@Safihre
Copy link
Contributor Author

Safihre commented Feb 17, 2018

@ymartin59 What I do not understand is why this package includes the whole of python?!?
It's listed as a BUILD_DEPENDS but for some reason it gets copied to the lib directory.. This is not the case for other packages like SABnzbd. Do you maybe know why?

EDIT: Found it, it's becuase cross/boost lists cross/python as a dependency.. Not sure if that's true and that it's not just a build-dependency.

EDIT 2: Yeah, probably it needs that for Boost.Python. Makes sense.

@ymartin59
Copy link
Contributor

OK. But there is probably means to point Python libraries for linking Boost API instead of embedding it in package... the same I did for chromaprint (#3136) thanks to search location to spk/python and RPATH variable in produced ELF binaries.

@Safihre
Copy link
Contributor Author

Safihre commented Feb 17, 2018

@Diaoul do you know if Boost really needs cross/python to be included or just at build-time? I see you specifically made these changes back in 2013: 8a9e3dc

@Safihre
Copy link
Contributor Author

Safihre commented Feb 18, 2018

I changed it from DEPENDS to BUILD_DEPENDS and the Deluge package is ~20MB smaller and still works as far as I can see.
The only other package using Boost is domoticz, but that one doesn't use Python so should be fine.
Would still like some feedback from @Diaoul just to be sure 👨‍🎓

@ymartin59
Copy link
Contributor

@Safihre Just as a notice... Boost 1.63 no longer build because of download/checksum issue. I am investigating

@Safihre
Copy link
Contributor Author

Safihre commented Mar 3, 2018

Probably Sourceforge.net being offline, it's been a major issue the past 3 weeks. They had so many outages.

@ymartin59
Copy link
Contributor

@Safihre You're right ! From distrib/boost_1_63_0.tar.bz2.wrong: "We're sorry -- the Sourceforge site is currently in Disaster Recovery mode". I will try to manually get it from elsewhere.

@ymartin59
Copy link
Contributor

This package update would have been opportunity to update Boost to latest, even if Deluge itself is not updated. For next time...

@ymartin59
Copy link
Contributor

@Safihre I find it quite heavy to get Python rebuild for Deluge. Do you think it may be possible to reuse previous python (if it already exists), the same way I did dynamic linking to ffmpeg libraries: f3d629f#diff-b18021950851bbb4e94acd6843c4156cR22

@Safihre
Copy link
Contributor Author

Safihre commented Mar 3, 2018

Aah, I see I never explained it here. But libtorrent 1.1 is a bigger updated than the small version increase indicates. So I didn't want to include that.
Additionally it seems the same is true for Boost 66, which also makes some minor changes that cause libtorrent to fail to build. Also libtorrent 1.1 (that says in the release notes its fixed for Boost 66) doesn't cross compile.
So that's the reason to keep these versions.

@Safihre
Copy link
Contributor Author

Safihre commented Mar 3, 2018

Python can indeed probably be linked like that. Maybe it is better to find some way to make these libraries easier reusable?
To not have them compile in the SPK folder but in their own folder? Just like native packages do?

@ymartin59
Copy link
Contributor

@Safihre Yes. But there may be some troubles, as a spk or library can pass additional arguments to a dependency... so result may depend on caller. I think it cannot be generalized easily. Doing so only for large shared pieces like ffmpeg, python, and maybe boost if shared too (I have not checked yet) may be enough to speed up round-trips like clean/build sequence.

@Safihre
Copy link
Contributor Author

Safihre commented Mar 4, 2018

Honestly I think the solution here is not to do this hardlinking to other packages. The future proof solution in my opinion is to use Travis to do the publishing.
Not any of the automated stuff yet, but just manually set the build matrix to the package and archs you want, like me and @m4tt075 have been doing. But then include the publishing step.
Of course testing compiles will be local, but the final step to publish all of the archs Travis can do so so much faster and has no limits.
I suggested it couple of times, but you don't seem to like this idea?

@ymartin59
Copy link
Contributor

@Safihre I like automation and probably Travis is the best option... but I do not consider it first priority. If you want, just do progress on it.
From my point of view, group permission to get producer/consumer file sharing pattern properly working is a requirement before publishing additional impacted packages.
Deluge publication is pending for that reason.

@Safihre
Copy link
Contributor Author

Safihre commented Mar 7, 2018

👍 makes sense. Unfortunately, I don't understand what is going wrong with these permissions.. I wouldn't even know what to ask users to check or change. Do you have new ideas?

I also want to fix #3201, this is the final issue for Python that also came up with the previous test release.

@Safihre Safihre deleted the just-deluge branch March 7, 2018 21:54
@ymartin59 ymartin59 removed the bug label Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants