[aur-general] Please Review: mock-git: Mock takes an SRPM and builds RPM packages from it in a chroot

[Ivan Shapovalov]

On Thursday 16 January 2014 at 11:25:01, Peter wrote:
> I just uploaded this tool and please you to send me your comments about
> the included PKGBUILD.
> Will this be  okay in this form or should it better be improved and what?
> 
> The check runs for a while but then ended with failures but I think
> these are somewhere in Enterprise Linux cause the fedora seems not to be
> a problem.
> 
> Thanks a lot for all your works in AUR.

-----

Please, include links to all packages you mention
to make life easier for people who would want to help you.

Here are the links for others:
https://aur.archlinux.org/packages/mock-git
https://aur.archlinux.org/packages/mo/mock-git/PKGBUILD

-----

And here is my commentary:

>  echo "Need sudo for groupadd -r mock if not already exist"
>  getent group mock &>/dev/null || sudo groupadd -r mock

This does not belong in here, does it?
prepare() is build-time, and configuring users and groups is install-time...

BTW, remove sudo in the install file - package installation is performed as 
root.

>  # fix incorrect shebang
>  sed -i 's_#!/usr/bin/python_&2_' py/mock.py
>  sed -i 's_#!/usr/bin/python_&2_' py/mockchain.py
>  sed -i 's_#!/usr/bin/python_&2_' py/mockbuild/mounts.py

As mentioned somewhere earlier in this list, better to add $ to match regexes
in order for the package not to break if the upstream decides to fix shebangs 
itself (with your regexes, python2 will turn to python22).
So, use something like "s|#!/usr/bin/python$|&2|".

>  make DESTDIR=${pkgdir} install

>  install -d ${pkgdir}/usr/bin
>  ln -s /usr/bin/consolehelper ${pkgdir}/usr/bin/mock

>  mkdir -p ${pkgdir}/var/{lib,cache}/mock
>  chown -R root:mock ${pkgdir}/var/{lib,cache}/mock
>  chmod -R 02755 ${pkgdir}/var/{lib,cache}/mock

Please double-quote all pathes containing ${pkgdir} and other variables.
Otherwise your PKGBUILD will break on whitespace-containing pathes.

Something like this:
make DESTDIR="${pkgdir}" install
install -d "${pkgdir}/usr/bin"

Otherwise your PKGBUILD is fine IMO.

Cheers!

-- 
Ivan Shapovalov / intelfx /
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 230 bytes
Desc: This is a digitally signed message part.
URL: <http://mailman.archlinux.org/pipermail/aur-general/attachments/20140116/5e0421f2/attachment.asc>