[aur-general] PKGBUILD Critique
Hi fellow archers, Would you be so kind as to provide a critique of my PKGBUILD? This is only my third package so bear with me, please. <http://codepad.org/Az4Hk78W> Notes: 1)I have emailed current owner/maintainer but no response yet; hopefully he will reply soon. If not will try again and if still no joy, will follow procedure to become new owner/maintainer, if possible. Or just post it under "comments" with a link to my PKGBUILD, perhaps? 2) You can see I have some questions/doubts re. line 26/27 (original PKGBUILD has 26, I changed it to 27, out of habit I guess. I don't think it's an important point though as I guess one should clean up the various dirs left after installing an AUR package, in any event, though. Comments?) 3) Line 37; I tried that and it didn't seem to work for me (not sure why. Comments?) It gives me errors like: warning: directory permissions differ on /home/myusername/ filesystem: 700 package: 755 warning: directory ownership differs on /home/myusername/ filesystem: 1000:100 package: 0:0 (I have a user umask of 075). When I just use: make install, it installs without errors. Thanks!
Hi stef, contragulations on writing your first (?) PKGBUILD. There are a few points that need clearing up before your PKGBUILD can be released for use.
2) You can see I have some questions/doubts re. line 26/27 (original PKGBUILD has 26, I changed it to 27, out of habit I guess. I don't think it's an important point though as I guess one should clean up the various dirs left after installing an AUR package, in any event, though. Comments?)
You should almost never use `make clean`. It is up to `makepkg` and the user to ensure the build directory is fresh. Also `|| return 1` is not required. Do not use `configure --foo="<anything including pkgdir>"`. It will not do what you expect it to. `./configure` is a script which dictates various aspects of the suite after compilation. Options such as --prefix or --<foo>dir set values which become compiled into the program. Imagine that $pkgdir is `/home/user/blah/pkg`. Setting --prefix=$pkgdir will mean that instead of /{etc,usr,var}/ the program will have /home/user/blah/pkg/{etc,usr,var}/. So when installed using pacman, the package is going to put a bunch of files at /home/user/blah/pkg/, right? That's why you're getting errors involving your home dir. Since the program is going to eventually be installed with pacman to `/` then you need to treat the build as if you're installing it to `/` up until the point that you run make install. This way, all the compiled-in values won't involve /home/blah/. I'm probably really crap at explaining this, but it was worth a shot. I can only suggest you read makefiles and learn what each recipe generally does and what variables it draws on. -- Four word witty remark
10.02.2015, 17:20, "David Phillips" <dbphillipsnz@gmail.com>:
. There are a few points that need clearing up before your PKGBUILD can be released for use.
I'm probably really crap at explaining this, but it was worth a shot.
Dave, Thanks for feedback. It's helpful. You're right about just using ./configure --prefix=/user (only that). I took a existing build and tried to update it to new standard, which uses package () , etc. And modified very little, really, above package(). I did see the oddity in the ./configure option but got so involved in the error I was getting from package () that I forgot to backtrack, Anyway, here is an updated version. <https://bpaste.net/show/8fdb2226c3d7> Better? Thanks.
When you do grep like that, you should use two [[, to make sure that it doesn't care about spaces. http://stackoverflow.com/questions/669452/is-preferable-over-in-bash-scripts and in this case, you don't need them at all if grep -qi 'utf-*8' /etc/locale.conf; then The above will fail and not execute the 'then' portion of the if statement if the grep fails, the -q also prevents it from out putting anything, and it will return an exit status of not one. Even with that, the current PKGBUILD looks fine... just wanted to point it out. Daniel On Tue, Feb 10, 2015 at 7:02 PM, stef204 <stef204@yandex.com> wrote:
10.02.2015, 17:20, "David Phillips" <dbphillipsnz@gmail.com>:
. There are a few points that need clearing up before your PKGBUILD can be released for use.
I'm probably really crap at explaining this, but it was worth a shot.
Dave,
Thanks for feedback. It's helpful.
You're right about just using ./configure --prefix=/user (only that).
I took a existing build and tried to update it to new standard, which uses package () , etc.
And modified very little, really, above package(). I did see the oddity in the ./configure option but got so involved in the error I was getting from package () that I forgot to backtrack,
Anyway, here is an updated version.
<https://bpaste.net/show/8fdb2226c3d7>
Better?
Thanks.
10.02.2015, 18:25, "Daniel Wallace" <danielwallace@gtmanfred.com>:
When you do grep like that, you should use two [[, to make sure that it doesn't care about spaces.
http://stackoverflow.com/questions/669452/is-preferable-over-in-bash-scripts
and in this case, you don't need them at all
if grep -qi 'utf-*8' /etc/locale.conf; then
The above will fail and not execute the 'then' portion of the if statement if the grep fails, the -q also prevents it from out putting anything, and it will return an exit status of not one.
Even with that, the current PKGBUILD looks fine... just wanted to point it out.
Daniel
Daniel, Thanks a lot for that. Will read the reference you provided and amend the grep line.
* stef204 <stef204@yandex.com> [2015-02-10 18:02:15 -0700]:
Anyway, here is an updated version.
<https://bpaste.net/show/8fdb2226c3d7>
Better?
Some more remarks: - Is it really a good idea to check /etc/locale.conf? Wouldn't something like [[ ${LANG,,} == *utf-8* ]] be more appropriate? (the ,, converts it to lower-case, see http://wiki.bash-hackers.org/syntax/pe#case_modification ) - I'd move the 'cd' in line 32 to the top of package() and remove the second one in line 35, but that's just a small style issue Flo -- http://www.the-compiler.org | me@the-compiler.org (Mail/XMPP) GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc I love long mails! | http://email.is-not-s.ms/
I question whether checking the locale is appropriate at all. It isn't something that should be compiled-in, surely? -- Four word witty remark
10.02.2015, 22:33, "David Phillips" <dbphillipsnz@gmail.com>:
I question whether checking the locale is appropriate at all. It isn't something that should be compiled-in, surely?
Dave, The INSTALL file in he source states: "the source files of the fortunes are coded in ISO-8859-15 format, and are installed as they are by default. If you want to use an other format, you can specify it by executing the configure script with the option --with-charset=charset, where charset represents the format to use. To have the possibility to use this option, you should have the recode program installed on your system. To get the list of the supported formats, you can use the -l option of the recode program." That is why the check seems to be needed; but what do I know...I'm just a n00b... :) Set me straight as needed, I am not being ironic here; just trying to learn and improve.
10.02.2015, 22:22, "Florian Bruhin" <me@the-compiler.org>:
* stef204 <stef204@yandex.com> [2015-02-10 18:02:15 -0700]:
Anyway, here is an updated version.
<https://bpaste.net/show/8fdb2226c3d7>
Better? Some more remarks:
- Is it really a good idea to check /etc/locale.conf? Wouldn't something like [[ ${LANG,,} == *utf-8* ]] be more appropriate?
(the ,, converts it to lower-case, see http://wiki.bash-hackers.org/syntax/pe#case_modification )
- I'd move the 'cd' in line 32 to the top of package() and remove the second one in line 35, but that's just a small style issue
Flo
Hi, Thanks for feedback. I am looking at the utf-8 issue as you and others have pointed out to check on best way to resolve that; but it could be 'compiled-in' as mentioned by David Phillips. As far as moving the cd and removing the other one, style is good--and I can always do better so that is helpful as well.
10.02.2015, 22:22, "Florian Bruhin" <me@the-compiler.org>:
Some more remarks:
- Is it really a good idea to check /etc/locale.conf? Wouldn't something like [[ ${LANG,,} == *utf-8* ]] be more appropriate?
I don't know. Is it? Is $LANG set automatically or all arch users will have that set for sure? Does it take precedence over /etc/locale.conf ? (Or does it read it from there?) I'm open to suggestions as to which way to best do this; see my reply to David as well, with regards to why this even needs to be checked.
(the ,, converts it to lower-case, see http://wiki.bash-hackers.org/syntax/pe#case_modification )
- I'd move the 'cd' in line 32 to the top of package() and remove the second one in line 35, but that's just a small style issue
OK, I cleaned up the style. And, so far, I followed the suggestion: if grep -qi 'utf-*8' /etc/locale.conf; then (Ignore line 21 for now, will remove it later). @everyone i this thread: Here is revised (cleaned up) version: <https://bpaste.net/show/35fd72050984> Thanks for any additional feedback, as needed.
10.02.2015, 22:22, "Florian Bruhin" <me@the-compiler.org>:
- Is it really a good idea to check /etc/locale.conf? Wouldn't something like [[ ${LANG,,} == *utf-8* ]] be more appropriate?
correcting my last reply to you regarding locale.connf and $LANG. /etc/locale.conf is what sets the $LANG parameter. from the installation wiki: <https://wiki.archlinux.org/index.php/Installation_guide#Configure_the_system> "Set locale preferences in /etc/locale.conf and possibly $HOME/.config/locale.conf: # echo LANG=your_locale > /etc/locale.conf" So, if one has set $LANG via $HOME/.config/locale.conf ; the PKGBUILD would not be dong its job properly, correct? (assuming $HOME/.config/locale.conf and /etc/locale.conf do NOT match.) Thus, I believe you are correct and checking $LANG might be more appropriate. Does everyone agree?
participants (4)
-
Daniel Wallace
-
David Phillips
-
Florian Bruhin
-
stef204