search

linuxmint / xapp

Cross-desktop libraries and common resources
GNU Lesser General Public License v3.0
126 stars 44 forks source link

Improve Debian packaging #162

Closed uhle closed 1 year ago

uhle commented 1 year ago

The current Debian packaging has several issues that this PR is addressing, most notably:

Unfortunately, I had to add an unreleased version to debian/changelog as an intermediate solution, so that the relations between the packages can be properly resolved (mostly because of the Replaces and Breaks in debian/control). This changelog entry can be fully replaced with the real entry for version 2.4.2 once this is going to be released.

Fantu commented 1 year ago

@uhle thanks for prepare this PR, some fix like 80xapp-gtk3-module.sh path and package split was already done in debian packages (https://salsa.debian.org/cinnamon-team/xapp) but in something seems you did more improvements here I not had a long time so I hadn't yet prepared a merger of the debian improvements, also because it's not easy to get them accepted when they involve significant changes like these in this regard it would be good to prepare a PR in any software that uses xapp to adapt to these package changes (maybe only debian/control changes) in debian I did split "faster" in one additional package instead, the mate-xapp-status-applet it seems good to have it splitted, I will modify it the same also in the debian packages d/copyright still miss something, look the debian one: https://salsa.debian.org/cinnamon-team/xapp/-/blob/master/debian/copyright (the debian/* entry can be ignored) I already proposed the dbgsym migration for all cinnamon components and related software long time ago but was rejected so probably should be splitted on another commit at the end of the serie if they will don't want accept it

Fantu commented 1 year ago

for don't waste time on other changes and related PRs probably is better ask before to @clefebvre @mtwebster if they accept this

uhle commented 1 year ago

Fabio, I know these are quite a lot of packages. It's not like I just would want it to be like that. I do also prefer to have as few packages as possible. But still I have decided for the necessary but least intrusive changes. It is all for good reasons. I have tried to explain why I think these separate packages are necessary with respect to the Debian policies, but it seems that I have to elaborate on it a little more.

So why do I think these changes are necessary? It may help with an example at hand. Let's say we have a dual-architecture system with 64-bit as well as 32-bit GTK3 applications linked to libxapp.so.1 (e.g. amd64 and i386 are both supported by Linux Mint). Then you definitely need to have these files among others:

/usr/lib/i386-linux-gnu/libxapp.so.1 -> libxapp.so.2.4.1
/usr/lib/i386-linux-gnu/gtk-3.0/modules/libxapp-gtk3-module.so
/usr/lib/x86_64-linux-gnu/libxapp.so.1 -> libxapp.so.2.4.1
/usr/lib/x86_64-linux-gnu/gtk-3.0/modules/libxapp-gtk3-module.so
/usr/libexec/xapps/sn-watcher/xapp-sn-watcher

So it is clear that we cannot leave xapp-sn-watcher in the package libxapp1 if we want to install libxapp1:amd64 and libxapp1:i386 side by side. It would be good if we could move it to xapps-common but this package is architecture-independent and xapp-sn-watcher needs to be compiled for each and every architecture. That is why it needs its own package. Still we have the policy violation that you are referring to in #149. Thus, libxapp-gtk3-module.so also needs to be separated from libxapp.so.1. But this is nothing too special. Other packages in Debian have done so as well in the past (libcanberra-gtk3-0 and libcanberra-gtk3-module for instance). Anyway, it is a different thing with mate-xapp-status-applet. It could well stay in xapps-common if xapps-common has got the missing dependencies (gir1.2-gdkpixbuf-2.0, gir1.2-glib-2.0, gir1.2-gtk-3.0, gir1.2-matepanelapplet-4.0, gir1.2-xapp-1.0, mate-panel and python3-setproctitle). But then mate-panel and gir1.2-matepanelapplet-4.0 would pull in other MATE packages on a system without MATE previously. That is why I think it would be good to separate mate-xapp-status-applet from xapps-common.

Do these changes imply any changes in projects that depend on libxapp.so.1 and its friends? No. I have added dependencies on the new packages libxapp-gtk3-module and xapp-sn-watcher to libxapp1. So any package with binaries linked to libxapp.so.1 automatically has also a transitive dependency on the new packages. You are free to add an explicit dependency to any package though if you still want to do so, but it's not necessary. I have also added a Suggests relation for the new package mate-xapp-status-applet and its corresponding XFCE package xfce4-xapp-status-plugin.

Speaking of dbgsym migration. I don't know the reason why it was rejected when you proposed that. Perhaps it was because of still having to support older Ubuntu versions with debhelper being older than version 10 which was introduced in Ubuntu 16.10 at first. The current packaging instructions depend on debhelper version 12 or later (I haven't touched this). This debhelper version is in Ubuntu since version 19.04. AFAIK the Linux Mint packages are still built on an Ubuntu 20.04 LTS system with debhelper 12.10. So it would be safe now to let debhelper automatically build the packages for the debug symbols. I have chosen to do this migration before splitting libxapp1 to avoid adding yet another two debug packages next to libxapp-dbg in debian/control.

