PKGBUILD for GNUMP3d - please comment

Bebo · 2006-08-05 04:27:47

Hello,

First, sorry if I post in the wrong forum. This seemed to be the one with the most "please review my PKGBUILD" threads. Please move it if some other place is more appropriate.

I've made a PKGBUILD for GNUMP3d, a streaming server that I appreciate a lot. I've noticed that there have been a few (three to be exact) threads on GNUMP3d in the past, but these are from 2004, and since then there seems to have been no activity on this.

I'm planning to submit it to the AUR, but would like to get some comments first. I've tested it, and it seems to work fine, but I've done a few "ugly" things which seemed necessary, but I'm not sure what the Arch way is in these respects.

So, first the PKGBUILD itself:

pkgname=gnump3d
pkgver=2.9.8
pkgrel=1
pkgdesc="GNUMP3d is a streaming server for MP3s, OGG vorbis files, movies and other media formats."
install=gnump3d.install
url="http://www.gnump3d.org"
source=("http://savannah.gnu.org/download/gnump3d/gnump3d-$pkgver.tar.gz")
md5sums=('329507d7c6b3a05549fc04e223908825')

build() {
  cd $startdir/src/$pkgname-$pkgver

  make install 
        PREFIX=$startdir/pkg/usr 
        CONFDIR=$startdir/pkg/etc/gnump3d 
        CACHEDIR=$startdir/pkg/var/cache/gnump3d 
        SERVEDIR=$startdir/pkg/var/cache/gnump3d/serving 
        LOGDIR=$startdir/pkg/var/log/gnump3d 
        LIBDIR=$startdir/pkg$(perl bin/getlibdir)

  install -D -m755 $startdir/gnump3d $startdir/pkg/etc/rc.d/gnump3d

  sed -i 's|'$startdir/pkg/'|/|g' $startdir/pkg/etc/gnump3d/gnump3d.conf
}

Since the GNUMP3d Makefile doesn't care about DESTDIR, that long make install line seems necessary. The install line installs a start/stop script that I've written; more on that later. The sed line is one of the ugly things. The GNUMP3d Makefile is built a bit dynamically during install, which makes some configuration options point to directories inside $startdir/pkg - not very good, and hence this line. Is there a better/an Arch way?

As you see the PKGBUILD have an install script, gnump3d.install. Here goes:

# arg 1:  the new package version
post_install() {
    rm /usr/bin/gnump3d && ln -s /usr/bin/gnump3d2 /usr/bin/gnump3d

    echo 
    echo "----[ NOTE ]----------------------------------------------------------"
    echo "| Before starting the server, edit /etc/gnump3d/gnump3d.conf to your |"
    echo "| liking. For instance, you will probably want to change the "root"  |"
    echo "| variable so that it points to your music collection.               |"
    echo "|                                                                    |"
    echo "| When the "root" variable is set, run                               |"
    echo "|     /usr/bin/gnump3d-index                                         |"
    echo "| This will create a database in /var/cache/gnump3d/ for gnump3d.    |"
    echo "|                                                                    |"
    echo "| Per default, the server runs as "nobody", which is a Good Thing.   |"
    echo "|                                                                    |"
    echo "| To make the server start on boot, add "gnump3d" to the DAEMONS     |"
    echo "| array in /etc/rc.conf.                                             |"
    echo "----------------------------------------------------------------------"
    echo
}

op=$1
shift
$op $*

Sorry about the long NOTE That is maybe a bit ugly, but it is mostly the first line that I'm not so happy with. During the make install the GNUMP3d Makefile creates a symlink from /usr/bin/gnump3d to /usr/bin/gnump3d2. As in the case of the configuration options, this too points to a file in $startdir/pkg. So the first line here is removing it (since it is pointing at a nonexistent $startdir/pkg/usr/bin/gnump3d2 file) and then correcting it. Is there a better way to do this?

And finally, the /etc/rc.d/gnump3d start/stop script:

#!/bin/sh

. /etc/rc.conf
. /etc/rc.d/functions

get_gnump3d_pid() {
  ps -C gnump3d -o pid= -o args= | grep /usr/bin/gnump3d | awk '{print $1}' | tr 'n' ' '
}

gnump3d_start() {
  stat_busy "Starting GNUMP3d Streaming Server"
  [ "x$(get_gnump3d_pid)" == "x" ] && /usr/bin/gnump3d --quiet --background &
  if [ $? -gt 0 ]; then
    stat_fail
  else
    add_daemon gnump3d
    stat_done
  fi
}

