libelektra icon indicating copy to clipboard operation
libelektra copied to clipboard

Arch package: unnecessary deps

Open eiskasten opened this issue 3 years ago • 13 comments

Steps to Reproduce the Problem

Using the package from the Arch User Repository throws an compilation error as of now. This can be reproduced with

git clone https://aur.archlinux.org/elektra.git
cd elektra
makepkg

Expected Result

A package in the directory where makepkg was called.

Actual Result

A compilation error:

[ 27%] Building CXX object src/tools/kdb/CMakeFiles/kdb-objects.dir/command.cpp.o
/home/richi/.cache/yay/elektra/src/libelektra-0.9.3/src/tools/kdb/cmdline.cpp: In constructor 'Cmdline::Cmdline(int, char**, Command*)':
/home/richi/.cache/yay/elektra/src/libelektra-0.9.3/src/tools/kdb/cmdline.cpp:39:99: error: 'numeric_limits' was not declared in this scope
   39 |   debug (), force (), load (), humanReadable (), help (), interactive (), minDepth (0), maxDepth (numeric_limits<int>::max ()),
      |                                                                                                   ^~~~~~~~~~~~~~
/home/richi/.cache/yay/elektra/src/libelektra-0.9.3/src/tools/kdb/cmdline.cpp:39:114: error: expected primary-expression before 'int'
   39 |   debug (), force (), load (), humanReadable (), help (), interactive (), minDepth (0), maxDepth (numeric_limits<int>::max ()),
      |                                                                                                                  ^~~
/home/richi/.cache/yay/elektra/src/libelektra-0.9.3/src/tools/kdb/cmdline.cpp:429:59: error: expected primary-expression before 'int'
  429 |                                 maxDepth = numeric_limits<int>::max ();
      |

System Information

  • Elektra: 0.9.3
  • Operating System: Arch Linux

Further Information

The package is flagged out-of-date in the Arch User Repository.

eiskasten avatar Mar 30 '22 15:03 eiskasten

I would like to work on this issue. As the elektra-git package works, my suggestion is to try to update the version of this package first.

eiskasten avatar Mar 30 '22 15:03 eiskasten

I also took a look at it since we are working on this together and I am the one who flagged the AUR package. The easiest fix is to just bump the package version, as can be seen here:

diff --git a/PKGBUILD b/PKGBUILD
index 1701538..c2b60eb 100644
--- a/PKGBUILD
+++ b/PKGBUILD
@@ -2,7 +2,7 @@
 # Maintainer: Stefan Husmann <[email protected]>
 
 pkgname=elektra
-pkgver=0.9.3
+pkgver=0.9.9
 pkgrel=1
 pkgdesc="A universal hierarchical configuration store"
 url="https://www.libelektra.org"
@@ -15,7 +15,7 @@ optdepends=('ruby: for ruby bindings'
 	    'lua: for lua bindings')
 makedepends=('docbook-xsl' 'cmake' 'doxygen' 'git')
 source=("$pkgname-$pkgver.tar.gz::https://github.com/ElektraInitiative/libelektra/archive/$pkgver.tar.gz")
-sha256sums=('b54e17cdf0065e7c2682f7c72fadb144ca54c137ddef1f078d60cf8e8542008a')
+sha256sums=('12e94a58d1a707f8cf359d888c19c8f4e059b3bacc0c35c86dfc309ebd0a385b')
 
 build() {
   cd lib$pkgname-$pkgver

This does not result in a compile error (at least not on my machine), that that seems to be fixed by a version bump. But I also noticed that it is probably a good idea to also rework the package, as it does not compile some plugins/bindings (like the java one), but also has some problems that are reported by namcap(1):

$ namcap elektra-0.9.9-1-x86_64.pkg.tar.zst 
elektra E: ELF file ('usr/share/elektra/tool_data/hub-zeromq/hub-zeromq') outside of a valid path.
elektra W: Directory (usr/share/elektra/test_data/gen/gen/CMakeFiles) is empty
elektra W: Package was 50% docs by size; maybe you should split out a docs package
elektra E: Dependency yaml-cpp detected and not included (libraries ['usr/lib/libyaml-cpp.so.0.7'] needed in files ['usr/lib/libelektra-yamlcpp.so'])
elektra E: Dependency gpgme detected and not included (libraries ['usr/lib/libgpgme.so.11'] needed in files ['usr/lib/libelektra-gpgme.so'])
elektra W: Dependency lua detected but optional (libraries ['usr/lib/liblua.so.5.4'] needed in files ['usr/lib/lua/5.2/kdb.so'])
elektra W: Dependency qt5-base included but already satisfied
elektra W: Dependency curl included but already satisfied

bhampl avatar Apr 06 '22 07:04 bhampl

I have now assigned this issue 1p. This applies, if you just make the packages work by updating the versions. If you also fix the warnings (Note: not all of them can be fixed), and maybe rework the package (e.g separate packages for docs, experimental plugins, etc.) more points would be awarded. For some ideas, on how the package could be split look at the Debian/Fedora packages.

kodebach avatar Apr 06 '22 21:04 kodebach

I already managed to update the versions and separate between the docs and the rest in the same PKGBUILD file. My current plan is to outsource all plugins and experimental plugins as the grade of separation seems a little bit too fine in the Fedora repository to my opinion (Not even kubernetes nor QEMU with all its architectures provide as many packages in Arch).

So my question is how can I run something such as make install ... with keeping in mind to only deploy the elektra plugins, or experimental plugins without the rest of elektra and without cleaning and rebuilding targets. The plan is to run cmake ... and make once and then repeatedly run make install ... but I do not know how.

eiskasten avatar May 25 '22 17:05 eiskasten

I don't think the current CMake setup allows a partial install (wouldn't really make sense). You could choose a reduced set of plugins when configuring CMake and then do that repeatedly. You could also delete files after running make install, but that isn't really efficient, if you want the packages to build from source.