And now to the license file. The only substantial difference that I am aware of is the license of libxapp/xapp-debug.c and libxapp/xapp-debug.h which is LGPL v2.1 or later. I am not a copyright expert though. Does "LGPL v2.1 or later" include LGPL v3, doesn't it? I would have thought that it's just fine if the Linux Mint developers want to redistribute it under LGPL v3 as well since the other code is distributed under LGPL v3. But what do I really know? The missing part can easily be added to debian/copyright. Just let me know if that is wanted or needed. Then I would update the corresponding commit.

Speaking of updating the commits. Once when it is decided if and what needs any changes, I would also rebase this PR to Git HEAD. It does not really make sense to rebase it every once in a while.

Fantu commented 1 year ago

I looked too fast the first time, after better review of changes I saw that are needed and I'll do in debian packages anyway, hope will be accepted also here. About libxapp1 recommends (for compatibility and to avoid PRs of changes in software that use xapp) is ok for debian packages but probably not for upstream, if I remember good mint don't have recommends enabled by default. About dbgsym if I remember good I tried to ask also when stopped to support older distro with debhelper < 12 but was still rejected, you can reask to Clem and Mtwebster. If accepted should be done in all components and if I remember good there is one mint package that install all debug packages to change. You right about the license, can be only LGPL v3 and add only the missed copyright lines in it (nokia, collabora, red hat), was because I'm used in debian where every step in NEW (for new binaries) goes through a deep d/copyright check by ftpmaster team and there mustn't be the slightest lack but here it seems here instead they don't give much importance to it.

Fantu commented 1 year ago

@uhle as I saw in debian that remove a conf file moved to another package with maintscript (I mean /etc/xdg/autostart/xapp-sn-watcher.desktop) cause it to be removed from new packages in many cases, keep the warning of obsolete file is better that risk to don't have it: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=886389#50

edit: another thing, /etc/xdg/autostart/xapp-sn-watcher.desktop probably should stay in xapps-common, I'll probably ask advice about it on debian-devel irc

uhle commented 1 year ago

I don't like to comment on dbgsym migration any longer. It's not worth it. Although I don't see the benefit of not letting debhelper automatically do its job, I am going to revert that change and add the other two debug packages to debian/control instead if the Linux Mint developers want it that way. Just let me know.

For the dependencies on the new packages, it has to be Recommends and not Depends to prevent a circular dependency (e.g. xapp-sn-watcher -> libxapp1 -> [Recommends] xapp-sn-watcher) which otherwise would make it impossible for APT to decide on the correct order while installing the packages. And for the Suggests relation to mate-xapp-status-applet, it has to be that way so that mate-xapp-status-applet is not automatically installed on a non-MATE system (which was the reason for the split). Anyway, I cannot confirm that Recommends relations are ignored, at least on LMDE5 they aren't. I have such a working system and it's just working fine.

Regarding the XDG autostart file xapp-sn-watcher.desktop, I don't think it is a good idea to put it in xapps-common because in an unlikely situation like that (having only installed xapps-common for whatever reason), the XDG autostart file fails to start xapp-sn-watcher and then you would probably have messages in .xsession-errors. However, it is always a good thing to keep things together which means having the autostart file next to xapp-sn-watcher in one package. By the way, this is exactly what you already have done in Debian.

Anyway, I have not experienced any problems with rm_conffile but of course I can remove the maintainer script to avoid any unnecessary problems. Thanks for the hint!

Fantu commented 1 year ago

