Arch Linux

yamad

Member

Registered: 2010-12-06

Posts: 8

PKGBUILD critique: cweb

Hi all,

Planning on submitting a few packages to AUR for the first time, so I thought a review was in order. Here's the cweb one:

PKGBUILD

# Maintainer: Jason Yamada-Hanff <jyamada1 at gmail dot com>

pkgname=cweb
pkgver=3.64
pkgrel=1
pkgdesc="Literate programming tools for structured documentation"
arch=('any')
url="http://www-cs-faculty.stanford.edu/~uno/cweb.html"
license=('custom')
depends=(glibc)
source=("ftp://ftp.cs.stanford.edu/pub/cweb/$pkgname.tar.gz"
        "Makefile.patch")
md5sums=('8e488ec9932c117908c92d1c4614cdee'
         'b3dc92683fa52dba2f3d3ff07a437dd9')

build() {
    cd ${srcdir}
    sed -n '/Copyright/,/ levy@math.berkeley.edu./p' README > LICENSE
    sed -i 's/^% //' LICENSE
    patch -p0 < ${startdir}/Makefile.patch
    make all
}

package() {
    cd ${srcdir}
    install -Dm644 LICENSE "${pkgdir}/usr/share/licenses/$pkgname/LICENSE"
    make DESTDIR="${pkgdir}" install
}

Makefile.patch

--- Makefile	2003-12-28 16:05:31.000000000 -0800
+++ Makefile.new	2015-08-25 12:51:39.480019077 -0700
@@ -17,21 +17,25 @@
 # Read the README file, then edit this file to reflect local conditions
 #

+INSTALL=/usr/bin/install
+INSTALL_PROGRAM=$(INSTALL)
+INSTALL_DATA=$(INSTALL)
+
 # directory for TeX inputs (cwebmac.tex goes here)
 MACROSDIR= /usr/share/texmf/tex/generic

 # directory for CWEB inputs in @i files
-CWEBINPUTS= /usr/local/lib/cweb
+CWEBINPUTS= /usr/lib/cweb

 # extension for manual pages ("l" distinguishes local from system stuff)
-MANEXT= l
-#MANEXT= 1
+#MANEXT= l
+MANEXT= 1

 # directory for manual pages (cweb.1 goes here)
 MANDIR= /usr/share/man/man$(MANEXT)

 # destination directory for executables; must end in /
-DESTDIR= /usr/local/bin/
+BINDIR= /usr/bin

 # directory for GNU EMACS Lisp code (cweb.el goes here)
 EMACSDIR= /usr/share/emacs/site-lisp
@@ -163,23 +167,12 @@
 	  *.log *.dvi *.toc *.idx *.scn *.pdf core cweave ctangle

 install: all
-	- mkdir $(DESTDIR)
-	$(CP) cweave $(DESTDIR)$(DESTPREF)weave
-	chmod 755 $(DESTDIR)$(DESTPREF)weave
-	$(CP) ctangle $(DESTDIR)$(DESTPREF)tangle
-	chmod 755 $(DESTDIR)$(DESTPREF)tangle
-	- mkdir $(MANDIR)
-	$(CP) cweb.1 $(MANDIR)/cweb.$(MANEXT)
-	chmod 644 $(MANDIR)/cweb.$(MANEXT)
-	- mkdir $(MACROSDIR)
-	$(CP) cwebmac.tex $(MACROSDIR)
-	chmod 644 $(MACROSDIR)/cwebmac.tex
-	- mkdir $(EMACSDIR)
-	$(CP) cweb.el $(EMACSDIR)
-	chmod 644 $(EMACSDIR)/cweb.el
-	- mkdir $(CWEBINPUTS)
-	$(CP) c++lib.w $(CWEBINPUTS)
-	chmod 644 $(CWEBINPUTS)/c++lib.w
+	$(INSTALL_PROGRAM) -Dm755 cweave $(DESTDIR)$(BINDIR)/$(DESTPREF)weave
+	$(INSTALL_PROGRAM) -Dm755 ctangle $(DESTDIR)$(BINDIR)/$(DESTPREF)tangle
+	$(INSTALL_DATA) -Dm644 cweb.1 $(DESTDIR)$(MANDIR)/cweb.$(MANEXT)
+	$(INSTALL_DATA) -Dm644 cwebmac.tex $(DESTDIR)$(MACROSDIR)/cwebmac.tex
+	$(INSTALL_DATA) -Dm644 cweb.el $(DESTDIR)$(EMACSDIR)/cweb.el
+	$(INSTALL_DATA) -Dm644 c++lib.w $(DESTDIR)$(CWEBINPUTS)/c++lib.w

 floppy: $(ALL) examples
 	bar cvhf /dev/rfd0 $(ALL) examples

Thanks for your feedback.

Scimmia

