You are not logged in.

#1 2020-07-16 09:26:26

fiskhest
Member
Registered: 2020-07-16
Posts: 5

PKGBUILD review request: bspi-git

I found the very small script bspi useful for my workflow and have been helping the author make it a bit more dynamic and ready for packaging. The next step for me in this would be to add it to the AUR so I don't have to manually keep track of the github repo, and also taking the time to learn and understand how to package for the AUR.

I created a PKGBUILD based on the processes outlined in the arch linux wiki*, and am now ready to publish, but of course do not want to publish something tainted or bad according to the great standards set by the arch linux project. I'm super happy for any feedback on this PKGBUILD:

# Maintainer: fiskhest <johan+bspi at radivoj dot se>
# Maintainer: Adam Rutkowski <hq at mtod dot org>

_pkgname=bspi
pkgname=${_pkgname}-git
pkgver=9.7f58a6a
pkgrel=1
pkgdesc='Icons for bspwm, sort of'
arch=('any')
url="https://github.com/aerosol/${_pkgname}"
license=('custom:BSD')
depends=('bspwm' 'xorg-xprop' 'ttf-font-awesome' 'python')
makedepends=('git')
provides=("${_pkgname}")
conflicts=("${_pkgname}")
source=("git+${url}.git")
md5sums=('SKIP')

pkgver() {
        cd "$srcdir/$_pkgname"
            echo `git rev-list --count HEAD`.`git rev-parse --short HEAD`
        }

package() {
      cd "${_pkgname}"
      mkdir -p "${pkgdir}/usr/bin"
      mkdir -p "${pkgdir}/usr/share/licenses/${pkgname}"
      install bspi.py bspi_listen "${pkgdir}/usr/bin"
      install LICENSE "${pkgdir}/usr/share/licenses/${pkgname}/"
    }

For maintaining the package, is the wisest choice to have a separate git repo specifically for the PKGBUILD and .SRCINFO? Or is it enough to have it on my local filesystem? Should I suggest adding those files to the package git repo?

Many thanks in advance!


*
https://wiki.archlinux.org/index.php/Ar … guidelines
https://wiki.archlinux.org/index.php/AU … g_packages

Offline

#2 2020-07-16 10:28:46

Lone_Wolf
Member
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 11,866

Re: PKGBUILD review request: bspi-git

https://wiki.archlinux.org/index.php/PKGBUILD#license wrote:

The BSD, ISC, MIT, zlib/png, Python and OFL licenses are special cases and could not be included in the licenses package. For the sake of the license array, it is treated as a common license

To me that means all variants of the BSD license are covered by putting license=('BSD') and putting the actual license in the correct spot.


      mkdir -p "${pkgdir}/usr/bin"
      mkdir -p "${pkgdir}/usr/share/licenses/${pkgname}"
      install bspi.py bspi_listen "${pkgdir}/usr/bin"
      install LICENSE "${pkgdir}/usr/share/licenses/${pkgname}/"

You can get rid of the mkdir commands by using the -D option of the install command.

You may want to set permisions for the files explicitly (look up -m / --mode option of install) to ensure they're correct.
files in /usr/bin typically have 755 , while license files typically have 644 .


[ rant ]
My personal opinion is that the PKGBUILD looks like it prefers ease of maintenance / usability as a template   over readability.

example :
one of the things I look at when installing a PKGBUILD is the source array.

step 1 source=("git+${url}.git")
no idea where that is

step 2 check the url-field value and re-parse source=
result : source=("git+https://github.com/aerosol/${_pkgname}.git")

step 3 check the _pkgname-field value and re-parse source=
result : source=("git+https://github.com/aerosol/bspi.git")
Finally I know where the sources come from.

I would use the result from step 3 in source= in the PKGBUILD.

NOTE : This is a personal style preference, use what you feel comfortable with.

Edit : missed adding  .git to the source= several times

Last edited by Lone_Wolf (2020-07-17 10:39:57)


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.


(A works at time B)  && (time C > time B ) ≠  (A works at time C)

Offline

#3 2020-07-16 11:57:48

fiskhest
Member
Registered: 2020-07-16
Posts: 5

Re: PKGBUILD review request: bspi-git

A huge thank you for taking the time and giving honest feedback, Lone_Wolf!

Lone_Wolf wrote:

To me that means all variants of the BSD license are covered by putting license=('BSD') and putting the actual license in the correct spot.

And the specified path

