-
Notifications
You must be signed in to change notification settings - Fork 123
CPack debian packaging #3583
CPack debian packaging #3583
Conversation
I ran into following problems with CPack:
|
Thank you so much, this looks like a massive amount of work! 🚀 Please ping me when it is ready to review.
What does that mean? That an i386 package we create with cpack is uninstallable on an amd64 system? Is there already an upstream issue about that?
I don't think this is a problem. We can create an extra package that depends on all other -dbgsym packages.
No cmake flag should be added. If we want to have them in the deb, we also add them in the normal installation. About the files in the debian branch:
|
Exactly. Packages would need to be multiarch-aware wich is not supported by CPack. The upstream issue: https://gitlab.kitware.com/cmake/cmake/-/issues/21445
A machine-readable copyright file is required for for each debian package, so I don't think that LICENSE.md and THIRD-PARTY-LICENSES replaces it. We could probably generate the copyright file from the LICENSE.md and THIRD-PARTY-LICENSES files. |
Yes, I agree. It is not really a problem. And it is an upstream issue anyway, so I wouldn't do anything about it.
Did you already check for the other distributions (you want to write packages for) what they need? Maybe we can (manually) rewrite doc/THIRD-PARTY-LICENSES to the licencecheck format. Then we could use licensecheck2dep5 for generation of the dep5 license file. (See https://wiki.debian.org/CopyrightReviewTools for the various tools that Debian has.) We can also maintain doc/THIRD-PARTY-LICENSES directly in dep5 format, depending on which format other distributions need and which conversion tools they provide. It is up to you what works best and what is the easiest for you. |
Nice work, thank you for creating the PR! Great to see so much progress. 🚀 It's already looking really great. I think the best way forward will be to test the installed packages in the CI (as we already discussed). |
RPM packages for example require only a mentioning of the used licenses and a short comment explaining the licensing breakdown. I have yet to check how CPack RPM handles the
Yes good idea, I will do this. Unfortunately there appears to be no simple way to have a common format, so we would need to maintain multiple licenses/copyright files for each package format. |
scripts/packaging/copyright
Outdated
@@ -0,0 +1,556 @@ | |||
Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, great improvements!
But we really cannot maintain several copyright files, we hardly can manage one. Simply modify doc/THIRD-PARTY-LICENSES or remove doc/THIRD-PARTY-LICENSES (after updating this file) but please do not duplicate copyright information. It is too much of a burden for anyone adding something with copyright. The copyright file of the debian branch was often forgotten to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maintaining the doc/THIRD-PARTY-LICENSES in licencecheck format, like suggested previously, wouldn't really work. licensecheck
analyzes each file in the project and returns the copyright information of the file. The problem is that wildcards for files are not supported, so we would need to mention each file explicitly.
It would be probably best if we maintain the doc/THIRD-PARTY-LICENSES in dep5 format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, doc/THIRD-PARTY-LICENSES in dep5 format is perfectly fine.
@markus2330 can you please review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing job! 🥇 I cannot wait to get this finally merged!
doc/THIRD-PARTY-LICENSES
Outdated
Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ | ||
Upstream-Name: Elektra | ||
Upstream-Contact: Markus Raab <[email protected]> | ||
Source: http://www.libelektra.org/ftp/elektra/releases/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Source: http://www.libelektra.org/ftp/elektra/releases/ | |
Source: https://www.libelektra.org/ftp/elektra/releases/ |
doc/THIRD-PARTY-LICENSES
Outdated
Source: http://www.libelektra.org/ftp/elektra/releases/ | ||
|
||
Files: * | ||
Copyright: 2003-2004, 2006-2008, 2010, 2013-2014 Markus Raab <[email protected]> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to simply refer to doc/AUTHORS.md here? (Btw. doc/AUTHORS can be removed, I do not think it is used anymore).
And please feel free to add yourself also to doc/AUTHORS.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find an example where a specific file was referenced, but we could simply write "libelektra contributors". Neovim for example is doing it this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good idea. I would say we can even write
Elektra contributors (https://www.libelektra.org/developers/authors)
It is free text, so the weirdest things are there. Also links are quite common there. (I used grep "Copyright:.*/" /usr/share/doc/*/copyright
to find out.)
scripts/cmake/ElektraPackaging.cmake
Outdated
@@ -8,6 +8,7 @@ set (CPACK_PACKAGE_VERSION "${PROJECT_VERSION}") | |||
set (CPACK_DEBIAN_PACKAGE_VERSION "${PROJECT_VERSION}") | |||
set (CPACK_DEBIAN_PACKAGE_RELEASE "1") | |||
set (CPACK_DEBIAN_FILE_NAME DEB-DEFAULT) | |||
set (CPACK_DEBIAN_PACKAGE_MAINTAINER "Pino Toscano <[email protected]>") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are now the Debian package maintainer 🥇
dpkg-maintscript-helper rm_conffile /etc/bash_completion.d/kdb 0.8.11-1~ -- "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this script doing? Why is a very outdated version number here?
The bash completion is not working well (or even not at all?), if there are packaging problems with it, it is probably better to simply remove it completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the generated output of the elektra-bin.maintscript.
The purpose of it was to remove a conffile which was necessary for versions prior to 0.8.11-1.
Since 0.8.11-1 was already 5 years ago, I think these maintscripts can be removed.
DESTINATION ${TARGET_DOCUMENTATION_TEXT_FOLDER} | ||
COMPONENT libelektra4) | ||
install ( | ||
FILES METADATA.ini |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, good that this finally gets installed! Does the script scripts/kdb/mount-info work? (It should as you use TARGET_DOCUMENTATION_TEXT_FOLDER)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an issue with it. Although these files are inside the debian package (at /usr/share/doc/elektra/) they somehow doesn't get installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like dpkg somehow removes these files. Running dpkg with debug=7200 returns following output:
D000010: tarobject ti->name='./usr/share/doc/elektra/NEWS.md' mode=100644 owner=0:0 type=48(-) ti->linkname='' namenode='/usr/share/doc/elektra/NEWS.md' flags=2 instead='<none>'
D000010: filter comparing '/usr/share/doc/elektra/NEWS.md' and '/usr/share/man/*'
D000010: filter comparing '/usr/share/doc/elektra/NEWS.md' and '/usr/share/locale/*/LC_MESSAGES/*.mo'
D000010: filter comparing '/usr/share/doc/elektra/NEWS.md' and '/usr/share/doc/*'
D000010: filter removing ./usr/share/doc/elektra/NEWS.md
D000010: filter comparing '/usr/share/doc/elektra/NEWS.md' and '/usr/share/doc/*/copyright'
D000010: filter comparing '/usr/share/doc/elektra/NEWS.md' and '/usr/share/doc/*/changelog.Debian.*'
This also happens on the previous Elektra packages not built with CPack.
@@ -162,7 +162,7 @@ you up to date with the multi-language support provided by Elektra. | |||
### CMake | |||
|
|||
- Use Lua 5.4 when available. _(Mihael Pranjić)_ | |||
- <<TODO>> | |||
- We now use CPack to build modular Debian and Ubuntu packages. _(Robert Sowula)_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make a highlight out of this!
scripts/cmake/ElektraPackaging.cmake
Outdated
set (CPACK_COMPONENT_LIBELEKTRA4_DESCRIPTION "This package contains the main elektra library, and most of the core plugins") | ||
set (CPACK_DEBIAN_LIBELEKTRA4_PACKAGE_SECTION "libs") | ||
set (CPACK_DEBIAN_LIBELEKTRA4_PACKAGE_BREAKS | ||
"elektra-bin (<< 0.8.14-5~), libelektra-core4 (<< 0.8.19-1), libelektra-full4 (<< 0.8.19-1)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt that these versions make sense (0.8.19) or are relevant anymore (0.8.14). I think everywhere with 0.8.19 actually the current version should be used. References to older versions we can simply ignore, without extensive testing an upgrade would not work from these versions anyway.
I now tried creating the debs with
and also:
The debs are now directly in the build folder. Is it possible to put all of this in a subfolder? There are also many install_manifest_* files. They look quite useless, the main purpose of packages is that it is not needed to externally remember what was installed. The descriptions are very short, there should be some explanation of what Elektra is. At one place we have What happens with the files that do not belong to any package? Where do they end up? (It would be nice if I also could install them as a deb, e.g. also the website, which I sometimes install locally). Overall amazing work, looking forward to see this as the new way of packaging. It is much easier and faster than the old way. 💖 |
This one is fine. It only indicates that a package has no explicitly set dependencies other than the automatically discovered dependencies.
This is also fine. We only use symbol versioning in src/libs and for a few tools. This warning only indicates that symbolic versioning is not used and shlibs are not generated for this package. The generated shlibs are also identical to the previous debian packages generated with gpb.
I will look into this.
They actually are useless and can be safely removed.
The package description of each package (e.g. CPACK_COMPONENT_LIBELEKTRA4_DESCRIPTION) is appended to CPACK_PACKAGE_DESCRIPTION_SUMMARY. The descriptions should be therefore identical to the previous packages.
Thanks, I will fix this.
Files that do not belong to a component will not be packaged. The easiest solution would be to simply add a default component where each file will be installed that is not part of a package. |
The debs are now in the subfolder "packages".
I added the package "elektra-misc" which contains all files that are not part of any other package. I overlooked that the changelog file was missing. CPack DEB has unfortunatly no feature to automatically update the debian/changelog. I have now added the changelog file from the debian branch. |
All files that are not part of a released package are packaged into the elektra-misc package
d9f349d
to
959ac98
Compare
76e017f
to
71bc1ce
Compare
Maybe you find some way to not show these warnings. There are so many that we would not see relevant warnings anymore.
Maybe directly after building?
Thank you, great! 🎆
Perfect! Is it possible to generate a warning if someone installs something and has no
Yes, and dch can be called during the continuous releases. Amazing work! For me it looks like that we can already generate the Debian packages with cpack in the next release! |
install_manifest_elektra-misc.txt contains some indications of left-overs for other packages:
Probably fine to have them in misc is:
|
|
Definitely. I also look forward to other packages 🚀 |
Unfortunately I couldn't find a way to supress only these warnings. For our use in Jenkins we could filter out the warnings for the shlbis generation with e.g. awk.
The
This actually is possible, but we would loose the option to exclude packages from being generated (which IMHO is not a big problem). Currently we are setting
We didn't package the jni plugin nor the jna bindings previously, so I though they shouldn't be packaged (see scripts/packaging/package.sh for currently included bindings/packages). But I can of course create packages for the missing plugins/bindings. Since you also mentioned yamlcpp, what plugins and bindings should actually be packaged now? |
Let it be for now, maybe we find later a solution. Not urgent or critical.
Again, not urgent or critical. If cmake makes them by itself, it is best if we do not touch them.
Yes, sounds great! In this error message we can write that they need to choose in which package it should be. This sounds much better than simply throwing everything into the misc package.
It would be nice, but if we get maven packages anyway it is not highest priority. I see that other Java Debian packages have a symlink from
Also here, not urgent as the plugin has "unfinished concept discouraged" status. I think it should be in its own package, as it has a dependency no other part of Elektra has. @sanssecours what is missing that you would recommend the plugin more? |
Regarding throwing errors if no component is provided during install. This is in our case apparantly not possible. Somehow a "ghost" package with the CMAKE_INSTALL_DEFAULT_COMPONENT_NAME name is generated. It has an empty install_manifest and no content. I checked everything multiple times but couldn't figure out what was causing it, so I had to remove the component checking and set CMAKE_INSTALL_DEFAULT_COMPONENT_NAME to elektra-misc.
I added the java packages and will change them to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 🏅
Are any further changes planned within this PR or is it ready to merge?
This PR is ready to merge. I will create a second PR for the RPM packaging as soon as I am done with it. |
Thank you, I look forward to the next PR! 😃 |
set ( | ||
CMAKE_INSTALL_DEFAULT_COMPONENT_NAME | ||
"elektra-misc" | ||
CACHE STRING "Default component name" FORCE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this really need to be in the cache? If yes, put it to scripts/cmake/ElektraCache.cmake and use mark_as_advanced as it is not something a normal user wants to change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently a previous configuration issue led me to cache this variable. This is definetly not needed and a mistake from my side. I will fix this in the upcoming PR, since this one is already merged, if this is fine for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, of course it can be fixed with the next PR. But it would be good if it will be fixed before the release as cmake cache variables are user-facing (to maintainers) and should be documented and so on. So better to never introduce this cache variable to any release. Anyway: would be great to have RPM+DEB for next release. Is it your plan to do so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was currently working on extending the release automation (ABI tests, external test suites etc) but I can of course also create RPM packages for this release. With our current CPack setup this probably shouldn't be that much work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The primary reasons is probably that I am still the maintainer of the plugin 😊. I do not think that I will add features that are still missing, such as proper support for
|
Adds creating of modular debian packages with CPack.
Closes #3558
Basics
These points need to be fulfilled for every PR:
(added as entry in
doc/news/_preparation_next_release.md
whichcontains
_(my name)_
)Please always add something to the release notes.
(first line should have
module: short statement
syntax)close #X
, are in the commit messages.doc/news/_preparation_next_release.md
scripts/dev/reformat-all
If you have any troubles fulfilling these criteria, please write
about the trouble as comment in the PR. We will help you.
But we cannot accept PRs that do not fulfill the basics.
Checklist
Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.
(not in the PR description)
Review
Reviewers will usually check the following:
Labels
If you are already Elektra developer:
say that everything is ready to be merged.