Request review of PKGBUILD

pest · 2019-07-22 00:04:51

Hi all! im getting my first PKGBUILD ready and would like some feed back
the github is: https://github.com/pppest/devault-pkgbu … ter/v1.0.5
and the PKGBUILD (has been edited to reflect the suggested changes) looks like this:

# Maintainer: Pest <ppest@protonmail.com>
pkgname=devault
pkgver=1.0.5
pkgrel=1
pkgdesc="Desktop wallet for the dvt blockchain"
arch=('x86_64')
license=('MIT')
url="https://github.com/devaultcrypto/"
source=(${pkgname}-${pkgver}::\
https://github.com/devaultcrypto/devault/archive/v${pkgver}.tar.gz)
depends=('boost-libs' 'qt5-base' 'qrencode' 'zeromq' 'miniupnpc' 'hicolor-icon-theme' 'libevent')
makedepends=('python' 'boost')
md5sums=('9ca9b2a3b0e0aeafdb5f974c2004ccac')

prepare() {
  cd "${srcdir}/${pkgname}-${pkgver}"
  ./autogen.sh
}

build() {
  cd "${srcdir}/${pkgname}-${pkgver}"
  ./configure --prefix=/usr/
  make
}

package() {
  cd "${srcdir}/${pkgname}-${pkgver}"
  make DESTDIR="${pkgdir}/" install
  install -D -m644 \
   contrib/debian/devault-qt.desktop \
   "${pkgdir}/usr/share/applications/devault-qt.desktop"
  install -D -m644 \
   share/pixmaps/devault-128.png \
   "${pkgdir}/usr/share/icons/hicolor/128x128/apps/devault-128.png"
  install -D -m644 COPYING  "${pkgdir}/usr/share/licenses/${pkgname}/LICENSE"
}

Last edited by pest (2019-07-22 18:00:52)

jasonwryan · 2019-07-22 00:13:45

Please edit your post and use code tags when pasting to the boards: https://wiki.archlinux.org/index.php/Co … s_and_code

pest · 2019-07-22 00:27:11

thanks for your reply. it's done.

Trilby · 2019-07-22 00:57:07

Why are you including seperate desktop, png, and license files? These are all provided by upstream. Use theirs (definitely use their license).

It also looks like this uses autotools, so you should not need to do that sed hack to set the prefix. Just provide the parameter to configure.

EDIT: I just saw your "License". That's not a license at all, it's a README file, and by distributing it as the license for the code, you are violating the license you received it under. Don't do that.

Last edited by Trilby (2019-07-22 01:01:41)

Scimmia · 2019-07-22 03:21:21

source=("https://github.com/devaultcrypto/devault/archive/v1.0.5.tar.gz"

That's a problem. First, if will create a file called "v1.0.5.tar.gz" in $SRCDEST, which could easily conflict with other packages; second, you're hardcoding the version number, so you have to change it in two places when you update the PKGBUILD.

Why is git needed at build time?

Mix of tabs and spaces for indenting. Pick a style and stick with it.

Why use install sometimes and cp others? Again, pick a style.

libevent and libsodium are needed at build time but not at runtime? Seems unlikely.

Last edited by Scimmia (2019-07-22 03:24:00)

pest · 2019-07-22 04:21:55

Thank you for your replies! They have been very instructive.
I have update the code above to reflect the changes I have made based on your comments.
All except the file name problem (if will create a file called "v1.0.5.tar.gz" in $SRCDEST, which could easily conflict with other packages), which I can't find a solution to as it seems it depends on the file pulled.

respiranto · 2019-07-22 04:42:58

In order to rename the source, prepend the source url with `<new_filename>::', e.g. `${pkgname}-${pkgver}::'.

Further, I'd suggest some stylistic changements. It is encouraged to keep lines not longer than 80 characters. Also, the indentation is wrong in the first line of build(). I do not know whether the spaces before `}' are intentional. Being pedantic, I'd avoid the double spacing in the second last line. For me, newlines around the functions would ease readability.

Finally, I'd either use `install' for everything or nothing. Note also it's `-D' option which will create any non-existing directories in the target path.

Lone_Wolf · 2019-07-22 11:55:07

./autogen.sh

This should be in a prepare() function.
Also archlinux autotools versions are often much newer then those supported by upstream,
In many cases we have to use autoreconf --force to get autotools working in archlinux packages.

pest · 2019-07-22 16:30:28

thank you all for your help!
I have tried to implements best as I could.
except the autoreconf --force. dont feel it is necessary. but you can correct me. i have looked in many PKGBUILDs and do not see it in use.

Last edited by pest (2019-07-22 16:34:10)

Trilby · 2019-07-22 16:38:32

Looking a lot better. The package function needs a little touch up. At *very* least, you can get rid of that install command making all those paths as you are now properly using the -D flag for the actuall file installations.

You could also shorten all those installs by removing the redundant ${srcdir}/${pkgname}-${pkgver}/ from the start of the source paths.

But I'd be quite surprised if *any* of this was necessary. This has an autotools generated Makefile, `make install` *should* put most if not all of those files in place already. I'm testing that now (edit: scratch that. This is way to big to reasonably build on my hardware).

Last edited by Trilby (2019-07-22 16:50:04)

respiranto · 2019-07-22 16:56:38

Using `install -D', you no longer need the `install -d' command. In the $depends array, you do not need the backslash.

Further, I'd add indentation after linebreaks being part of a logical line (e.g., after `\'). This does not work for the source string, which I'd suggest to keep in one line. This is purely stylistic though.

pest · 2019-07-22 17:33:06

thanks you!

about the install -d line, then I tried skipping it but then i get errors that the directories don't exist, so I put them back using mkdir if it makes it more clear?

Last edited by pest (2019-07-22 17:40:13)

pest · 2019-07-22 17:34:34

one question, I use the -j4 flag for make, should I leave it in or take it out?
EDIT: I made a mistake and the ${srcdir}/${pkgname}-${pkgver} prefix could be taken out

Last edited by pest (2019-07-22 17:42:10)

Trilby · 2019-07-22 17:35:32

pest wrote:

I also tried not using {srcdir}/${pkgname}-${pkgver} when copying but then i get errors that the files doesn't exist so i chose to keep them.

You need to remove the subsequent slash as well for relative path names. E.g.:

install -Dm644 contrib/debian/devault-qt.desktop "${pkgdir}/usr/share/applications/"

pest wrote:

one question, I use the -j4 flag for make, should I leave it in or take it out?

Remove it. That should be in your makepkg.conf and you definitely shouldn't override other user's settings. That might explain why the build on my machine failed so spectacularly forcing me to hard poweroff.

Last edited by Trilby (2019-07-22 17:37:57)

respiranto · 2019-07-22 17:39:19

Apparently, you need to either give the full path of the target (e.g. a/b/c instead of a/b/) or use the -t flag in order for -D to work.

make's -j flag should be left to the user to customize (makepkg.conf - MAKEFLAGS), unless it does only compile with one specific value (sometimes -j1 is required).

Trilby · 2019-07-22 17:56:04

Yup the -j flag was what crashed my system. Building it is fine here with that removed. And there is no need for the mkdir:

# Maintainer: Pest <ppest@protonmail.com>
pkgname=devault
pkgver=1.0.5
pkgrel=1
pkgdesc="Desktop wallet for the dvt blockchain"
arch=('x86_64')
license=('MIT')
url="https://github.com/devaultcrypto/"
source=(${pkgname}-${pkgver}::https://github.com/devaultcrypto/devault/archive/v${pkgver}.tar.gz)
depends=('boost-libs' 'qt5-base' 'qrencode' 'zeromq' 'miniupnpc' 'hicolor-icon-theme' 'libevent')
makedepends=('python' 'boost')
md5sums=('9ca9b2a3b0e0aeafdb5f974c2004ccac')

prepare() {
  cd "${srcdir}/${pkgname}-${pkgver}"
  ./autogen.sh
}

build() {
  cd "${srcdir}/${pkgname}-${pkgver}"
  ./configure --prefix=/usr/
  make
}

package() {
  cd "${srcdir}/${pkgname}-${pkgver}"
  make DESTDIR="${pkgdir}/" install

  install -Dm644 contrib/debian/devault-qt.desktop "${pkgdir}/usr/share/applications/devault.desktop"
  install -Dm644 share/pixmaps/devault-128.png "${pkgdir}/usr/share/icons/hicolor/128x128/apps/devault.png"
  install -Dm644 COPYING  "${pkgdir}/usr/share/licenses/${pkgname}/LICENSE"
}

~~I still doubt those individual install commands are even needed - I'm testing that next.~~

edit: Those do need to be installed explicitly, it seems.

Last edited by Trilby (2019-07-22 18:14:03)

pest · 2019-07-22 18:00:40

I got the install -D to work using the full path.
thanks a lot for taking your time to help, appreciate it!

pest · 2019-07-22 18:08:40

I don't get those files (icon,desktop,license) installed if I leave out the install commands.

pest · 2019-07-22 20:31:28

I decided to submit it.
thanks again for all the help!

Last edited by pest (2019-07-22 21:10:35)

Arch Linux

#1 2019-07-22 00:04:51

Request review of PKGBUILD

#2 2019-07-22 00:13:45

Re: Request review of PKGBUILD

#3 2019-07-22 00:27:11

Re: Request review of PKGBUILD

#4 2019-07-22 00:57:07

Re: Request review of PKGBUILD

#5 2019-07-22 03:21:21

Re: Request review of PKGBUILD

#6 2019-07-22 04:21:55

Re: Request review of PKGBUILD

#7 2019-07-22 04:42:58

Re: Request review of PKGBUILD

#8 2019-07-22 11:55:07

Re: Request review of PKGBUILD

#9 2019-07-22 16:30:28

Re: Request review of PKGBUILD

#10 2019-07-22 16:38:32

Re: Request review of PKGBUILD

#11 2019-07-22 16:56:38

Re: Request review of PKGBUILD

#12 2019-07-22 17:33:06

Re: Request review of PKGBUILD

#13 2019-07-22 17:34:34

Re: Request review of PKGBUILD

#14 2019-07-22 17:35:32

Re: Request review of PKGBUILD

#15 2019-07-22 17:39:19

Re: Request review of PKGBUILD

#16 2019-07-22 17:56:04

Re: Request review of PKGBUILD

#17 2019-07-22 18:00:40

Re: Request review of PKGBUILD

#18 2019-07-22 18:08:40

Re: Request review of PKGBUILD

#19 2019-07-22 20:31:28

Re: Request review of PKGBUILD

Board footer