"${pkgdir}/usr/share/licenses/${pkgname}/

in this case is the correct spot?

Lone_Wolf wrote:

You can get rid of the mkdir commands by using the -D option of the install command.

Aah! I must've somehow mistaken the usage of -D with -d.

Lone_Wolf wrote:

You may want to set permisions for the files explicitly (look up -m / --mode option of install) to ensure they're correct.
files in /usr/bin typically have 755 , while license files typically have 644 .

Is there a best practice on being explicit in doing this for the /usr/bin files aswell (as I understand the default is 755), or should I do it only for the license file?

Lone_Wolf wrote:

My personal opinion is that the PKGBUILD looks like it prefers ease of maintenance / usability as a template   over readability.

Absolutely correctly interpreted, it was a last minute change on my end before posting this thread :-) but I don't see that the project shall change too much, so readability is definitely of more worth in this case.

This is the latest result based on my interpretation of your feedback:

# Maintainer: fiskhest <johan+bspi at radivoj dot se>
# Maintainer: Adam Rutkowski <hq at mtod dot org>


_pkgname=bspi
pkgname=${_pkgname}-git
pkgver=9.7f58a6a
pkgrel=1
pkgdesc='Icons for bspwm, sort of'
arch=('any')
url="https://github.com/aerosol/${_pkgname}"
license=('BSD')
depends=('bspwm' 'xorg-xprop' 'ttf-font-awesome' 'python')
makedepends=('git')
provides=("${_pkgname}")
conflicts=("${_pkgname}")
source=("git+https://github.com/aerosol/bspi")
md5sums=('SKIP')

pkgver() {
        cd "$srcdir/$_pkgname"
            echo `git rev-list --count HEAD`.`git rev-parse --short HEAD`
        }

package() {
      cd "${_pkgname}"
      install -D bspi.py bspi_listen "${pkgdir}/usr/bin"
      install -D -m 644 LICENSE "${pkgdir}/usr/share/licenses/${pkgname}/"
    }

Offline

#4 2020-07-16 23:59:48

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: PKGBUILD review request: bspi-git

I am quite fond of source=("git+${url}.git") and would recommend people use makepkg --printsrcinfo | grep 'source =' to parse the sources. An advantage of this is that for tarball downloads of non-git packages, it expands the $pkgver too...

      install -D bspi.py bspi_listen "${pkgdir}/usr/bin"

This won't work, usually each file is installed one by one:

      install -D bspi.py "${pkgdir}/usr/bin/bspi.py"
      install -D bspi_listen "${pkgdir}/usr/bin/bspi_listen"

Note how the filename is repeated on both sides. The -D option only works when naming one SOURCE file, and one DEST file, per the --help output and manpage. If the last argument is an *existing* directory, you can have multiple sources.

Or, you could use:

install -D -t "${pkgdir}/usr/bin" bspi.py bspi_listen

The -t option is a GNU extension (so, Linux only, it will break on BSD) that explicitly takes a directory name and is compatible with -D. Then again, -D is a GNU extension too, which is why Makefiles usually use:

install -d /path/to/dir
install -m755 foo.py /path/to/dir/foo.py

So people often don't realize -t is available. But it's completely okay to use it in a PKGBUILD, as AUR packages for archlinux do not need to worry about BSD. big_smile

...

Your indentation is a bit odd, different lines have different sized indents and the closing } for functions usually doesn't get indented at all, but you're indenting them.

Also you are using `` but this is deprecated and obsolete and kept for compatibility with the 90s. Modern shell scripting should use $() instead, which properly supports nesting without painful multi-level escaping. Regrettably, allowing it to work for backwards compatibility means people see it used in old examples, use it themselves, and never realize that it's bad. I guess it doesn't help that the manual doesn't really point this out, and merely calls it "the old-style backquote form of substitution".


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#5 2020-07-17 07:48:43

fiskhest
Member
Registered: 2020-07-16
Posts: 5

Re: PKGBUILD review request: bspi-git

eschwartz wrote:

I am quite fond of source=("git+${url}.git") and would recommend people use makepkg --printsrcinfo | grep 'source =' to parse the sources. An advantage of this is that for tarball downloads of non-git packages, it expands the $pkgver too...

Hadn't heard about this procedure before, good arguments. I guess in the end it comes down to what I personally prefer, as Lone_Wolf initially pointed out.

eschwartz wrote:

This won't work, usually each file is installed one by one:
[...]
If the last argument is an *existing* directory, you can have multiple sources.

