[aur-general] review of getax2019 PKGBUILD
Hi folks, I've built my first PKGBUILD based on Brunio Renié's vaudtax, https://aur.archlinux.org/packages/vaudtax/ https://gitlab.com/greut/getax Do you think I could propose it as is to aur-request? Do you see anything that could be improved? Cheers, -- Yoan
On Sun, 2020-02-23 at 13:17 +0100, Yoan Blanc via aur-general wrote:
Hi folks,
I've built my first PKGBUILD based on Brunio Renié's vaudtax, https://aur.archlinux.org/packages/vaudtax/
https://gitlab.com/greut/getax
Do you think I could propose it as is to aur-request? Do you see anything that could be improved?
Cheers,
Some things that stand out: - You prepare shouldn't ever download things; you should use the sources array for that. - The build function isn't mandatory and can be left out if unused. - The included `getax` script checks for java somehow, which is a direct dependency. If this script comes from somewhere else, include it in the sources array. Otherwise, just assume that your dependencies are actualy isntalled. - You should consistently quote $srcdir and $pkgdir since they may contain spaces. - Not sure if there is a rule about it, but a package description in French seems odd to me. - You depend on a specific java package rather than a specific java version, or just java-environment. Why? - Take a look at the java packaging guidelines in [1], it has more general tips wrt to packaging java apps. Cheers, Bert. [1]: https://wiki.archlinux.org/index.php/Java_package_guidelines
On Sun, 23 Feb 2020 13:17:44 +0100
Yoan Blanc via aur-general
Hi folks,
I've built my first PKGBUILD based on Brunio Renié's vaudtax, https://aur.archlinux.org/packages/vaudtax/
https://gitlab.com/greut/getax
Do you think I could propose it as is to aur-request? Do you see anything that could be improved?
Cheers,
You should not be downloading anything in the prepare function if it can be avoided. $_pkgver is being added to the downloaded filename, does it not contain the updates? If this is really necessary, it should all be in the source array. Pick a variable style and stick with it. Some have braces, some don't. Make sure to quote all variables that you don't control and could contain spaces. Get rid of the empty build function. Why call cp 3 times when once would do it? Don't use tabs for alignment (sha256sums). aur-requests is a mailing list for automated listing of requests for people to reply to. You don't send new threads to it directly, and the requests in question are only deleting package or merging them into other packages, it's not what you want here. You just need to use the comments.
On 2/23/20 7:17 AM, Yoan Blanc via aur-general wrote:
Hi folks,
I've built my first PKGBUILD based on Brunio Renié's vaudtax, https://aur.archlinux.org/packages/vaudtax/
https://gitlab.com/greut/getax
Do you think I could propose it as is to aur-request? Do you see anything that could be improved?
Added to what others have pointed out... for v in $(seq 1 2); do varname=_update_1_0$v for f in ${!varname}; do You could just use: for f in _update_1_0{1,2} Your file https://gitlab.com/greut/getax/-/blob/master/getax contains bash-specific &>, but a /bin/sh shebang. You can easily use /bin/sh compatible syntax: type java &> /dev/null becomes type java > /dev/null 2>&1 ... but you should not be using "type" at all, just use command -v >/dev/null You also don't need to check if [ "$?" -ne "0" ] ; then Instead, just check if ! command -v java >/dev/null; then later you use: which zenity The "which" program is not guaranteed to be installed, stick to one builtin method (command -v is preferred, type has reliable results if you stick to #!/bin/bash). (Of course this is mostly moot if your package depends on java-runtime anyway.) cd $(dirname $(dirname $(realpath $0))) Each $() should be quoted, just like a variable, to avoid unexpected whitespace in paths. Since you're not hardcoding the same directory you installed to, but building it using dirname/realpath, you should not be assuming *anything* about the path. pkgver=2019 _pkgver=1.02 This is confusing and the latter should maybe be "_minor_version" or something that makes it clear why they are so different. -- Eli Schwartz Bug Wrangler and Trusted User
Le dim. 23 févr. 2020 à 15:36, Bert Peters via aur-general < aur-general@archlinux.org> a écrit :
On Sun, 2020-02-23 at 13:17 +0100, Yoan Blanc via aur-general wrote:
Hi folks,
I've built my first PKGBUILD based on Brunio Renié's vaudtax, https://aur.archlinux.org/packages/vaudtax/
https://gitlab.com/greut/getax
Do you think I could propose it as is to aur-request? Do you see anything that could be improved?
Cheers,
Some things that stand out: - You prepare shouldn't ever download things; you should use the sources array for that. - The build function isn't mandatory and can be left out if unused. - The included `getax` script checks for java somehow, which is a direct dependency. If this script comes from somewhere else, include it in the sources array. Otherwise, just assume that your dependencies are actualy isntalled. - You should consistently quote $srcdir and $pkgdir since they may contain spaces. - Not sure if there is a rule about it, but a package description in French seems odd to me. - You depend on a specific java package rather than a specific java version, or just java-environment. Why? - Take a look at the java packaging guidelines in [1], it has more general tips wrt to packaging java apps.
Wow, thanks a lot. I've managed to: - use source for all the files - simplify the shell script by assuming all the dependencies are installed - translate the description :) - and more. Although, it only works with Java 8... so it has to stick to jre8-openjdk. I did use some trickery to recreate the downloaded file hierarchy. I'm hoping very much that my state (Geneva, hence GeTax) will provide a tarball containing the updates which will make my life much easier. Automatic updates makes close to no sense in an environment with a working package management. But that's another story. Thanks to Doug and Eli as well, much appreciated! Shall I proceed with creating the AUR on aur.archlinux.org? -- Yoan Cheers,
Bert.
[1]: https://wiki.archlinux.org/index.php/Java_package_guidelines
On 2/23/20 1:11 PM, Yoan Blanc via aur-general wrote:
Le dim. 23 févr. 2020 à 15:36, Bert Peters via aur-general < aur-general@archlinux.org> a écrit :
On Sun, 2020-02-23 at 13:17 +0100, Yoan Blanc via aur-general wrote:
Hi folks,
I've built my first PKGBUILD based on Brunio Renié's vaudtax, https://aur.archlinux.org/packages/vaudtax/
https://gitlab.com/greut/getax
Do you think I could propose it as is to aur-request? Do you see anything that could be improved?
Cheers,
Some things that stand out: - You prepare shouldn't ever download things; you should use the sources array for that. - The build function isn't mandatory and can be left out if unused. - The included `getax` script checks for java somehow, which is a direct dependency. If this script comes from somewhere else, include it in the sources array. Otherwise, just assume that your dependencies are actualy isntalled. - You should consistently quote $srcdir and $pkgdir since they may contain spaces. - Not sure if there is a rule about it, but a package description in French seems odd to me. - You depend on a specific java package rather than a specific java version, or just java-environment. Why? - Take a look at the java packaging guidelines in [1], it has more general tips wrt to packaging java apps.
Wow, thanks a lot. I've managed to: - use source for all the files - simplify the shell script by assuming all the dependencies are installed - translate the description :) - and more.
Although, it only works with Java 8... so it has to stick to jre8-openjdk.
You should use "java-runtime=8" as the dependency since that is provided by jre8-openjdk but may also be provided by oracle java from the AUR. On the changes you have made: for f in $(ls config\|* lib\|* model\|*); do Don't do this: http://mywiki.wooledge.org/BashPitfalls#for_f_in_.24.28ls_.2A.mp3.29 config\|* lib\|* model\|* This is already a list of filenames, there is no need to run them through a subshell and the ls program just to break on strange corner cases, and eventually get the glob expansion right back anyway. You're still using unquoted: cd $(dirname $(dirname $(realpath $0))) Also, license=('custom') -- where is the license text? See https://wiki.archlinux.org/index.php/PKGBUILD#license -- Eli Schwartz Bug Wrangler and Trusted User
Le dim. 23 févr. 2020 à 19:24, Eli Schwartz via aur-general < aur-general@archlinux.org> a écrit :
On 2/23/20 1:11 PM, Yoan Blanc via aur-general wrote:
Le dim. 23 févr. 2020 à 15:36, Bert Peters via aur-general < aur-general@archlinux.org> a écrit :
On Sun, 2020-02-23 at 13:17 +0100, Yoan Blanc via aur-general wrote:
Hi folks,
I've built my first PKGBUILD based on Brunio Renié's vaudtax, https://aur.archlinux.org/packages/vaudtax/
https://gitlab.com/greut/getax
Do you think I could propose it as is to aur-request? Do you see anything that could be improved?
Cheers,
Some things that stand out: - You prepare shouldn't ever download things; you should use the sources array for that. - The build function isn't mandatory and can be left out if unused. - The included `getax` script checks for java somehow, which is a direct dependency. If this script comes from somewhere else, include it in the sources array. Otherwise, just assume that your dependencies are actualy isntalled. - You should consistently quote $srcdir and $pkgdir since they may contain spaces. - Not sure if there is a rule about it, but a package description in French seems odd to me. - You depend on a specific java package rather than a specific java version, or just java-environment. Why? - Take a look at the java packaging guidelines in [1], it has more general tips wrt to packaging java apps.
Wow, thanks a lot. I've managed to: - use source for all the files - simplify the shell script by assuming all the dependencies are installed - translate the description :) - and more.
Although, it only works with Java 8... so it has to stick to jre8-openjdk.
You should use "java-runtime=8" as the dependency since that is provided by jre8-openjdk but may also be provided by oracle java from the AUR.
On the changes you have made:
for f in $(ls config\|* lib\|* model\|*); do
Don't do this: http://mywiki.wooledge.org/BashPitfalls#for_f_in_.24.28ls_.2A.mp3.29
config\|* lib\|* model\|*
This is already a list of filenames, there is no need to run them through a subshell and the ls program just to break on strange corner cases, and eventually get the glob expansion right back anyway.
You're still using unquoted:
cd $(dirname $(dirname $(realpath $0)))
Also, license=('custom') -- where is the license text? See https://wiki.archlinux.org/index.php/PKGBUILD#license
Good point, I've switched to unknown and will ask the administration what is actual status. Probably unkown to them as well. With the archilnux-java tool and the current situation around Java 8, 11 and 13. It's hard to predict what will be the default java. I don't expect many people to use this anyways. Thanks for comments nonetheless!
-- Eli Schwartz Bug Wrangler and Trusted User
participants (4)
-
Bert Peters
-
Doug Newgard
-
Eli Schwartz
-
Yoan Blanc