[aur-general] Review request for a PKGBUILD
Hi, I'd like to push the following PKGBUILD to AUR. It is useful for those who want to use HDF5 files with LZ4 compression. There are two things that bother me about it: 1) The upstream repository contains 4 directories: LZ4, BLOSC, BZIP2, and docs. Of which only LZ4 is used. I just "git rm -rf" the unneeded directories, as you can see. But it doesn't seem very elegant. 2) The package will work properly only if the "HDF5_PLUGIN_PATH" env. variable is set. Putting hdf5_env.sh into /etc/profile.d takes care of that for any subsequent login. But is there a way to set this variable in the same shell from which the installation is being performed? And to unset it on removal? It's not critical at all - no problem to open a new shell after installation, just for the sake of correctness. Of course, if there are other problems - I'd like to hear as well. Thanks! Leonid. ___ PKGBUILD: ~~~~~~~~~~ # Package maintainer: Leonid B <leonid dot bloch at esrf dot fr> pkgname=hdf5-lz4-filter-git pkgver=r106.863db28 pkgrel=1 pkgdesc="LZ4 filter for the HDF5 data format" arch=('i686' 'x86_64') url="https://github.com/nexusformat/HDF5-External-Filter-Plugins/tree/master/LZ4" license=('BSD') depends=('hdf5' 'lz4') makedepends=('git') source=("${pkgname%-git}::git+https://github.com/nexusformat/HDF5-External-Filter-Plugins.git" "hdf5_env.sh") sha1sums=('SKIP' 'c82f6a025138c3f9430ab930a7dd42ad0963918b') pkgver() { cd "${srcdir}/${pkgname%-git}" printf "r%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)" } prepare() { cd "${srcdir}/${pkgname%-git}" git rm -rf BLOSC BZIP2 docs } build() { cd "${srcdir}/${pkgname%-git}/LZ4" ./configure --prefix=/usr --with-hdf5=/usr --with-lz4lib=/usr make } package() { cd "${srcdir}/${pkgname%-git}/LZ4" make DESTDIR="${pkgdir}/" install install -D -m755 "${srcdir}/hdf5_env.sh" "${pkgdir}/etc/profile.d/hdf5_env.sh" install -D -m644 COPYING "${pkgdir}/usr/share/licenses/${pkgname}/LICENSE" } hdf5_env.sh: ~~~~~~~~~~~ export HDF5_PLUGIN_PATH=/usr/lib
On 01/07/2017 11:59 PM, Leonid Bloch wrote:
Hi,
I'd like to push the following PKGBUILD to AUR. It is useful for those who want to use HDF5 files with LZ4 compression.
There are two things that bother me about it: 1) The upstream repository contains 4 directories: LZ4, BLOSC, BZIP2, and docs. Of which only LZ4 is used. I just "git rm -rf" the unneeded directories, as you can see. But it doesn't seem very elegant.
Any especial need to rm it? If it isn't used then you can just ignore that it is there. :) Your PKGBUILD syntax looks fine, the only small nit I have is that by convention the printf uses "r%s.g%s" (see the "VCS package guidelines" wiki page). sha1 is weak, but given that the /etc/profile.d dropin is downloaded with the PKGBUILD it is no worse than using md5sums=('SKIP') for git repos rather than skipping sha256 instead. :D It remains debatable whether the average AUR maintainer actually checks sources first anyway... -- Eli Schwartz
Eli, thanks for your review! Comments inline. On Sun, Jan 8, 2017 at 7:13 AM, Eli Schwartz via aur-general <aur-general@archlinux.org> wrote:
On 01/07/2017 11:59 PM, Leonid Bloch wrote:
Hi,
I'd like to push the following PKGBUILD to AUR. It is useful for those who want to use HDF5 files with LZ4 compression.
There are two things that bother me about it: 1) The upstream repository contains 4 directories: LZ4, BLOSC, BZIP2, and docs. Of which only LZ4 is used. I just "git rm -rf" the unneeded directories, as you can see. But it doesn't seem very elegant.
Any especial need to rm it? If it isn't used then you can just ignore that it is there. :)
Space considerations? Is there a problem with removing them if so?
Your PKGBUILD syntax looks fine, the only small nit I have is that by convention the printf uses "r%s.g%s" (see the "VCS package guidelines" wiki page).
Maybe I'm missing something, but the wiki says "r%s.%s" if there are no tags.
sha1 is weak, but given that the /etc/profile.d dropin is downloaded with the PKGBUILD it is no worse than using md5sums=('SKIP') for git repos rather than skipping sha256 instead. :D It remains debatable whether the average AUR maintainer actually checks sources first anyway...
Agree. Will use sha256, just as a good practice. Just as I look at others' PKGBUILDs for examples, someone might look at mine. :)
-- Eli Schwartz
On 01/08/2017 12:42 AM, Leonid Bloch wrote:
Any especial need to rm it? If it isn't used then you can just ignore that it is there. :)
Space considerations? Is there a problem with removing them if so?
No problem, I just figured if it only exists in the build directory anyway it might not matter. They will probably remove the build directory afterwards anyway.
Maybe I'm missing something, but the wiki says "r%s.%s" if there are no tags.
Hmm, I guess I was missing something actually. But I personally like the consistency with the "tags exist" version, so I will allow my personal opinions to override the Wiki on that (at least in my own PKGBUILDs). It was subjective anyway. Good catch! -- Eli Schwartz
Le 08/01/2017 à 05:59, Leonid Bloch a écrit :
pkgver() { cd "${srcdir}/${pkgname%-git}"
prepare() { cd "${srcdir}/${pkgname%-git}"
build() { cd "${srcdir}/${pkgname%-git}/LZ4"
package() { cd "${srcdir}/${pkgname%-git}/LZ4"
Just a tiny thing, but makepkg always start each function into "${srcdir}$", so you could remove it from these paths (and thus the quote altogether).
install -D -m755 "${srcdir}/hdf5_env.sh" "${pkgdir}/etc/profile.d/hdf5_env.sh"
However, don’t remove it here, because you’re not in the "${srcdir}" anymore at this point. I’m sure you know, but that’s just in case you could have went too fast in removing ${srcdir} everywhere (I did when I learned about makepkg and "${srcdir}"). ;) Regards, Bruno
On Sun, Jan 8, 2017 at 4:49 PM, Bruno Pagani via aur-general <aur-general@archlinux.org> wrote:
cd "${srcdir}/${pkgname%-git}/LZ4"
Just a tiny thing, but makepkg always start each function into "${srcdir}$", so you could remove it from these paths (and thus the quote altogether).
Bash gurus recommend keeping the quotes, as ${pkgname%-git} is also a parameter, and we generally encourage those be quoted. cheers! mar77i
On Sun, 8 Jan 2017 20:04:26 +0100 Martin Kühne via aur-general <aur-general@archlinux.org> wrote:
On Sun, Jan 8, 2017 at 4:49 PM, Bruno Pagani via aur-general <aur-general@archlinux.org> wrote:
cd "${srcdir}/${pkgname%-git}/LZ4"
Just a tiny thing, but makepkg always start each function into "${srcdir}$", so you could remove it from these paths (and thus the quote altogether).
Bash gurus recommend keeping the quotes, as ${pkgname%-git} is also a parameter, and we generally encourage those be quoted.
cheers! mar77i
$pkgname is controlled by the PKGBUILD and is guaranteed to to contain spaces. Quotes wouldn't be needed.
On Sun, 8 Jan 2017 13:08:57 -0600 Doug Newgard <scimmia@archlinux.info> wrote:
On Sun, 8 Jan 2017 20:04:26 +0100 Martin Kühne via aur-general <aur-general@archlinux.org> wrote:
On Sun, Jan 8, 2017 at 4:49 PM, Bruno Pagani via aur-general <aur-general@archlinux.org> wrote:
cd "${srcdir}/${pkgname%-git}/LZ4"
Just a tiny thing, but makepkg always start each function into "${srcdir}$", so you could remove it from these paths (and thus the quote altogether).
Bash gurus recommend keeping the quotes, as ${pkgname%-git} is also a parameter, and we generally encourage those be quoted.
cheers! mar77i
$pkgname is controlled by the PKGBUILD and is guaranteed to to contain spaces. Quotes wouldn't be needed.
Crap, that should be "not to contain", rather than "to to contain"
On 01/08/2017 02:04 PM, Martin Kühne via aur-general wrote:
Bash gurus recommend keeping the quotes, as ${pkgname%-git} is also a parameter, and we generally encourage those be quoted.
Just as many gurus recommend the exact opposite if the *variable* is guaranteed to not have spaces. -- Eli Schwartz
Bruno, thanks for the catch! You are right! But I prefer to keep the quotes: too many times I fell for some unexplained behavior in Bash the reason for which was forgetting to quote a variable. I try to quote wherever possible since. :) Updated PKGBUILD: ~~~~~~~~~~~~~~~~~ # Package maintainer: Leonid B <leonid dot bloch at esrf dot fr> pkgname=hdf5-lz4-filter-git pkgver=r106.g863db28 pkgrel=1 pkgdesc="LZ4 filter for the HDF5 data format" arch=('i686' 'x86_64') url="https://github.com/nexusformat/HDF5-External-Filter-Plugins/tree/master/LZ4" license=('BSD') depends=('hdf5' 'lz4') makedepends=('git') source=("${pkgname%-git}::git+https://github.com/nexusformat/HDF5-External-Filter-Plugins.git" "hdf5_env.sh") sha256sums=('SKIP' '643d90a15a5105d891adea12806d468aef134f902a38761e864a1085370fb4f9a') pkgver() { cd "${pkgname%-git}" printf "r%s.g%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)" } prepare() { cd "${pkgname%-git}" git rm -rf BLOSC BZIP2 docs } build() { cd "${pkgname%-git}/LZ4" ./configure --prefix=/usr --with-hdf5=/usr --with-lz4lib=/usr make } package() { cd "${pkgname%-git}/LZ4" make DESTDIR="${pkgdir}/" install install -D -m755 "${srcdir}/hdf5_env.sh" "${pkgdir}/etc/profile.d/hdf5_env.sh" install -D -m644 COPYING "${pkgdir}/usr/share/licenses/${pkgname}/LICENSE" } On Sun, Jan 8, 2017 at 5:49 PM, Bruno Pagani via aur-general <aur-general@archlinux.org> wrote:
Le 08/01/2017 à 05:59, Leonid Bloch a écrit :
pkgver() { cd "${srcdir}/${pkgname%-git}"
prepare() { cd "${srcdir}/${pkgname%-git}"
build() { cd "${srcdir}/${pkgname%-git}/LZ4"
package() { cd "${srcdir}/${pkgname%-git}/LZ4"
Just a tiny thing, but makepkg always start each function into "${srcdir}$", so you could remove it from these paths (and thus the quote altogether).
install -D -m755 "${srcdir}/hdf5_env.sh" "${pkgdir}/etc/profile.d/hdf5_env.sh"
However, don’t remove it here, because you’re not in the "${srcdir}" anymore at this point. I’m sure you know, but that’s just in case you could have went too fast in removing ${srcdir} everywhere (I did when I learned about makepkg and "${srcdir}"). ;)
Regards, Bruno
Le 08/01/2017 à 22:13, Leonid Bloch a écrit :
Bruno, thanks for the catch! You are right!
But I prefer to keep the quotes: too many times I fell for some unexplained behavior in Bash the reason for which was forgetting to quote a variable. I try to quote wherever possible since. :)
Well that’s a good precaution generally, and I know what I’m talking about since my involvement in Bumblebee (and FLOSS more generally) started partially because someone didn’t took it[0], but here see Eli and Doug comments, this is really unnecessary. ;) Cheers, Bruno [0] https://github.com/MrMEEE/bumblebee-Old-and-abbandoned/commit/a047be85247755...
On 01/08/2017 04:35 PM, Bruno Pagani via aur-general wrote:
started partially because someone didn’t took it[0], but here see Eli and Doug comments, this is really unnecessary. ;)
Would this be a bad time to say that my personal style of choice is to over-quote? :p All I said was "appeal to authority" == fail because you can get gurus to say whatever you want. It's a very subjective bikeshed. As long as you *do* use quotes in circumstances where you *can* get unpredictable user input containing spaces (or nothing of course!) it really doesn't matter what you do the rest of the time. It is a style preference. -- Eli Schwartz
Le 08/01/2017 à 22:55, Eli Schwartz via aur-general a écrit :
On 01/08/2017 04:35 PM, Bruno Pagani via aur-general wrote:
started partially because someone didn’t took it[0], but here see Eli and Doug comments, this is really unnecessary. ;) Would this be a bad time to say that my personal style of choice is to over-quote? :p
Well, in that case, I’ll call only to Doug. ;p
All I said was "appeal to authority" == fail because you can get gurus to say whatever you want. It's a very subjective bikeshed.
As long as you *do* use quotes in circumstances where you *can* get unpredictable user input containing spaces (or nothing of course!) it really doesn't matter what you do the rest of the time. It is a style preference.
You’re absolutely right. :) My own preference goes to less chars, and thus making sure I know what I’m doing. ;) But I’m OK with people over-quoting, like you said it’s under-quoting which is an issue. I was just applying a second layer of “there is nothing to be feared here in the absence of quotes”. Bruno
Thanks for my sunday night's identiy crisis, I can no longer tell whether I was implying myself as an authority there or not. Or whether I intended to imply somesuch. cheers! mar77i
On 01/08/2017 05:42 PM, Martin Kühne via aur-general wrote:
Thanks for my sunday night's identiy crisis, I can no longer tell whether I was implying myself as an authority there or not. Or whether I intended to imply somesuch.
Happy to be of service. :p -- Eli Schwartz
participants (5)
-
Bruno Pagani
-
Doug Newgard
-
Eli Schwartz
-
Leonid Bloch
-
Martin Kühne