I tested that command inside my file structure and it worked, but I hadn't cleaned up my pkg dir, just like you pointed out. Good catch!

eschwartz wrote:

The -t option is a GNU extension (so, Linux only, it will break on BSD) that explicitly takes a directory name and is compatible with -D.

Nice one, thank you for pointing that out!

eschwartz wrote:

Your indentation is a bit odd, different lines have different sized indents and the closing } for functions usually doesn't get indented at all, but you're indenting them.

Also you are using `` but this is deprecated and obsolete and kept for compatibility with the 90s.

I'll admit, it was a bad copy/pasta-job, and the few packages I used as inspiration all had differing indentation rules (in some cases, mixed indentation within that PKGBUILD). I was hoping someone would say what's what, thanks! :-)


This is the latest PKGBUILD:

# Maintainer: fiskhest <johan+bspi at radivoj dot se>
# Maintainer: Adam Rutkowski <hq at mtod dot org>

_pkgname=bspi
pkgname=${_pkgname}-git
pkgver=9.7f58a6a
pkgrel=1
pkgdesc='Icons for bspwm, sort of'
arch=('any')
url="https://github.com/aerosol/${_pkgname}"
license=('BSD')
depends=('bspwm' 'xorg-xprop' 'ttf-font-awesome' 'python')
makedepends=('git')
provides=("${_pkgname}")
conflicts=("${_pkgname}")
source=("git+${url}")
md5sums=('SKIP')

pkgver() {
    cd "$srcdir/$_pkgname"
    echo "$(git rev-list --count HEAD).$(git rev-parse --short HEAD)"
}

package() {
    cd "${_pkgname}"
    install -D -m 755 -t "${pkgdir}/usr/bin" bspi.py bspi_listen
    install -D -m 644 LICENSE "${pkgdir}/usr/share/licenses/${pkgname}/"
}

Offline

#6 2020-07-17 11:29:37

Lone_Wolf
Member
From: Netherlands, Europe
Registered: 2005-10-04
Posts: 11,866

Re: PKGBUILD review request: bspi-git

source=("git+${url}")

You forgot .git (and I did also twice in #2, corrected now)



And the specified path

"${pkgdir}/usr/share/licenses/${pkgname}/

in this case is the correct spot?

yes, that's the correct spot for licenses of a package.



Lone_Wolf wrote:

    You may want to set permisions for the files explicitly (look up -m / --mode option of install) to ensure they're correct.
    files in /usr/bin typically have 755 , while license files typically have 644 .

Is there a best practice on being explicit in doing this for the /usr/bin files aswell (as I understand the default is 755), or should I do it only for the license file?

Usually those permissions are set by build tools ( make , ninja , setup.py ) , no idea whether there's a best practice for it.

A quick search in the folder where I store aur pkgbuilds shows several (not created/maintained by me) do set the 755 for executable files explicitly.

I am likely to set them explicitly, but I've encountered many problems that could have been avoided by following the Principle of least privilege so am biased.



For maintaining the package, is the wisest choice to have a separate git repo specifically for the PKGBUILD and .SRCINFO? Or is it enough to have it on my local filesystem? Should I suggest adding those files to the package git repo?

Depends.

Having PKGBUILD only publicly accessable through aur keeps it separate from the sourcecode.
This will give viewers the impresssion the sourcecode is usable on every linux distro, not just pacman based ones.
OTOH it will limit cooperation options for the PKGBUILD to what the aur supports.

Do you want people to be able to create issues , pull requests etc for the PKGBUILD ?
If so, put them in a separate repo .
(you could set that repo as remote for the aur repo so you only have to push to one repo).

Last edited by Lone_Wolf (2020-07-17 11:31:08)


Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.


(A works at time B)  && (time C > time B ) ≠  (A works at time C)

Offline

#7 2020-07-17 13:21:40

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: PKGBUILD review request: bspi-git

fiskhest wrote:

I'll admit, it was a bad copy/pasta-job, and the few packages I used as inspiration all had differing indentation rules (in some cases, mixed indentation within that PKGBUILD). I was hoping someone would say what's what, thanks! :-)

Well, indentation styles for source code are an even more divisive topic than the vim vs. emacs wars...

That being said, 4 space indents is objectively the best. big_smile

    echo "$(git rev-list --count HEAD).$(git rev-parse --short HEAD)"

