Bug 48196 - audio/abcde minor nits
Summary: audio/abcde minor nits
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-02-12 03:20 UTC by Gregory Bond
Modified: 2003-03-31 17:39 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gregory Bond 2003-02-12 03:20:02 UTC
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.

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.

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.

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}

Add the following patchfile to the port to fix #2:



No solution to #3 tho.--gI2DS9ZqP5cnaiYZfTKJnpcufT1gpJM7P9tdKsvvP97HCAI0
Content-Type: text/plain; name="patch-eject"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="patch-eject"

--- 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
How-To-Repeat: 
	cd /usr/ports/audio/abcde && make install
	(note eject is not built)
	abcde -x
	(note eject fails)
	cd /usr/ports/sysutils/eject && make install
	abcde -x
	....
	eject: no such file or directory
Comment 1 Giorgos Keramidas freebsd_committer 2003-02-16 21:20:45 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 ---
Comment 2 Tilman Keskinoz freebsd_committer 2003-03-31 17:39:20 UTC
State Changed
From-To: open->closed

I committed the patch from Munish Chopra, thanks for reporting