I agree with Allan on pretty much everything he said below. I'll add a few things too. On Sat, Dec 6, 2008 at 10:07 PM, Allan McRae <allan@archlinux.org> wrote:
Jud wrote:
--- PKGBUILD.proto 2008-12-05 23:32:33.000005000 +1000 +++ PKGBUILD.proto.new 2008-12-07 13:49:43.889569135 +1000 @@ -3,13 +3,15 @@ # NOTE: Please fill out the license field for your package! If it is unknown, # then please put 'unknown'. + # Contributor: Your Name <youremail@domain.com> + pkgname=NAME -pkgver=VERSION +pkgver=VERSION # Note: if pkgver is '0.99-10' then use an underscore, i.e. '0.99_10' pkgrel=1
Can we make this shorter. I.e "# Note: Cannot contain hyphens." This really should get documented in the man page. Agreed on both counts. Patches welcome. :P
pkgdesc="" -arch=() -url="" +url="http://ADDRESS/" +arch=('i686' 'x86_64')
Re-ordering why? Also, adding Arch's standard arch values is distro specific. We try and avoid that in the pacman code. Mostly agreed. This is a shell script so order really doesn't matter. I feel like default arch values might be nice to show what we are going for, however. I'm really impartial on this though, and I'd defer to Allan on this decision as I've made him Mr. Makepkg. :)
license=('GPL') groups=() depends=() @@ -20,17 +22,13 @@ replaces=() backup=() options=() -install= -source=($pkgname-$pkgver.tar.gz) -noextract=() -md5sums=() #generate with 'makepkg -g' +install=${pkgname}.install +source=(http://ADDRESS/TO/FILE/${pkgname}-${pkgver}.tar.gz) +md5sums=() # Generate with 'makepkg -g'
Why remove noextract? The prototype should have every field. Agreed.
build() { - cd "$srcdir/$pkgname-$pkgver" - - ./configure --prefix=/usr + cd ${srcdir}${pkgname}-${pkgver}
Still missing a / in the middle of this line and the quotes are needed for people who build in directories with spaces. And why the 6 unnecessary characters? {}{}{} is a waste IMO as you didn't really give a good reason to change what is already there. Please leave this line as-is.
+ ./configure --prefix=usr make || return 1 - make DESTDIR="$pkgdir/" install + make DESTDIR=${pkgdir} install || return 1
The trailing slash was added here deliberately as some makefiles suck. Also, quotes are needed. Yep.
We may sound harsh but we try to do reviews of every patch- don't get discouraged, but please try and incorporate our feedback. -Dan