Fellow

Registered: 2012-09-01

Posts: 11,553

Re: PKGBUILD critique: cweb

This seems to build binaries, so arch=('any') is probably wrong. 'any' means the final package isn't architecture specific, usually either data or scripts.

cd ${srcdir} is a no-op. Functions are guaranteed to start in srcdir.

Patching should be done in a separate prepare function. The sed commands most likely belong in the package function (and could be combined), but could work in the prepare function.

Never use $startdir.

Build fails:

==> Starting build()...
patching file Makefile
cc -g -D_FORTIFY_SOURCE=2  -c -o ctangle.o ctangle.c
cc -g -DCWEBINPUTS=\"/usr/lib/cweb\" -c common.c
./ctangle cweave
make: ./ctangle: Command not found
Makefile:148: recipe for target 'cweave.c' failed
make: *** [cweave.c] Error 127

Edit: Build failure is a race condition. I would suggest forcing make to only use one job with -j 1

Last edited by Scimmia (2015-08-26 01:39:32)

Xyne

Administrator/PM

Registered: 2008-08-03

Posts: 6,963

Website

Re: PKGBUILD critique: cweb

Use quotes around all variables to prevent word expansion, especially those that you do not control such as "srcdir".

Functions are guaranteed to start in srcdir.

Is this documented and guaranteed for all future versions?

---

My Arch Linux Stuff • Forum Etiquette • Community Ethos - Arch is not for everyone

Scimmia

Fellow

Registered: 2012-09-01

Posts: 11,553

Re: PKGBUILD critique: cweb

Functions are guaranteed to start in srcdir.

Is this documented and guaranteed for all future versions?

https://projects.archlinux.org/pacman.g … UILD.5.txt
Line 378

Xyne

Administrator/PM

Registered: 2008-08-03

Posts: 6,963

Website

Re: PKGBUILD critique: cweb

Thanks.

---

My Arch Linux Stuff • Forum Etiquette • Community Ethos - Arch is not for everyone

yamad

Member

Registered: 2010-12-06

Posts: 8

Re: PKGBUILD critique: cweb

Ok, thanks for all the feedback. I've made the suggested changes and added a check function. Passes namcap with no complaints now.

I opted to put the sed commands in prepare. Is there a convention for which function to use to generate a custom license? I also kept the sed operations separate because it is more obvious to a sed-newbie like me what is going on this way.

# Maintainer: Jason Yamada-Hanff <jyamada1 at gmail dot com>

pkgname=cweb
pkgver=3.64
pkgrel=1
pkgdesc="Literate programming tools for structured documentation"
arch=('i686' 'x86_64')
url="http://www-cs-faculty.stanford.edu/~uno/cweb.html"
license=('custom')
depends=('glibc')
source=("ftp://ftp.cs.stanford.edu/pub/cweb/$pkgname.tar.gz"
        "Makefile.patch")
md5sums=('8e488ec9932c117908c92d1c4614cdee'
         'b3dc92683fa52dba2f3d3ff07a437dd9')

prepare() {
  patch -p0 < Makefile.patch
  sed -n '/Copyright/ ,/levy@math.berkeley.edu./p' README > LICENSE
  sed -i 's/^% //' LICENSE

}

build() {
  make -j 1 all
}

check() {
  make cautiously
}

package() {
  install -Dm644 LICENSE "${pkgdir}/usr/share/licenses/${pkgname}/LICENSE"
  make DESTDIR="${pkgdir}" install
}

I also realized that there is a deprecated cweb package here http://pkgbuild.com/git/aur-mirror.git/tree/cweb . Ultimately does things differently than this one and appears to have been abandoned in the AUR4 move. I've contacted the maintainer of the old package.

Scimmia

Fellow

Registered: 2012-09-01

Posts: 11,553

Re: PKGBUILD critique: cweb

Ok, thanks for all the feedback. I've made the suggested changes and added a check function. Passes namcap with no complaints now.

Looks good, I don't see any obvious issues left.

I opted to put the sed commands in prepare. Is there a convention for which function to use to generate a custom license? I also kept the sed operations separate because it is more obvious to a sed-newbie like me what is going on this way.

I mostly see things like this in the package function, but that's when it's put directly into $pkgdir instead of making a file in $srcdir. Making a file like you are doing makes sense in prepare. Overall it has the same effect so I don't think it matters too much. Nothing wrong with keeping the sed command separate, either, making sense to the person maintaining it is what is most important.

severach

Member

Registered: 2015-05-23

Posts: 192

Re: PKGBUILD critique: cweb

Patches that affect the build go into prepare. Patches that don't can be in prepare but it's often better to put them in package. My preference would be to patch LICENSE after it's been placed in $pkgdir, unless, of course, the build actually used it.

I keep sed commands separate with the liberal use of -e.