PKGBUILD review request: opendbviewer

liums · 2017-09-27 13:16:16

Hi all,

I wrote a PKGBUILD for https://github.com/Jet1oeil/opendbviewer ( a simple graphical explorer for databases ) ; it works but since that's my first PKGBUILD I prefer post it here before AUR. Just not sure about the icon, I suppose it should be copied without my addition in prepare() but it wasn't.

The file :

# Maintainer: liums <liums at openaliasbox.org>
pkgname=opendbviewer-git
pkgver=v1.0.2.r14.g6bcbd8d
pkgrel=1
pkgdesc="A simple database explorer for SQLITE, MySQL, PostgreSQL."
arch=('x86_64' 'i686')
url="https://github.com/Jet1oeil/opendbviewer"
license=('GPL3')
depends=('qt5-base')
makedepends=('cmake' 'git' 'qt5-tools')
provides=('opendbviewer')
conflicts=('opendbviewer' 'opendbviewer-git')
source=("git://github.com/Jet1oeil/opendbviewer.git")
_gitname='opendbviewer'
md5sums=('SKIP')

pkgver() {
	cd "$_gitname"
	git describe --long --tags | sed 's/\([^-]*-g\)/r\1/;s/-/./g'
}

prepare() {
	cd "$_gitname"
	./build.sh regen
	sed -i -e 's/\/local//' CMakeCache.txt
	sed -i -e 's/share\/\${APPLICATION_PACKAGE_NAME}\/doc/share\/doc\/\${APPLICATION_PACKAGE_NAME}/' CMakeLists.txt
}

build() {
	cd "$_gitname"
	make
}

package() {
	cd "$_gitname"
	make DESTDIR="$pkgdir/" install
	mkdir "$pkgdir/usr/share/icons"
	cp data/logos/opendbviewer.png "$pkgdir/usr/share/icons/"
}

Then

> pacman -Ql opendbviewer-git 
opendbviewer-git /usr/
opendbviewer-git /usr/bin/
opendbviewer-git /usr/bin/opendbviewer
opendbviewer-git /usr/share/
opendbviewer-git /usr/share/applications/
opendbviewer-git /usr/share/applications/opendbviewer.desktop
opendbviewer-git /usr/share/doc/
opendbviewer-git /usr/share/doc/opendbviewer/
opendbviewer-git /usr/share/doc/opendbviewer/ChangeLog
opendbviewer-git /usr/share/doc/opendbviewer/README
opendbviewer-git /usr/share/icons/
opendbviewer-git /usr/share/icons/opendbviewer.png

Is it ok ?
Thank for your check !

Trilby · 2017-09-27 13:24:40

Looks fine to me, but why is cmake listed as a makedepends if you only use make?

liums · 2017-09-27 13:41:30

As far as I know ( i'm not the dev of this software ), cmake generate a Makefile for gnu make

Trilby · 2017-09-27 14:00:32

That is the purpose of cmake, yes. But my point was that your PKGBUILD does not run cmake anywhere. So either the software does not use cmake, or it was already run upstream and a Makefile is distributed.

In any case, if cmake is not used by the build process, it is not build depdency.

EDIT: oops, unless it's in that build.sh file, ~~standby...~~ Yes, cmake is called by build.sh, but in that case those lines should probably be in the build function not a prepare function.

Last edited by Trilby (2017-09-27 14:02:57)

liums · 2017-09-27 14:20:41

ok, i've put it here cause I've read in the wiki that the modifications to the code or patch should go in prepare(), and the 'sed' lines correct the installation path ( but not in the code in fact )

but if that's part of the build I'll change it

progandy · 2017-09-27 15:58:04

Patching CMakeLists.txt should work before running your build.sh. Patching CMakeCache is ugly, it would be better to fix up the CMakeLists or set the needed variabes for build.sh so that cmake can generate the correct values.

By the way, you shoudn't add the package name to the conflicts array.

pkgname=opendbviewer-git
conflicts=('opendbviewer' 'opendbviewer-git')

Last edited by progandy (2017-09-27 16:10:18)

eschwartz · 2017-09-27 16:07:28

I don't see what purpose there is in running that `build.sh regen` script. Just run cmake -DWhatever yourself.

In fact, that embedded cmake command is wrong. It does nothing other than 1) hardcode the "Eclipse CDT4 - Unix Makefiles" generator and 2) run a bunch of useless code beforehand to clean the old build resources for the in-tree builds that CMake famously disapproves of.

You don't need to do inappropriate things to the CmakeCache.txt if you just use the highly standard -DCMAKE_INSTALL_PREFIX=/usr as an option to `cmake` itself rather than relying on that terrible build.sh wrapper script.

Last edited by eschwartz (2017-09-27 16:09:17)

liums · 2017-09-27 18:42:44

Thank for your interest !

@propagandy I don't know nothing about cmake; here I just don't found any reference to "/usr/local" in the folder before I ran build.sh
and thank for the conflicts, I was not sure I add it "just in case"

@Eschwartz ok, the use of build.sh was the procedure the dev wrote in the README

So this work but I still use sed for the doc path ; and the icons still be copied manually.

# Maintainer: liums <liums at openaliasbox.org>
pkgname=opendbviewer-git
pkgver=v1.0.2.r14.g6bcbd8d
pkgrel=1
pkgdesc="A simple database explorer for SQLITE, MySQL, PostgreSQL."
arch=('any')
url="https://github.com/Jet1oeil/opendbviewer"
license=('GPL3')
depends=('qt5-base')
makedepends=('cmake' 'git' 'qt5-tools')
provides=('opendbviewer')
conflicts=('opendbviewer')
source=("git://github.com/Jet1oeil/opendbviewer.git")
_gitname='opendbviewer'
md5sums=('SKIP')