gnump3d_stop() {
  stat_busy "Stopping GNUMP3d Streaming Server"
  for PID in $(get_gnump3d_pid) ; do
    kill $PID 2> /dev/null
  done
  if [ $? -gt 0 ]; then
    stat_fail   
  else
    rm_daemon gnump3d
    stat_done
  fi
}

gnump3d_restart() {
  gnump3d_stop
  sleep 1
  gnump3d_start
}

case "$1" in
  'start')
    gnump3d_start
    ;;
  'stop')
    gnump3d_stop
    ;;
  'restart')
    gnump3d_restart
    ;;
  *)
    echo "usage: $0 {start|stop|restart}"
esac

Pretty much Arch style I think, but the PID stuff is not very nice. killall gnump3d simply won't do, since that would kill the script itself. GNUMP3d doesn't create any pid file so I do it this way instead. I could perhaps change get_gnump3d_pid to something like

ps -C gnump3d -o pid= -o args= | awk '{print $1}' | grep -v $$ | tr 'n' ' '

but in my mind that doesn't really matter, the solution is the same anyway. So, anyone having a better idea?

Cheers!

<EDIT>
Oh, BTW. Dependencies... I did start with having perl as a dependency, but then namcap said it was unnecessary. So I removed it again. How is it, should I add these "obvious" dependencies, such as perl, bash, awk, grep, sed?
</EDIT>

tomk · 2006-08-05 09:35:15

Here's an initial response/idea, after just a brief scan of the tarball - forget the makefile and install everything yourself, using install and/or mkdir/cp commands. I've often found this to be the best option for perl/python/etc packages i.e. situations where you don't need ./configure && make. You have full control over where everything goes, and you don't have to do any kludgy fixes to undo Arch-unfriendly stuff that the Makefile forces on you.

Don't worry about the length of your post-install message - it's best to get the info out there. Just be prepared for the fact that not everyone reads these messages.

Your init script - I'd have to say it's not "Arch style", IMO. Do what you have to do to get the PID, but if you look at a typical Arch init script (I always use portmap as my typical example), you should put the start/stop/restart code under their respective options in case, rather than creating separate functions for them.

Let me know what you think of the above.

Bebo · 2006-08-05 20:11:47

Thanks for the comments, Tomk. I see what you mean that the init script is not completely Arch style (even though the output looks like it ). I had a look in for instance the sshd, cups and network scripts before/as I wrote it. The network script has things in separate functions, but that's not a real daemon script. I did it that way to avoid repeating code. Is there some "hard" argument to not do it this way, or is there only style arguments?

Thanks for the tip on losing the Makefile. It does indeed give better control. And then I get can rid of the symlink fix and setup the configuration file as it should be from the start.

About the note. Yeah, I'm very aware that people have a tendency to not care or see them (in a big update, the notes can scroll by pretty fast ). Oh well, they are installing a server, so they should be knowing what they're doing. And anyway, man gnump3d is not a very far-fetched thought

I'll make some changes and post again, but maybe not until tomorrow.

Bebo · 2006-08-06 02:34:41

Okay, here are updated scripts. PKGBUILD:

pkgname=gnump3d
pkgver=2.9.8
pkgrel=1
pkgdesc="GNUMP3d is a streaming server for MP3s, OGG vorbis files, movies and other media formats."
install=gnump3d.install
url="http://www.gnump3d.org"
source=("http://savannah.gnu.org/download/gnump3d/gnump3d-$pkgver.tar.gz")
md5sums=('329507d7c6b3a05549fc04e223908825')