For packages that build from source I see two options:

  1. create multiple packages that install different subsets of Elektra and mark them as conflicting (i.e. only one of the packages can be installed on any system)
  2. create a single package and add some variable at the top of PKGBUILD file. The variables should have reasonable defaults and advanced users could use them to e.g. configure the set of plugins.

However, I actually think pre-build binary packages might a better option. There you could re-use the existing CPack setup. You can use -DCPACK_ARCHIVE_COMPONENT_INSTALL=ON when configuring CMake. Then you should get separate archive files for each of the components used by RPM/DEB packages, when you run make package. (Note: I could only get this working, by using -DBUILD_FULL=ON -DBUILD_SHARED=ON).

There was some work to automatically create PKGBUILD files with CPack (same as generating RPM/DEB packages), but it seems that work was abandoned: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4185

kodebach avatar May 25 '22 18:05 kodebach

Thank you. I think that the first option is the best one. This would also prevent a failure of a plugin build which does not require any experimental plugins which might not build or work. However, providing binaries would be the last option to my opinion as the AUR is meant to be source based as far as possible.

As for now, the fixed package versions can be found at https://aur.archlinux.org/packages/libelektra and https://aur.archlinux.org/packages/libelektra-docs.

eiskasten avatar May 25 '22 19:05 eiskasten

If there is no possibility to create several packages out of one source package with one PKGBUILD, it is better to leave a single package. Having two packages is twice the maintenance work.

There seem to be a few unnecessary dependencies: we don't use boost and docbook-xsl anymore.

Looking forward to see another update after the next release (with fixed deps).

markus2330 avatar May 27 '22 09:05 markus2330

It is possible and it is already done this way. The web interfaces just generates separate pages for each package which results into multiple links.

Thank you for mention them, they are removed now.

I wonder how a PR for this issue should look like since the PKGBUILD is committed in the AUR and I prefer not committing it here since it would violate the single source of truth principle.

eiskasten avatar Jun 01 '22 10:06 eiskasten

I wonder how a PR for this issue should look like since the PKGBUILD is committed in the AUR

I don't think a PR is necessary, unless you need to e.g. change our CMake config. Since the AUR also provides the history and we can see what you changed, it should be good enough to commit your changes there.

Regarding multiple packages: If the PKGBUILD has to be manually updated, every time the component definitions from CMake change, I don't think splitting the package would make sense.

I also just noticed a few minor things regarding dependencies: There is an optional dependency on ruby, but both the plugin and the binding are always excluded. Either we should include ruby when available, or we should not list the optional dependency. We should also decide whether the dependencies for the bindings are required (like Lua, Pyhton, Java) or optional (like Ruby).

Also the description for the xerces-c dependency is wrong.

kodebach avatar Jun 01 '22 10:06 kodebach

I wonder how a PR for this issue should look like since the PKGBUILD is committed in the AUR

I don't think a PR is necessary, unless you need to e.g. change our CMake config. Since the AUR also provides the history and we can see what you changed, it should be good enough to commit your changes there.

I did not modify anything in this repository that is why I ask and because I got points deducted because of this.

Regarding multiple packages: If the PKGBUILD has to be manually updated, every time the component definitions from CMake change, I don't think splitting the package would make sense.

This really depends on how the packages are split. As of now it should be fine since it only separates the documentation (which is always in the doc directory after the build from the actual libelektra. But when splitting up into different sets of plugins you are totally right.

I also just noticed a few minor things regarding dependencies: There is an optional dependency on ruby, but both the plugin and the binding are always excluded. Either we should include ruby when available, or we should not list the optional dependency. We should also decide whether the dependencies for the bindings are required (like Lua, Pyhton, Java) or optional (like Ruby).

Also the description for the xerces-c dependency is wrong.

Thank you, I will have a look on this.

eiskasten avatar Jun 01 '22 10:06 eiskasten

I got points deducted because of this.

I think it's just weird formatting in the feedback. The deduction was because of the missing PR for #4001 (you and @bhampl are assigned to this issue).

kodebach avatar Jun 01 '22 11:06 kodebach

Yes, it was only confusing because of the formatting. The contrary is true: the group got plus points because of this excellent package update. Great that you found out how to split the package.

I wonder why it is ../LICENSE.md for one package and ../../LICENSE.md for the other package?

The whole "optdepends" can be removed, the python2 bindings+plugins are already removed, we now have only python3 bindings+plugins. (Only the deprecated pythongen remained using python2 but this is not worth packaging. We will remove it before 1.0, see #2987)

markus2330 avatar Jun 04 '22 14:06 markus2330

As the release happened it would be a good time to upload a new package fixing some unnecessary deps.

markus2330 avatar Jul 14 '22 13:07 markus2330

I have bumped the version to 0.9.14 and removed the optional dependencies. I think this can be closed.

eiskasten avatar May 14 '23 09:05 eiskasten

@Eiskasten Thank you!

markus2330 avatar May 14 '23 09:05 markus2330