[aur-general] review of getax2019 PKGBUILD

[Eli Schwartz]

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 at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1601 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/aur-general/attachments/20200223/8b0258ee/attachment-0001.sig>