build() {
    _PKGDIR=$startdir/pkg

    cd $startdir/src/$pkgname-$pkgver

    _PREFIX=$_PKGDIR/usr
    _CONFDIR=$_PKGDIR/etc/gnump3d
    _CACHEDIR=$_PKGDIR/var/cache/gnump3d
    _SERVEDIR=$_PKGDIR/var/cache/gnump3d/serving
    _LOGDIR=$_PKGDIR/var/log/gnump3d
    _LIBDIR=$_PKGDIR$(perl bin/getlibdir)

    _BINDIR=$_PREFIX/bin
    _TEMPDIR=$_PREFIX/share/gnump3d
    _MANDIR=$_PREFIX/man/man1
    _PLUGDIR=$_LIBDIR/gnump3d/plugins
    _LANGDIR=$_LIBDIR/gnump3d/lang
 
    install -d $_CONFDIR
    install -d $_BINDIR
    install -d $_TEMPDIR
    install -d $_MANDIR
    install -d $_PLUGDIR
    install -d $_LANGDIR
    install -d $_LOGDIR

    install -d -o nobody -m 755 $_CACHEDIR
    install -d -o nobody -m 755 $_SERVEDIR

    install -m 644 lib/gnump3d/*.pm         $_LIBDIR/gnump3d
    install -m 644 lib/gnump3d/plugins/*.pm $_PLUGDIR
    install -m 644 lib/gnump3d/lang/*.pm    $_LANGDIR

    install -D bin/gnump3d2      $_BINDIR/gnump3d
    install -D bin/gnump3d-top   $_BINDIR/gnump3d-top
    install -D bin/gnump3d-index $_BINDIR/gnump3d-index

    install -D -m 644 man/gnump3d-top.1   $_MANDIR
    install -D -m 644 man/gnump3d-index.1 $_MANDIR
    install -D -m 644 man/gnump3d.1       $_MANDIR
    install -D -m 644 man/gnump3d.conf.1  $_MANDIR

    install -d $_TEMPDIR
    cp -R templates/* $_TEMPDIR
    chmod -R a+r $_TEMPDIR
    chmod +rx $_TEMPDIR/*/

    sed "s|PLUGINDIR|$_LIBDIR|g" etc/gnump3d.conf | sed "s|$startdir/pkg/|/|g" > etc/gnump3d.conf.new
    install -D -m 600 etc/gnump3d.conf.new $_CONFDIR
    install -D -m 600 etc/mime.types $_CONFDIR

    install -D -m 755 $startdir/gnump3d $_PKGDIR/etc/rc.d/gnump3d
}

gnump3d.install:

# arg 1:  the new package version
post_install() {
    echo 
    echo "----[ NOTE ]----------------------------------------------------------"
    if [ -e /etc/gnump3d/gnump3d.conf ] ; then
        echo "| Since /etc/gnump3d/gnump3d.conf already exists, this package's     |" 
        echo "| configuration file is put in /etc/gnump3d/gnump3d.conf.new         |"
        echo "|                                                                    |"
    else
        mv /etc/gnump3d/gnump3d.conf.new /etc/gnump3d/gnump3d.conf
    fi
    echo "| Before starting the server, edit /etc/gnump3d/gnump3d.conf to your |"
    echo "| liking. For instance, you will probably want to change the "root"  |"
    echo "| variable so that it points to your music collection.               |"
    echo "|                                                                    |"
    echo "| When the "root" variable is properly set, run                      |"
    echo "|     /usr/bin/gnump3d-index                                         |"
    echo "| This will create a database in /var/cache/gnump3d/ for gnump3d.    |"
    echo "|                                                                    |"
    echo "| Per default, the server runs as "nobody", which is a Good Thing.   |"
    echo "|                                                                    |"
    echo "| To make the server start on boot, add "gnump3d" to the DAEMONS     |"
    echo "| array in /etc/rc.conf.                                             |"
    echo "----------------------------------------------------------------------"
    echo
}

# arg 1:  the new package version
# arg 2:  the old package version
post_upgrade() {
    if [ -e /etc/gnump3d/gnump3d.conf ] ; then
        echo
        echo "----[ NOTE ]----------------------------------------------------------"
        echo "| Since /etc/gnump3d/gnump3d.conf already exists, this package's     |"
        echo "| configuration file is put in /etc/gnump3d/gnump3d.conf.new         |"
        echo "----------------------------------------------------------------------"
        echo
    else
        mv /etc/gnump3d/gnump3d.conf.new /etc/gnump3d/gnump3d.conf
    fi
}

op=$1
shift
$op $*

gnump3d init script:

#!/bin/sh

. /etc/rc.conf
. /etc/rc.d/functions

get_gnump3d_pid() {
  ps -C gnump3d -o pid= -o args= | grep /usr/bin/gnump3d | awk '{print $1}' | tr 'n' ' '
}

case "$1" in
  start)
    stat_busy "Starting GNUMP3d Streaming Server"
    [ "x$(get_gnump3d_pid)" == "x" ] && /usr/bin/gnump3d --quiet --background &
    if [ $? -gt 0 ]; then
      stat_fail
    else
      add_daemon gnump3d
      stat_done
    fi
    ;;

  stop)
    stat_busy "Stopping GNUMP3d Streaming Server"
    for PID in $(get_gnump3d_pid) ; do
      kill $PID 2> /dev/null
    done
    if [ $? -gt 0 ]; then
      stat_fail   
    else
      rm_daemon gnump3d
      stat_done
    fi
    ;;

  restart)
    $0 stop
    sleep 1
    $0 start
    ;;

  *)
    echo "usage: $0 {start|stop|restart}"
