You are not logged in.

#1 2019-02-01 16:03:23

ChinaskiJr
Member
Registered: 2019-02-01
Posts: 3

[SOLVED] PKBUILD review request

Hello everyone,
After a week on Arch, this is my first PKGBUILD, thank you very much in advance for your help :
https://github.com/ChinaskiJr/vpndemon-cli-aur

Last edited by ChinaskiJr (2019-02-02 09:59:01)

Offline

#2 2019-02-01 17:10:40

Piri
Member
Registered: 2018-06-02
Posts: 43

Re: [SOLVED] PKBUILD review request

Some points I noticed:

* you got a couple of implicit dependencies that are not needed (i.e. 'dbus' and 'openvpn')
* bash is not really needed in makedepends since it is assumed for builders to have base-devel installed and pacman depends on bash anyway (and so do other core parts of arch).
* the source array should hold a Release URL, not the master branch. Also it should be named $pkgname-$pkgver (so people who set SRCDEST don't get into trouble). For an example look at https://git.archlinux.org/svntogit/comm … ages/piper

* you only need one shasum array. Either drop sha256sums or sha512sums.
* you don't need all those mkdir/cp/mv in package(), install can do all that for you:

package() {                                                                     
    install -D -m755 "${srcdir}/vpndemon-cli-master/vpndemon-cli.sh" "${pkgdir}/usr/bin/vpndeom-cli"
}                                                                               

Also, your project is not really properly licensed, neither is the upstream project. I'd suggest adding a LICENSE file to the project. Github can create the template for you!

Offline

#3 2019-02-01 17:23:20

Slithery
Forum Moderator
From: Norfolk, UK
Registered: 2013-12-01
Posts: 3,130

Re: [SOLVED] PKBUILD review request

Welcome to the forums ChinaskiJr smile
A couple of points to get you started...

The source URL is incorrect. As you are building a specific version you need to specify this otherwise you'll be pulling from git master instead.
Also read this.

source=("$pkgname-$pkgver.zip::$url/archive/v$pkgver.zip")

You don't need both checksums. You can remove sha256sums.

Why do you have bash listed as a makedepend?

You haven't provided the correct license...
https://wiki.archlinux.org/index.php/PKGBUILD#license

Your entire package function can be replaced by one line...

man install

No, it didn't "fix" anything. It just shifted the brokeness one space to the right. - jasonwryan
Closing -- for deletion; Banning -- for muppetry. - jasonwryan

aur - dotfiles

Offline

#4 2019-02-01 18:46:40

ChinaskiJr
Member
Registered: 2019-02-01
Posts: 3

Re: [SOLVED] PKBUILD review request

Hello guys,
Thank you both for your answers !
I updated the repository with your recommendations. I just only have one last question, it's about the dependencies :

Piri wrote:

* you got a couple of implicit dependencies that are not needed (i.e. 'dbus' and 'openvpn')

I now understand well for openvpn and networkmanager-openvpn, but what about dbus ? Do you mean the dependency is not needed because it's part of the Arch core as a dependency of systemd ?

Offline

#5 2019-02-01 20:45:50

Trilby
Inspector Parrot
Registered: 2011-11-29
Posts: 20,941
Website

Re: [SOLVED] PKBUILD review request

Piri wrote:

* bash is not really needed in makedepends since it is assumed for builders to have base-devel installed

Bash is not in base-devel.


"UNIX is simple and coherent..." - Dennis Ritchie, "GNU's Not UNIX" -  Richard Stallman

Offline

#6 2019-02-01 21:21:38

Scimmia
Bug Wrangler
Registered: 2012-09-01
Posts: 7,141

Re: [SOLVED] PKBUILD review request

It is transitively, though pacman and probably others.

Offline

#7 2019-02-01 21:32:56

Trilby
Inspector Parrot
Registered: 2011-11-29
Posts: 20,941
Website

Re: [SOLVED] PKBUILD review request

Eh, no.  The second half of the statement, which I did not quote, indicates correctly that bash is required by pacman and thus would already be installed.  But this was in addition to the statement that I quoted and responded to that bash would already be installed because it is in base-devel.  It is not.

It may be logical to assume that bash would already be installed for other reasons, but then those other reasons should stand on their own, not be an after-thought to a false premise.

Last edited by Trilby (2019-02-01 21:34:15)


"UNIX is simple and coherent..." - Dennis Ritchie, "GNU's Not UNIX" -  Richard Stallman

Offline

#8 2019-02-02 00:22:19

Piri
Member
Registered: 2018-06-02
Posts: 43

Re: [SOLVED] PKBUILD review request

ChinaskiJr wrote:

I now understand well for openvpn and networkmanager-openvpn, but what about dbus ? Do you mean the dependency is not needed because it's part of the Arch core as a dependency of systemd ?

You could logically assume that systemd is installed, but NetworkManager actually has a dependency chain that leads to dbus.
networkmanager -> polkit -> systemd -> dbus

Trilby wrote:

It may be logical to assume that bash would already be installed for other reasons, but then those other reasons should stand on their own, not be an after-thought to a false premise.

I see your point, I tried to cram two things into one sentence and it kinda ruined it all. I should have made a separate sentence for the last part smile

Offline

#9 2019-02-02 01:44:44

Scimmia
Bug Wrangler
Registered: 2012-09-01
Posts: 7,141

Re: [SOLVED] PKBUILD review request

Trilby wrote:

Eh, no.  The second half of the statement, which I did not quote, indicates correctly that bash is required by pacman and thus would already be installed.  But this was in addition to the statement that I quoted and responded to that bash would already be installed because it is in base-devel.  It is not.

Pacman is in base-devel. Pacman depends on bash. Hence, bash is in base-devel transitively, as I said.

Offline

#10 2019-02-02 09:57:27

ChinaskiJr
Member
Registered: 2019-02-01
Posts: 3

Re: [SOLVED] PKBUILD review request

Ok then, thank you very much for all your help. I'm going to submit this to the AUR now.
Bye !

Offline

Board footer

Powered by FluxBB