Just FYI: https://wiki.archlinux.org/index.php/VC … elines#Git recommends adding an "r" before the commit count, since otherwise if the upstream repo ever gets tagged versions those will look like downgraded versions. e.g. if the current git master is tagged, going from version "9.7f58a6a" to version "0.1.r0.gb3f8ace".

Going from "r9.7f58a6a" to "0.1.r0.gb3f8ace" will make sure it looks like an update.

fiskhest wrote:

For maintaining the package, is the wisest choice to have a separate git repo specifically for the PKGBUILD and .SRCINFO? Or is it enough to have it on my local filesystem? Should I suggest adding those files to the package git repo?

If you upload it to the AUR, it *must* be a different git repo.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#8 2020-07-17 14:07:53

fiskhest
Member
Registered: 2020-07-16
Posts: 5

Re: PKGBUILD review request: bspi-git

Lone_Wolf wrote:

You forgot .git (and I did also twice in #2, corrected now)

>_< Doh! Thank you for spotting that, and thanks for the rest of the clarifications in your post. Great advice.

eschwartz wrote:

Well, indentation styles for source code are an even more divisive topic than the vim vs. emacs wars...

That being said, 4 space indents is objectively the best. lol

I see what you did there.. tongue

eschwartz wrote:

Just FYI: https://wiki.archlinux.org/index.php/VC … elines#Git recommends adding an "r" before the commit count, since otherwise if the upstream repo ever gets tagged versions those will look like downgraded versions. e.g. if the current git master is tagged, going from version "9.7f58a6a" to version "0.1.r0.gb3f8ace".

Going from "r9.7f58a6a" to "0.1.r0.gb3f8ace" will make sure it looks like an update.

Another thing I had totally overlooked. Thanks! cool

---

Finally, I tried verifying the PKGBUILD (`makepkg -s`) and struck an issue with the install command for the LICENSE file, I have modified the execution parameters to fix that issue but realised that I'm unsure if I should create the LICENSE for ${pkgname}=(bspi-git) or ${_pkgname}=(bspi)? My gut feeling tells me that the license is for the package itself, not for my AUR package, and thus the answer is ${_pkgname}. But gut feelings can never be enough in our line of business. smile What would you suggest?

original:     install -D -m 644 LICENSE "${pkgdir}/usr/share/licenses/${pkgname}"
new:          install -D -m 644 LICENSE "${pkgdir}/usr/share/licenses/${_pkgname}/LICENSE"

The full PKGBUILD with the latest modifications:

# Maintainer: fiskhest <johan+bspi at radivoj dot se>
# Maintainer: Adam Rutkowski <hq at mtod dot org>

_pkgname=bspi
pkgname=${_pkgname}-git
pkgver=r15.ed6d090
pkgrel=1
pkgdesc='Icons for bspwm, sort of'
arch=('any')
url="https://github.com/aerosol/${_pkgname}"
license=('BSD')
depends=('bspwm' 'xorg-xprop' 'ttf-font-awesome' 'python')
makedepends=('git')
provides=("${_pkgname}")
conflicts=("${_pkgname}")
source=("git+${url}.git")
md5sums=('SKIP')

pkgver() {
    cd "$srcdir/$_pkgname"
    printf "r%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)"
}

package() {
    cd "${_pkgname}"
    install -D -m 755 -t "${pkgdir}/usr/bin" bspi.py bspi_listen
    install -D -m 644 LICENSE "${pkgdir}/usr/share/licenses/${pkgname}/LICENSE"
}

Last edited by fiskhest (2020-07-17 14:33:53)

Offline

#9 2020-07-17 14:29:27

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: PKGBUILD review request: bspi-git

The directory /usr/share/licenses/ is a flat-file database of licensing information, and the lookup key for the database is the pacman package you wish to check the licensing status of.

Does that way of looking at it, make things clearer? smile

...

i.e. you always use ${pkgname}, not ${_pkgname}.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#10 2020-07-17 14:33:23

fiskhest
Member
Registered: 2020-07-16
Posts: 5

Re: PKGBUILD review request: bspi-git

eschwartz wrote:

The directory /usr/share/licenses/ is a flat-file database of licensing information, and the lookup key for the database is the pacman package you wish to check the licensing status of.

Does that way of looking at it, make things clearer? smile

...

i.e. you always use ${pkgname}, not ${_pkgname}.

Gotcha, cheers for clarifying! I've modified my source (and previous post) accordingly.

Offline

Board footer

Powered by FluxBB