esac

exit 0

Comments? Also, as you probably noted in my first post, I'm a bit unsure about the dependencies. Should I add perl, even though namcap thinks it is not needed?

Bebo · 2006-08-06 02:59:14

Hm, I just found the backup directive. So, I'll remove the gnump3d.conf stuff in the post_install and remove post_upgrade completely, and put a backup=('etc/gnump3d/gnump3d.conf') line in the PKGBUILD instead.

Bebo · 2006-08-07 00:50:21

Perhaps I'm a bit impatient, but since there have been no more comments, I've submitted the PKGBUILD to the AUR.

tomk · 2006-08-07 06:22:12

OK Bebo - I got caught up with other things yesterday. I have some more feedback for you, so I'll put it in AUR comments later. For now, I would recommend you have another look at the Arch Packaging Standards, particularly the section dealing with custom variables. Also, with the exception of the install script, all files required to build the package should be listed in the source array, with corresponding entries in the md5sum array.

Bebo · 2006-08-07 16:34:37

Hello again Tomk,

Geez, I thought there would be things to comment on, but I didn't think I would screw up that much. My sincere apologies. And thanks again for your comments.

Tonight, I will remove the custom variables in the PKGBUILD and add the md5sum of the init script. I thought the custom variables were OK, since I prepended them with an underscore - but I missed or rather forgot the part that they had to be absolutely necessary. And the md5sum of the init script, that's just pure negligence on my part

I wasn't entirely happy with the getlibdir script that gnump3d uses. Good to hear that there is a simple solution to this. So, I'll use /usr/lib/perl5/current instead.

Finally, I am interested in your comments on tidiness and efficiency. I script a lot in bash, but there are probably things I do in odd ways - or at least (obviously ) not the Arch way.

Cheers!

Bebo · 2006-08-07 19:13:19

Hello again,

I've changed the PKGBUILD to better comply with the Arch Packaging Standards. I have already uploaded the new version; I really would have liked to give some time for comments first, but there was a permission error in the first version, so I wanted to be fast. It made /var/cache be owned by nobody, which isn't correct.

I've been so sloppy this far, I think I should never ever submit a package again... :?

tomk · 2006-08-07 22:22:17

Don't say that - seriously. You asked for comments, got them, and acted accordingly - so you won't be making those mistakes again. The AUR needs contributors like you.

Keep up the good work, and I look forward to your next package.

Bebo · 2006-08-07 23:00:35

Thank you, Tomk, now I feel a bit better Was kind of embarrassed...

Hm, about the permission issue; what is the best method to create a directory owned by for instance nobody in a package build? I mean, using install -d -o nobody <dir> is not very good, since some people may have another user id (not 99) as nobody. Should this kind of stuff be put in post_install or what do you usually do?

tomk · 2006-08-07 23:56:08

Yeah, post-install is generally the best place for that kind of stuff. Just add the required chown command before your message.

Arch Linux

#1 2006-08-05 04:27:47

PKGBUILD for GNUMP3d - please comment

#2 2006-08-05 09:35:15

Re: PKGBUILD for GNUMP3d - please comment

#3 2006-08-05 20:11:47

Re: PKGBUILD for GNUMP3d - please comment

#4 2006-08-06 02:34:41

Re: PKGBUILD for GNUMP3d - please comment

#5 2006-08-06 02:59:14

Re: PKGBUILD for GNUMP3d - please comment

#6 2006-08-07 00:50:21

Re: PKGBUILD for GNUMP3d - please comment

#7 2006-08-07 06:22:12

Re: PKGBUILD for GNUMP3d - please comment

#8 2006-08-07 16:34:37

Re: PKGBUILD for GNUMP3d - please comment

#9 2006-08-07 19:13:19

Re: PKGBUILD for GNUMP3d - please comment

#10 2006-08-07 22:22:17

Re: PKGBUILD for GNUMP3d - please comment

#11 2006-08-07 23:00:35

Re: PKGBUILD for GNUMP3d - please comment

#12 2006-08-07 23:56:08

Re: PKGBUILD for GNUMP3d - please comment

Board footer