pkgver() {
	cd "$_gitname"
	git describe --long --tags | sed 's/\([^-]*-g\)/r\1/;s/-/./g'
}

prepare() {
	cd "$_gitname"
	sed -i -e 's/share\/\${APPLICATION_PACKAGE_NAME}\/doc/share\/doc\/\${APPLICATION_PACKAGE_NAME}/' CMakeLists.txt
}

build() {
	cd "$_gitname"
	cmake -DCMAKE_INSTALL_PREFIX=/usr/
	make
}

package() {
	cd "$_gitname"
	make DESTDIR="$pkgdir/" install
	mkdir "$pkgdir/usr/share/icons"
	cp data/logos/opendbviewer.png "$pkgdir/usr/share/icons/"
}

progandy · 2017-09-27 20:23:44

liums wrote:

Thank for your interest !
@propagandy I don't know nothing about cmake; here I just don't found any reference to "/usr/local" in the folder before I ran build.sh
and thank for the conflicts, I was not sure I add it "just in case"

No problem. Everyone has to start learning somewhere

So this work but I still use sed for the doc path ; and the icons still be copied manually.

That is all good, since there is no other way to do it. Technically you could also move the doc files during package() instead of patching the cmake file, but I don't think that would be better.

eschwartz · 2017-09-27 23:23:40

liums wrote:

@Eschwartz ok, the use of build.sh was the procedure the dev wrote in the README
So this work but I still use sed for the doc path ; and the icons still be copied manually.

And I can certainly understand why you would by default assume the dev has the right directions. But as I said, the dev has the wrong directions -- which is somewhat of a weird situation!

Some other notes: It is traditional when using CMake to do:

mkdir -p "$_gitname/build"
cd "$_gitname/build"

cmake -DCMAKE_INSTALL_PREFIX=/usr ../

In order to build in a special build directory (a.k.a. out-of-tree builds) and passing the source directory via ../

Note that icons usually get installed to either /usr/share/pixmaps/ or to /usr/share/icons/hicolor/256x256/apps/ whereas /usr/share/icons/ itself doesn't do anything AFAIK.
You will also probably want to use `install -Dm644 sourcefile /path/to/destination` rather than mkdir -p followed by cp.

Using sed for the doc path is okay, but you may want to submit a Pull Request on Github to fix it, and also to install the icon so downstream packagers don't have to.

Last edited by eschwartz (2017-09-28 00:24:01)

liums · 2017-09-28 15:05:38

ok, I changed it ; I guess that's good now ?

# Maintainer: liums <liums at openaliasbox.org>
pkgname=opendbviewer-git
pkgver=v1.0.2.r14.g6bcbd8d
pkgrel=1
pkgdesc="A simple database explorer for SQLITE, MySQL, PostgreSQL."
arch=('any')
url="https://github.com/Jet1oeil/opendbviewer"
license=('GPL3')
depends=('qt5-base')
makedepends=('cmake' 'git' 'qt5-tools')
provides=('opendbviewer')
conflicts=('opendbviewer')
source=("git://github.com/Jet1oeil/opendbviewer.git")
_gitname='opendbviewer'
md5sums=('SKIP')

pkgver() {
	cd "$_gitname"
	git describe --long --tags | sed 's/\([^-]*-g\)/r\1/;s/-/./g'
}

prepare() {
	cd "$_gitname"
	sed -i -e 's/share\/\${APPLICATION_PACKAGE_NAME}\/doc/share\/doc\/\${APPLICATION_PACKAGE_NAME}/' CMakeLists.txt
}

build() {
	mkdir -p "$_gitname/build"
	cd "$_gitname/build"
	cmake -DCMAKE_INSTALL_PREFIX=/usr ../
	make
}

package() {
	cd "$_gitname"
	make -C "build/" DESTDIR="$pkgdir/" install
	install -Dm644 data/logos/opendbviewer.png "$pkgdir/usr/share/pixmaps/opendbviewer.png"
}

Thank you !

Trilby · 2017-09-28 15:08:26

It's looking good, but it seems somewhere along the line you changed the 'arch' variable to 'any'. That is wrong. It was correct at the start. Compiled packages cannot be for arch=any, that is for scripts or data-only packages (or anything that doesn't get compiled to machine code).

liums · 2017-09-28 16:36:04

ok yes I see

Arch Linux

#1 2017-09-27 13:16:16

PKGBUILD review request: opendbviewer

#2 2017-09-27 13:24:40

Re: PKGBUILD review request: opendbviewer

#3 2017-09-27 13:41:30

Re: PKGBUILD review request: opendbviewer

#4 2017-09-27 14:00:32

Re: PKGBUILD review request: opendbviewer

#5 2017-09-27 14:20:41

Re: PKGBUILD review request: opendbviewer

#6 2017-09-27 15:58:04

Re: PKGBUILD review request: opendbviewer

#7 2017-09-27 16:07:28

Re: PKGBUILD review request: opendbviewer

#8 2017-09-27 18:42:44

Re: PKGBUILD review request: opendbviewer

#9 2017-09-27 20:23:44

Re: PKGBUILD review request: opendbviewer

#10 2017-09-27 23:23:40

Re: PKGBUILD review request: opendbviewer

#11 2017-09-28 15:05:38

Re: PKGBUILD review request: opendbviewer

#12 2017-09-28 15:08:26

Re: PKGBUILD review request: opendbviewer

#13 2017-09-28 16:36:04

Re: PKGBUILD review request: opendbviewer

Board footer