Summary: | audio/abcde minor nits | ||
---|---|---|---|
Product: | Ports & Packages | Reporter: | Gregory Bond <gnb> |
Component: | Individual Port(s) | Assignee: | freebsd-ports-bugs (Nobody) <ports-bugs> |
Status: | Closed FIXED | ||
Severity: | Affects Only Me | CC: | mchopra |
Priority: | Normal | ||
Version: | Latest | ||
Hardware: | Any | ||
OS: | Any |
Description
Gregory Bond
2003-02-12 03:20:02 UTC
## Adding to the audit trail: : Message-Id: <20030212041807.GA52444@opiate.thirteenandtwo.org> : Date: Tue, 11 Feb 2003 23:18:07 -0500 : From: Munish Chopra <mchopra@engmail.uwaterloo.ca> : In-Reply-To: <200302120313.h1C3Djor045253@hellcat.itga.com.au> : : On 2003-02-12 14:13 +0000, Gregory Bond wrote: : > The abcde port has a couple of minor problems: : > : > 1) The -x option (for "eject cd at the end") relies on the sysutils/eject port : > being installed, but that is not listed in the dependencies. : : I've never noticed this, thanks for pointing it out. : : > 2) Even if eject is installed, the script doesn't eject (eject : > prints "no such file or directory") due to a clash over the use of the EJECT : > environment variable between eject and abcd. The attached patch fixes that. : : Great, thanks. : : > 3) The abcde.conf file is set up to use dagrab, even if that isn't defined : > when the port is built. I don't know enough about port hacking to work out : > how to patch the default config based on whether makefile variables : > are defined, so I can't offer a fix for this. : : The last PORTREVISION bump (to revision 3) on January 11 included a : dependency on cdparanoia, and modified the abcde.conf file to point to : cdparanoia as the default, which it should be even if it's not : explicitly set. Unfortunately I forgot that I had previously modified : abcde itself to point to dagrab by default. : : > >Fix: : > The following patch fixed #1: : > : > Index: Makefile : > =================================================================== : > RCS file: /usr/ncvs/ports/audio/abcde/Makefile,v : > retrieving revision 1.5 : > diff -u -r1.5 Makefile : > --- Makefile 11 Jan 2003 23:44:11 -0000 1.5 : > +++ Makefile 12 Feb 2003 03:11:20 -0000 : > @@ -17,7 +17,8 @@ : > RUN_DEPENDS= cd-discid:${PORTSDIR}/audio/cd-discid \ : > cdparanoia:${PORTSDIR}/audio/cdparanoia \ : > id3v2:${PORTSDIR}/audio/id3v2 \ : > - oggenc:${PORTSDIR}/audio/vorbis-tools : > + oggenc:${PORTSDIR}/audio/vorbis-tools \ : > + eject:${PORTSDIR}/sysutils/eject : > : > NO_BUILD= yes : > WRKSRC= ${WRKDIR}/${PORTNAME}-${PORTVERSION} : : I'm not a big fan of doing it this way. The majority of users don't seem : to be using the -x option - you're the first one to mention the missing : eject dependency. I'd rather see a Makefile knob to add the possibility : to use eject. : : > Add the following patchfile to the port to fix #2: : > : > --- patch-eject begins here --- : > --- abcde.orig Tue Feb 11 16:39:41 2003 : > +++ abcde Wed Feb 12 13:54:33 2003 : > @@ -1190,7 +1190,13 @@ : > # We are now finished with the cdrom - it can be safely ejected. Note that : > # abcde will not have completed yet. : > if [ "$EJECTCD" = "y" ]; then : > - $EJECT $EJECTOPTS $CDROM : > + # FreeBSD eject uses the EJECT environment variable to name the CDROM : > + # but in this script EJECT is in the envionment and names the program : > + eject=$EJECT : > + unset EJECT : > + # The FreeBSD eject needs "adc0" not "/dev/adc0c" : > + cd="$(echo $CDROM | sed -e 's=.*/==;s=[a-h]$==;')" : > + $eject $EJECTOPTS $cd : > fi : > ) | ( : > # Do the encoding, including parallelization of remote encoding : > --- patch-eject ends here --- : : I'm fine with this. : : > No solution to #3 tho. : : I have attached a patch to address all these problems, including #3. It : does the following: : : o Add WITH_EJECT knob : o Fold in Gregory Bond's patch to make eject work on FreeBSD : o Reset default ripper to cdparanoia, even with no abcde.conf to read : o Bump PORTREVISION : : Please let me know if there's anything else. : : -- : Munish Chopra : : --- start patch --- : diff -ruN /usr/ports/audio/abcde/Makefile abcde/Makefile : --- /usr/ports/audio/abcde/Makefile Sun Jan 12 06:15:09 2003 : +++ abcde/Makefile Tue Feb 11 22:45:59 2003 : @@ -7,7 +7,7 @@ : : PORTNAME= abcde : PORTVERSION= 2.0.3 : -PORTREVISION= 3 : +PORTREVISION= 4 : CATEGORIES= audio : MASTER_SITES= http://frantica.lly.org/~rcw/abcde/ : DISTNAME= abcde_2.0.3.orig : @@ -35,6 +35,13 @@ : DAGRAB_MSG= "Define WITH_DAGRAB to enable support for dagrab." : .endif : : +.if defined(WITH_EJECT) : +RUN_DEPENDS+= eject:${PORTSDIR}/sysutils/eject : +.else : +EJECT_MSG= "Define WITH_EJECT to enable auto-eject support." : +.endif : + : + : pre-fetch: : .if defined(DAGRAB_MSG) : @${ECHO_MSG} ${DAGRAB_MSG} : @@ -44,6 +51,11 @@ : @${ECHO_MSG} ${CDDA2WAV_MSG} : @${ECHO_MSG} "" : .endif : +.if defined(EJECT_MSG) : + @${ECHO_MSG} ${EJECT_MSG} : + @${ECHO_MSG} "" : +.endif : + : : do-install: : ${INSTALL_SCRIPT} ${WRKSRC}/abcde ${PREFIX}/bin : diff -ruN /usr/ports/audio/abcde/files/patch-aa abcde/files/patch-aa : --- /usr/ports/audio/abcde/files/patch-aa Mon Aug 5 16:57:08 2002 : +++ abcde/files/patch-aa Tue Feb 11 23:09:07 2003 : @@ -1,5 +1,5 @@ : ---- abcde.orig Mon Aug 5 16:56:33 2002 : -+++ abcde Mon Aug 5 16:53:52 2002 : +--- abcde.orig Mon Jan 28 00:44:02 2002 : ++++ abcde Tue Feb 11 23:08:44 2003 : @@ -534,7 +534,7 @@ : ;; : 12|13|14) : @@ -46,15 +46,6 @@ : esac : RETURN=$? : if [ "$RETURN" != "0" ]; then : -@@ -886,7 +887,7 @@ : - CDDBSUBMIT=freedb-submit@freedb.org : - HELLOINFO="$(whoami)@$(hostname)" : - INTERACTIVE=y : --CDROMREADERSYNTAX=cdparanoia : -+CDROMREADERSYNTAX=dagrab : - OUTPUTTYPE=ogg : - ENCODERSYNTAX=default : - OUTPUTFORMAT='${ARTISTFILE}/${TRACKFILE}.$OUTPUTTYPE' : @@ -913,7 +914,8 @@ : ID3V2=id3v2 : CDPARANOIA=cdparanoia : @@ -105,7 +96,21 @@ : do : # Cut off the command-line options we just added in : X=$(echo $X | cut -d' ' -f2) : -@@ -1191,7 +1197,7 @@ : +@@ -1184,14 +1190,20 @@ : + # We are now finished with the cdrom - it can be safely ejected. Note that : + # abcde will not have completed yet. : + if [ "$EJECTCD" = "y" ]; then : +- $EJECT $EJECTOPTS $CDROM : ++ # FreeBSD eject uses the EJECT environment variable to name the CDROM : ++ # but in this script EJECT is in the envionment and names the program : ++ eject=$EJECT : ++ unset EJECT : ++ # The FreeBSD eject needs "adc0" not "/dev/adc0c" : ++ cd="$(echo $CDROM | sed -e 's=.*/==;s=[a-h]$==;')" : ++ $eject $EJECTOPTS $cd : + fi : + ) | ( : + # Do the encoding, including parallelization of remote encoding : # Figure out where each track is going to be encoded : ENCODELOCATIONS="$(echo $REMOTEHOSTS | tr , ' ')" : if [ "$MAXPROCS" != "0" ]; then : : --- end patch --- State Changed From-To: open->closed I committed the patch from Munish Chopra, thanks for reporting |