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

chore(deb): Update debianization #1697

Merged
merged 9 commits into from Jan 6, 2024
Merged

chore(deb): Update debianization #1697

merged 9 commits into from Jan 6, 2024

Conversation

iluminat23
Copy link
Contributor

  • Drop CDBS and debhelper with cmake instead
  • cleanup dependencies
  • Use external libquazip1-qt5 (requieres Debian >= Bookworm || Ubuntu >= Lunar)

@bugwelle bugwelle changed the title Update debianization chore(deb): Update debianization Dec 27, 2023
Copy link
Collaborator

@bugwelle bugwelle left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR! You're the hero I need! 😄

I had to revert to the previous debian/ files while packaging the latest version, because a few things didn't work properly (I think because I still wanted to build for Ubuntu 18/20).
I still have a few dozen Wiki pages open in my browser: all about Debian/Ubuntu maintainer guides / packaging guides that I never got to ready 100%.

So I very much appreciate your PR! Before approving (and merging), I still have a few questions. It would be awesome if you could answer them, especially because it helps me to understand the packaging process better.

Thanks!

@@ -3,11 +3,10 @@ Section: misc
Priority: optional
Maintainer: Andre Meyering <info@andremeyering.de>
Standards-Version: 4.6.2
Build-Depends: debhelper (>= 13.0.0),
cdbs,
Build-Depends: debhelper-compat (= 13),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. I had issues with debhelper-compat locally, so switched back to debhelper + compat file. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you still want to build for Ubuntu 18, we need to go to debhelper 11. If i recall this correctly it is since debhelper 9 that the compat file can be dropped and debhelper-compat can be used as dependency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, Ubuntu >= 20 is fine. Builds failed for Ubuntu 18 anyway, because of some missing header file (some header has incorrect casing, e.g. Qmultimedia/ instead of QMultiMedia/ and I never found the root cause). Also, IIRC, Ubuntu 18 is EOL. If there are still users on Ubuntu 18, they can use the AppImage. :)

libzen0 | libzen0v5,
zlib1g,
Depends: ${misc:Depends},
${shlibs:Depends},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you bring me up to speed here? Why are the runtime dependencies not listed here? At least libmediainfo is loaded at runtime and auto-detection (if any used) fails. That's at least what's happening with openSUSE:

Requires: libmediainfo0 ffmpeg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I missed that.
The shlibs:Depends will resolve all the library dependencies. But only those which are done by the linker. Any library loaded with dlopen needs to be added manually to the dependencies.
https://www.debian.org/doc/debian-policy/ch-sharedlibs.html#generating-dependencies-on-shared-libraries

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh my god, that is awesome. I wasn't aware of that! I read that page once, but apparently didn't read it properly 😄

@@ -1,8 +1,14 @@
#!/usr/bin/make -f

QT_SELECT=qt5
export QT_SELECT
export DEB_BUILD_MAINT_OPTIONS = hardening=+all
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought those flags would be passed to the build via environment variables. Isn't it best practice to rely on env variables passed by e.g. launchpad.net's build system instead of hard-coding them ourselves?

(I've never fully read the debian maintainer guide, so no idea what's best practice :) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The package maintainer should know best which flags are safe to use for the package. So the build environment doesn't impose much on the package.
Make doesn't mind the spaces and this is taken directly from the Guide for Debian Maintainers
The Debian New Maintainers' Guide is replaced by the Guide for Debian Maintainers. The new guide is much better, and has more and better examples.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, thanks! All those flags are what I expected launchpad.net to add anyway.

I think we could even switch to Qt 6. I will use the "old" debian/ files for Ubuntu 20 and later anyway.
But you don't need to do that! I want to update all build-instructions to Qt6 next year anyway and will try to update this then.

I also thought about moving these files to a separate "packaging" repository once. But I'm still unsure whether that's a good idea. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my company we use http://honk.sigxcpu.org/projects/git-buildpackage/manual-html/gbp.html and different branches for the different distributions.
Then the debianization is located only on the distribution branch. This mirrors the way Debian/Ubuntu are managing packages. As they use the software from upstream and put a debianization on top of it.

debian/rules Outdated Show resolved Hide resolved
@iluminat23
Copy link
Contributor Author

As you wrote you need to also package for Ubuntu 18 and 20. You bundle the libquazip1-qt5. And this is a bit tricky. For system which have a libquazip1-qt5 this leads to collisions. The debianization for systems which have and those without libquazip1-qt5 needs to differ.

The debianization I did is not working with the bundled libquazip1-qt5. Give me a bit to find a solution for this.

@iluminat23
Copy link
Contributor Author