I hope that all changes including dbgsym will be accepted but we need to wait @clefebvre and @mtwebster reply to know (and hope that the evaluation/application of this PR is not deferred to the next version, after half a year) another small change I suggest to add is to rename 80xapp-gtk3-module.sh to 80xapp-gtk3-module (not in debian package) like another other /etc/X11/Xsession.d/ file About recommends to avoid circular dep you are right, I just specified a problem seen in the past and the important fact that upstream changes to packaging must fully support Mint (both the based on debian and the other based on ubuntu) including all version they still support (FWIK now is lmde5 based on debian 11 and mint 21 based on ubuntu 22.04) about the conf file with doubt to keep in -common I wrote wrong, was etc/X11/Xsession.d/80xapp-gtk3-module for multi-arch support of the package, I didn't remember if can still works with conffile, I need to ask on multiarch team on irc about rm_conffile issue in file moved is simple to check, after first upgrade that will use rm_conffile (based on the version) simple check if file still exist or is missed, anyway if I remember good a dpkg maintaner wrote me that should not be used on file moved (with same name and path) to another package

uhle commented 1 year ago

Of course I can change the file name 80xapp-gtk3-module.sh to 80xapp-gtk3-module. But this file was not in xapps-common like in Debian. It was in libxapp1 before the split. I think that this file should not be moved to xapps-common for the very same reason as the XDG autostart file xapp-sn-watcher.desktop. If it would have been in xapps-common and you would just install xapps-common, you would get unnecessary error messages like that in .xsession-errors:

Gtk-Message: 18:42:46.040: Failed to load module "xapp-gtk3-module"

Anyway, it is no problem to have this file in a package that is marked Multi-Arch: same because its content is the same for each architecture. It would only be a problem if the content would differ, and that is not the case here.

Fantu commented 1 year ago

I already applied major of your packaging changes and in debian: https://packages.debian.org/source/experimental/xapp

uhle commented 1 year ago

Looks good at a first glance.

clefebvre commented 1 year ago

We don't need a package for the MATE applet and we don't need dependencies on mate-applet. It doesn't matter if the applet isn't functional without the panel. Nobody's installing it or expecting it to work without MATE.

The argument that cinnamon/xfce users don't need the applet is more relevant though. But it's not a reason enough to complexify the packaging and have yet another package. Only disk space is at stake here and it's minimal. We could use the same argument for translations. A majority of users only use English, so we could make an -l10n package.. but then German users might argue they don't need non-German translations, so we'd need an -fr, -de, -es etc.... packages. Disk space optimization, at that scale, doesn't matter.

For these reasons I disagree with a dependency on mate-panel or the creation of a dedicated mate package.

clefebvre commented 1 year ago

The following commits look good to me:

Fantu commented 1 year ago

Hi, the package changes of this PR have already been applied to the debian packages for debian 12 and on Sunday I did the last remaining tests to upgrade debian 11->debian 12 without encountering any problems. regarding the mate package if upstream won't be done I will also modify it on the debian packages to try to keep them the same however, if there will be changes that would cause the packages to go against the policies again, they will not be made in debian

clefebvre commented 1 year ago

Looks good as well. I wouldn't mind going back to the original filename also (i.e. without the .sh suffix).

Fantu commented 1 year ago
  • Install xinitrc script 80xapp-gtk3-module.sh to /etc/X11/Xsession.d

Looks good as well. I wouldn't mind going back to the original filename also (i.e. without the .sh suffix).

good, thanks, debian packaging already did it for a long time (both the path change and remove the suffix)

clefebvre commented 1 year ago

/usr/libexec/xapps/sn-watcher/xapp-sn-watcher can be moved to an arch-dependent path and remain part of libxapp1. We don't need a separate package for that. LibXapp supports backwards compatibility as much as possible, that's part of our principles so there's no need for multiple versions to be concurrently installed.

Fantu commented 1 year ago

@clefebvre even if "mint softwares" unfortunately do not increment the versions in case of ABI break, keep inside libxapp1 will be a policy issue and can't be done in debian without risk to open RC (release critical) bug when someone spot them, see for example https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1000824

clefebvre commented 1 year ago

This has nothing to do with Mint software. Any app using libxapp is supported.

Fantu commented 1 year ago

xapp as used by many apps and not only components used only by cinnamon (like muffin) should increase the need for good packaging and increase the library version in case of ABI break

in debian for now there is already timeshift that use xapp and is out of packages maintained by cinnamon team for cinnamon components in some cases we have been keeping bad things for years to have less differences with upstream packaging but with packages linked to others outside it is more difficult

clefebvre commented 1 year ago

This is continued at https://github.com/linuxmint/xapp/pull/163.