| Message ID | 20200412210129.13345-1-polyzen@archlinux.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [pacman-dev] Touch up proto files | expand |
On 4/12/20 5:01 PM, Daniel M. Capella wrote: > - split: Fix erroneous $pkgname calls to use $pkgbase It's not automatically erroneous to use $pkgname in a split package, ${pkgname[0]} is commonly the same value as $pkgbase and there's no problem using either one. You might need to be more specific for package_*() alone... but those invocations actually use pkgbase.... If this is about making the current inconsistent use of both pkgbase and pkgname all over the file, then can the commit message mention this? At the moment it is certainly no *clearer* to the user which one they might want to use, and why one might recommend avoiding the use of $pkgname... > - VCS: > > - Add missing fields What is missing about them? These proto files don't need to be a complete command reference for makepkg variables, the authoritative documentation which people actually use is PKGBUILD(5). I'd even rather we remove the unused ones entirely to match how we expect users to write production PKGBUILDs. :p At least -vcs and -split probably don't need to repeat everything which PKGBUILD.proto has prototyped, they only really need to prototype how to adapt a git source or split packaging function. > - Remove unnecessary $srcdir's This is a stylistic choice, and cd'ing to absolute paths rooted in $srcdir is useful if you do something like cd "${srcdir}"/foo do_something cd "${srcdir}"/bar do_something since cd ../bar can be difficult to follow the more times you do it. Personally, I always use $srcdir in my cd's. > - Move ./autogen.sh to prepare() Running ./autogen.sh is actually one of my pet peeves, it is flatly incorrect. GNU autotools documentation considers this to be deprecated and you should use -- and ensure the build system uses -- autoreconf when- and where- ever possible (which is nearly always, unless configure.ac is missing some builtin macro, and that should be fixed). While I agree that autogen.sh should not be run in PKGBUILD-vcs.proto's build() function, I would change it to autoreconf -fi at the same time as moving it to prepare. > - 80 char line length for comments and make license note stand out The current comments are already 80 lines, so I'm not sure what this means? > - Add ./autogen.sh to the other proto's Continuing on from the previous point about autotools autogen/autoreconf usage... For dist tarballs uploaded to ftp sites, autoreconf will have already been run, there is no need to rerun it except for VCS packages, which is why PKGBUILD.proto and PKGBUILD-split.proto don't have it at all. I do not believe we should add this as it violates the expected use of autotools for stable release tarballs.
diff --git a/proto/PKGBUILD-split.proto b/proto/PKGBUILD-split.proto index 9898ef81..6fc91a56 100644 --- a/proto/PKGBUILD-split.proto +++ b/proto/PKGBUILD-split.proto @@ -1,7 +1,8 @@ -# This is an example of a PKGBUILD for splitting packages. Use this as a -# start to creating your own, and remove these comments. For more information, -# see 'man PKGBUILD'. NOTE: Please fill out the license field for your package! -# If it is unknown, then please put 'unknown'. +# This is an example of a PKGBUILD for splitting packages. Use this as a start +# to creating your own, and remove these comments. For more information, see +# 'man PKGBUILD'. +# NOTE: Please fill out the license field for your package! If it is unknown, +# then please put 'unknown'. # Maintainer: Your Name <youremail@domain.com> pkgname=('pkg1' 'pkg2') @@ -26,14 +27,15 @@ options=() install= changelog= source=("$pkgbase-$pkgver.tar.gz" - "$pkgname-$pkgver.patch") + "$pkgbase-$pkgver.patch") noextract=() md5sums=() validpgpkeys=() prepare() { - cd "$pkgname-$pkgver" - patch -p1 -i "$srcdir/$pkgname-$pkgver.patch" + cd "$pkgbase-$pkgver" + patch -p1 -i "$srcdir/$pkgbase-$pkgver.patch" + ./autogen.sh } build() { @@ -43,7 +45,7 @@ build() { } check() { - cd "$pkgname-$pkgver" + cd "$pkgbase-$pkgver" make -k check } diff --git a/proto/PKGBUILD-vcs.proto b/proto/PKGBUILD-vcs.proto index ae9956a9..01eddf98 100644 --- a/proto/PKGBUILD-vcs.proto +++ b/proto/PKGBUILD-vcs.proto @@ -1,5 +1,6 @@ -# This is an example PKGBUILD file. Use this as a start to creating your own, -# and remove these comments. For more information, see 'man PKGBUILD'. +# This is an example of a PKGBUILD for VCS packages. Use this as a start to +# creating your own, and remove these comments. For more information, see 'man +# PKGBUILD'. # NOTE: Please fill out the license field for your package! If it is unknown, # then please put 'unknown'. @@ -10,6 +11,7 @@ pkgname=NAME-VCS # '-bzr', '-git', '-hg' or '-svn' pkgver=VERSION pkgrel=1 +epoch= pkgdesc="" arch=() url="" @@ -17,21 +19,24 @@ license=('GPL') groups=() depends=() makedepends=('VCS_PACKAGE') # 'bzr', 'git', 'mercurial' or 'subversion' +checkdepends=() provides=("${pkgname%-VCS}") conflicts=("${pkgname%-VCS}") replaces=() backup=() options=() install= +changelog= source=('FOLDER::VCS+URL#FRAGMENT') noextract=() md5sums=('SKIP') +validpgpkeys=() # Please refer to the 'USING VCS SOURCES' section of the PKGBUILD man page for # a description of each element in the source array. pkgver() { - cd "$srcdir/${pkgname%-VCS}" + cd "${pkgname%-VCS}" # The examples below are not absolute and need to be adapted to each repo. The # primary goal is to generate version numbers that will increase according to @@ -56,23 +61,23 @@ pkgver() { } prepare() { - cd "$srcdir/${pkgname%-VCS}" + cd "${pkgname%-VCS}" patch -p1 -i "$srcdir/${pkgname%-VCS}.patch" + ./autogen.sh } build() { - cd "$srcdir/${pkgname%-VCS}" - ./autogen.sh + cd "${pkgname%-VCS}" ./configure --prefix=/usr make } check() { - cd "$srcdir/${pkgname%-VCS}" + cd "${pkgname%-VCS}" make -k check } package() { - cd "$srcdir/${pkgname%-VCS}" + cd "${pkgname%-VCS}" make DESTDIR="$pkgdir/" install } diff --git a/proto/PKGBUILD.proto b/proto/PKGBUILD.proto index a2c600d5..e72b1aa8 100644 --- a/proto/PKGBUILD.proto +++ b/proto/PKGBUILD.proto @@ -33,6 +33,7 @@ validpgpkeys=() prepare() { cd "$pkgname-$pkgver" patch -p1 -i "$srcdir/$pkgname-$pkgver.patch" + ./autogen.sh } build() {
- split: Fix erroneous $pkgname calls to use $pkgbase - VCS: - Add missing fields - Remove unnecessary $srcdir's - Move ./autogen.sh to prepare() - 80 char line length for comments and make license note stand out - Add ./autogen.sh to the other proto's Signed-off-by: Daniel M. Capella <polyzen@archlinux.org> --- proto/PKGBUILD-split.proto | 18 ++++++++++-------- proto/PKGBUILD-vcs.proto | 21 +++++++++++++-------- proto/PKGBUILD.proto | 1 + 3 files changed, 24 insertions(+), 16 deletions(-)