A question out of the direct scope of this ticket. Is libcurl anywhere used? I greped through the git repo and didn't find any reference that libcurl is used. Only the linker flags. Or is it implicit through some dependency?

@bugwelle
Copy link
Collaborator

Or is it implicit through some dependency?

IIRC, only implicit through Qt (unchecked, though!). We only use Qt's network capabilities, no raw curl commands. I'm not the original author of the debian packaging files, so no idea why it was added :)

Thank you very much for your detailed responses. I'm currently visiting my family, so can't test it myself (using the https://launchpad.net/~mediaelch/+archive/ubuntu/mediaelch-test repo), but will do so in January. :)

There is currently no user of libcurl in the code base. So we don't need
to link against libcurl at all.

Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
There seems to be no user for libpulse. If it is a dependency
of any of our dependencies, they should depend on it.

Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
The current package build with cdbs has several issues. Parallel build
is not working. The cdbs helper seem to be incompatible with debhelpers
>= 11. The debhelper.mk is using dh_systemd_enable which was replaced
with debhelper 11.

Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
libmediainfo-dev depends on libmediainfo0 | libmediainfo0v5 listing them
in the Build-Depends is redundant.

Remove unnecessary qt tools from build deps

The debhelper seem to be fairly good to determin the Depends them
selfs. So just keep the ${misc:Depends} and ${shlibs:Depends}
dependencies.

Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
As libquazip1-qt5 is available since Debian Bookworm and Ubuntu Lunar,
use the system library.

Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
Signed-off-by: Philipp Rosenberger <code@iluminat23.org>
@iluminat23
Copy link
Contributor Author

Hi, sorry for the delay.
I updated the debianization:

  • drop libcurl (also from MediaElch.pro and src/CMakeLists.txt
  • drop libpulse from build deps
  • readded some build/runtime dependencies
  • added libxkbcommon-dev to the dependencies
  • use Qt6 now

This works for Debian Bookworm ans should work for Ubuntu >= 22

@bugwelle
Copy link
Collaborator

bugwelle commented Jan 3, 2024

Hi, sorry for the delay.

Don't be! I still on vacation and didn't get a proper look at this PR, yet. My TODOs from #1697 (comment) are still open.

This works for Debian Bookworm ans should work for Ubuntu >= 22

You're awesome, thanks!

I'll review it properly on the weekend/ next week. But so far (on a quick glance), it looks good to me.

Would you be ok with me mentioning you in the changelog with a small note saying something along the lines of "updated debian packaging to Qt6, thanks to GitHub user iluminat23"? (or your real name or no mention at all; up to you)

Regards,
Andre

@iluminat23
Copy link
Contributor Author

I'll review it properly on the weekend/ next week. But so far (on a quick glance), it looks good to me.

I just wanted to do this before I'm back at work. Take your time!

Would you be ok with me mentioning you in the changelog with a small note saying something along the lines of "updated debian packaging to Qt6, thanks to GitHub user iluminat23"?

I'm fine with either.

Best regards,
Philipp

Copy link
Collaborator

@bugwelle bugwelle left a comment

Choose a reason for hiding this comment

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

Thank you very much! It works with Ubuntu 23.04 (Lunar).
I will have a look at your links later (I'll create a backlog item for that)

Notes

  • release build works
    • cmake
    • qmake
  • debug build works
    • cmake
    • qmake
  • local packaging works ( ./.ci/linux/package_linux_launchpad.sh deb)
  • launchpad packaging works ( ./.ci/linux/package_linux_launchpad.sh launchpad)
    • can be uploaded
    • can be build on launchpad.net
    • can be installed via apt

Other Notes

``` W: mediaelch source: inconsistent-appstream-metadata-license data/desktop/com.kvibes.MediaElch.metainfo.xml (cc-by-4.0 != lgpl-3.0-only) [debian/copyright] W: mediaelch source: no-debian-changes ```

Build via https://launchpad.net/~mediaelch/+archive/ubuntu/mediaelch-test/+build/27615875

Accepted:
 OK: mediaelch_2.10.7-dev.12.orig.tar.xz
 OK: mediaelch_2.10.7-dev.12-1~lunar.debian.tar.xz
 OK: mediaelch_2.10.7-dev.12-1~lunar.dsc
     -> Component: main Section: misc

mediaelch (2.10.7-dev.12-1~lunar) lunar; urgency=medium

  * next build
$ apt search MediaElch
Sorting... Done
Full Text Search... Done
mediaelch/lunar,now 2.10.7-dev.12-1~lunar amd64 [installed]
  Media Manager for Kodi

@bugwelle
Copy link
Collaborator

bugwelle commented Jan 6, 2024

Changelog entry via b3d7b7c

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.

None yet

2 participants