[aur-general] PKGBUILD review request
Hello, This is my first attempt at creating a package so I'd appreciate any feedback you have. The PKGBUILD is avilable here: https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD Thanks!
On Fri, Jan 10, 2014 at 6:59 PM, Douglas Christman <douglaschristman@gmail.com> wrote:
Hello,
This is my first attempt at creating a package so I'd appreciate any feedback you have. The PKGBUILD is avilable here: https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD
Thanks!
Hello, You should quote the link in the source array and any string that includes a Bash variable (e.g. use `install -Dm 755 "$pkgname-go-$pkgver" "$pkgdir/usr/bin/$pkgname-go"`). You can remove the dependency on 'bash' because you shouldn't include dependency information for any packages in the groups 'base' or 'base-devel'. Unless this package will _only_ work for 64-bit systems, you should change the arch array to the following: arch=('i686' 'x86_64') And this is a minor issue, but you should change the GitHub links to use https:// so the source download will _always_ use HTTPS, even if the http -> https redirect that GitHub uses stops working for whatever reason. Jason
On 01/10/2014 07:32 PM, Jason St. John wrote:
Unless this package will _only_ work for 64-bit systems, you should change the arch array to the following: arch=('i686' 'x86_64')
Should the documentation be updated? [1] describes "arch" as "An array of architectures that the PKGBUILD file is known to build and work on." That description implies to me that if you haven't tested it on i686 then you shouldn't list i686. However, the actual AUR convention is, as you say, to include i686 and x86_64 unless you know it's nonportable. [1] https://wiki.archlinux.org/index.php/PKGBUILD#arch
Hi On Fri, Jan 10, 2014 at 4:32 PM, Jason St. John <jstjohn@purdue.edu> wrote:
You should quote the link in the source array and any string that includes a Bash variable (e.g. use `install -Dm 755 "$pkgname-go-$pkgver" "$pkgdir/usr/bin/$pkgname-go"`).
Could you please please explain it? Why bash variables need quotes? Well, $srcdir/$destdir need double-quotes because they might contain spaces (although creating dirs with white-spaces on UNIX is a dumb idea). What about other variables like $pkgver? The same question applied to other parts of PKGBUILD - why quotes are needed for dependencies, license, arch, sources...?
On 2014-01-10 19:10, Anatol Pomozov wrote:
Well, $srcdir/$destdir need double-quotes because they might contain spaces (although creating dirs with white-spaces on UNIX is a dumb idea). What about other variables like $pkgver? The same question applied to other parts of PKGBUILD - why quotes are needed for dependencies, license, arch, sources...?
It's not actually required if you know that the variable won't expand incorrectly. However, it is good practice to do so. And, using double-quotes will ensure proper expansion. I tend to be incredibly verbose about this and even include braces for variables: "${pkgname}-${pkgver}.tar.gz" It looks cleanest to me this way, and I know that it will always expand the way I want it to. This is, however, a matter of style more than anything else. -- All the best, Sam Stuewe (HalosGhost)
Hi! On Friday 10 January 2014 18:59:35 Douglas Christman wrote:
This is my first attempt at creating a package so I'd appreciate any feedback you have. The PKGBUILD is avilable here: https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD
--------------- it was written by Jason --------------- 1. It is highly recommended using double quotes for paths (to avoid problems with paths with spaces, for example). 2. What about i686 architecture? If it is supported it must be added to arch list. 3. Dependency 'bash' could be removed because it is in base group and several packages in base-devel group depend on it [1, see Warning message]. --------------- it was written by Jason --------------- 4. It has several errors during build (could not find packages github.com/dobyrch/*). I think you must fix it in the upstream =) [1]. https://wiki.archlinux.org/index.php/AUR#Prerequisites -- С уважением, Е.Алексеев. Sincerely yours, E.Alekseev. e-mail: darkarcanis@mail.ru ICQ: 407-398-235 Jabber: arcanis@jabber.ru
It looks like GOPATH needed to be set for it to build correctly. Would you mind testing out the changes I made to see if it fixes the problem? https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD On Fri, Jan 10, 2014 at 7:36 PM, Evgeniy Alekseev <darkarcanis@mail.ru>wrote:
Hi!
On Friday 10 January 2014 18:59:35 Douglas Christman wrote:
This is my first attempt at creating a package so I'd appreciate any feedback you have. The PKGBUILD is avilable here: https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD
--------------- it was written by Jason --------------- 1. It is highly recommended using double quotes for paths (to avoid problems with paths with spaces, for example). 2. What about i686 architecture? If it is supported it must be added to arch list. 3. Dependency 'bash' could be removed because it is in base group and several packages in base-devel group depend on it [1, see Warning message]. --------------- it was written by Jason ---------------
4. It has several errors during build (could not find packages github.com/dobyrch/*). I think you must fix it in the upstream =)
[1]. https://wiki.archlinux.org/index.php/AUR#Prerequisites -- С уважением, Е.Алексеев. Sincerely yours, E.Alekseev.
e-mail: darkarcanis@mail.ru ICQ: 407-398-235 Jabber: arcanis@jabber.ru
On Friday 10 January 2014 20:35:45 Douglas Christman wrote:
It looks like GOPATH needed to be set for it to build correctly. Would you mind testing out the changes I made to see if it fixes the problem?
https://raw.github.com/dobyrch/termboy-go/master/PKGBUILD
On Fri, Jan 10, 2014 at 7:36 PM, Evgeniy Alekseev <darkarcanis@mail.ru>wrote:
4. It has several errors during build (could not find packages github.com/dobyrch/*). I think you must fix it in the upstream =)
Yes, now it builds normally. But anyway I think that patching of source files in the upstream (or creating a patch that does this in the prepare() function) will be better way than moving of sources to the correct path. But as you wish. And it is recommended using a patching/moving/etc in the prepare() function not build() or package(). BTW instead "src/github.com/dobyrch" will be better to use "$srcdir" variable (e.g. "$srcdir/github.com/dobyrch"). -- С уважением, Е.Алексеев. Sincerely yours, E.Alekseev. e-mail: darkarcanis@mail.ru ICQ: 407-398-235 Jabber: arcanis@jabber.ru
I created a pull request on GitHub. See: https://github.com/dobyrch/termboy-go/pull/1 --Jeremy
participants (7)
-
Anatol Pomozov
-
Douglas Christman
-
Evgeniy Alekseev
-
Isaac Dupree
-
Jason St. John
-
Jeremy Audet
-
Sam Stuewe