Bug 87396

Summary: Fix bsd.port.mk variable quoting issues
Product: Ports & Packages Reporter: Bill Fenner <fenner>
Component: Individual Port(s)Assignee: Port Management Team <portmgr>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description Bill Fenner 2005-10-13 19:30:16 UTC
	
When a variable like BROKEN or DEPRECATED has shell metacharacters,
it needs a certain amount of quoting.  That amount varies by
variable; e.g., BROKEN="hello (parenthesis)" but
DEPRECATED=""hello (parenthesis)"" or DEPRECATED=hello (parenthesis) .

Fix: Use ${___:Q} to call ${ECHO_MSG} for IGNORE, DEPRECATED and NO_PACKAGE.
This means that make takes care of quoting, so no quoting is necessary
either in the Makefile or on the command line.  Affected variables
include:

BROKEN
FORBIDDEN
IGNORE
MANUAL_PACKAGE_BUILD
NO_CDROM
NO_PACKAGE
RESTRICTED

- Tested with "make check-sanity" on the whole ports tree.

- Also tested with a custom BROKEN=>/etc/passwd `rm -rf *`
  which echoed just as written, and BROKEN=(shell) |meta| [chars]

- Also happens to improve the behavior of java/jdk13 - when it set
  FORBIDDEN, since printf got multiple args it would say things like

fenestro% make WITH_PLUGIN=YES check-sanity
printf: missing format character
===>  jdk-1.3.1p9_5 is*** Error code 1

Stop in /big/bsd-port-mk-quoting/ports/java/jdk13.

  but with this patch it prints a message with no trailing eol:

fenestro% make WITH_PLUGIN=YES check-sanity
===>  jdk-1.3.1p9_5 is forbidden: Vulnerabilities in the browser plugin.fenestro% 

  which seems much better from a usability standpoint.


NOTE that with this patch, some ports will echo messages like
foo-1.2.3 is broken: "here is the reason"

However, this is just cosmetic - there is no combination of quoting
and metacharacters that will cause the build to fail, unlike the
existing delicate situation.
How-To-Repeat: 	

Run "make check-status" with a port Makefile with

BROKEN=()|[]<>

or

DEPRECATED="()|[]<>"
Comment 1 Erwin Lansing freebsd_committer freebsd_triage 2005-10-13 19:36:59 UTC
Responsible Changed
From-To: freebsd-ports-bugs->portmgr

Over to portmgr
Comment 2 Mark Linimon freebsd_committer freebsd_triage 2005-12-11 06:54:01 UTC
State Changed
From-To: open->feedback

The patch no longer applies: hunks 4-7 don't work.  This is because 4, 5, and 
6 were patching code that is now in bsd.database.mk, and 7 was patching code 
that has been removed as obsolete. 

The patch is good-to-go as it is but the submitter might want to check the 
current state of the code in bsd.database.mk because it does not look right 
to me.  However, IMHO that should not hold up the adopting of this PR.
Comment 3 Bill Fenner 2005-12-12 19:00:13 UTC
Here is an updated patch.  I tested it with "make check-sanity"
on the whole ports tree (and took out the extraneous bsd.subdir.mk
diff that adds check-sanity to TARGETS; it was handy for testing
but is not part of this patch)

Index: bsd.apache.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.apache.mk,v
retrieving revision 1.6
diff -u -r1.6 bsd.apache.mk
--- bsd.apache.mk	7 Dec 2005 20:54:41 -0000	1.6
+++ bsd.apache.mk	12 Dec 2005 16:37:36 -0000
@@ -52,7 +52,7 @@
 .if defined(AP_PORT_IS_SERVER)
 # For slave ports:
 .if defined(SLAVE_DESIGNED_FOR) && ${PORTVERSION} != ${SLAVE_DESIGNED_FOR}
-IGNORE=	"Sorry, ${SLAVENAME} and ${PORTNAME} versions are out of sync"
+IGNORE=	Sorry, ${SLAVENAME} and ${PORTNAME} versions are out of sync
 .endif
 
 .if defined(SLAVE_PORT_MODULES)
Index: bsd.autotools.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.autotools.mk,v
retrieving revision 1.20
diff -u -r1.20 bsd.autotools.mk
--- bsd.autotools.mk	30 Nov 2005 18:21:38 -0000	1.20
+++ bsd.autotools.mk	12 Dec 2005 16:36:36 -0000
@@ -76,7 +76,7 @@
 #
 .if ${USE_AUTOTOOLS_COMPAT}!=""
 . if defined(USE_AUTOTOOLS)
-BROKEN+=	"Mix and match of old and new autotools system prohibited"
+BROKEN+=	Mix and match of old and new autotools system prohibited
 . else
 USE_AUTOTOOLS=	${USE_AUTOTOOLS_COMPAT}
 _AUTOTOOLS_PN=	${.CURDIR:C/${PORTSDIR}\///}
@@ -166,7 +166,7 @@
 # Make sure we specified a legal version of automake
 #
 . if !exists(${PORTSDIR}/devel/automake${AUTOMAKE_VERSION}/Makefile)
-BROKEN+=	"Unknown AUTOMAKE version: ${AUTOMAKE_VERSION}"
+BROKEN+=	Unknown AUTOMAKE version: ${AUTOMAKE_VERSION}
 . endif
 
 # Set up the automake environment
@@ -212,7 +212,7 @@
 # Make sure we specified a legal version of autoconf
 #
 . if !exists(${PORTSDIR}/devel/autoconf${AUTOCONF_VERSION}/Makefile)
-BROKEN+=	"Unknown AUTOCONF version: ${AUTOCONF_VERSION}"
+BROKEN+=	Unknown AUTOCONF version: ${AUTOCONF_VERSION}
 . endif
 
 # Set up the autoconf/autoheader environment
@@ -260,7 +260,7 @@
 # Make sure we specified a legal version of libtool
 #
 . if !exists(${PORTSDIR}/devel/libtool${LIBTOOL_VERSION}/Makefile)
-BROKEN+=	"Unknown LIBTOOL version: ${LIBTOOL_VERSION}"
+BROKEN+=	Unknown LIBTOOL version: ${LIBTOOL_VERSION}
 . endif
 
 # Set up the libtool environment
Index: bsd.database.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.database.mk,v
retrieving revision 1.3
diff -u -r1.3 bsd.database.mk
--- bsd.database.mk	4 Dec 2005 14:05:14 -0000	1.3
+++ bsd.database.mk	12 Dec 2005 16:36:55 -0000
@@ -102,7 +102,7 @@
 .endif # BROKEN_WITH_MYSQL
 LIB_DEPENDS+=	mysqlclient.${MYSQL${MYSQL_VER}_LIBVER}:${PORTSDIR}/databases/mysql${MYSQL_VER}-client
 .else
-IGNORE=		"Unknown MySQL version: ${MYSQL_VER}"
+IGNORE=		Unknown MySQL version: ${MYSQL_VER}
 .endif # Check for correct libs
 .endif # USE_MYSQL
 
@@ -135,13 +135,13 @@
 .if defined(BROKEN_WITH_PGSQL)
 .	for VER in ${BROKEN_WITH_PGSQL}
 .		if (${PGSQL_VER} == "${VER}")
-IGNORE=		"Does not work with postgresql${PGSQL_VER}-client PostgresSQL \(${BROKEN_WITH_PGSQL} not supported\)"
+IGNORE=		Does not work with postgresql${PGSQL_VER}-client PostgresSQL (${BROKEN_WITH_PGSQL} not supported)
 .		endif
 .	endfor
 .endif # BROKEN_WITH_PGSQL
 LIB_DEPENDS+=	pq.${PGSQL${PGSQL_VER}_LIBVER}:${PORTSDIR}/databases/postgresql${PGSQL_VER}-client
 .else
-IGNORE=		"Unknown PostgreSQL version: ${PGSQL_VER}"
+IGNORE=		Unknown PostgreSQL version: ${PGSQL_VER}
 .endif # Check for correct version
 CPPFLAGS+=		-I${LOCALBASE}/include
 LDFLAGS+=		-L${LOCALBASE}/lib
@@ -206,7 +206,7 @@
 
 # USE_BDB is specified incorrectly, so mark this as IGNORE
 .if ${_FOUND} == "no"
-IGNORE=	"Unknown bdb version: ${USE_BDB}"
+IGNORE=	Unknown bdb version: ${USE_BDB}
 .endif
 
 .endif # USE_BDB
@@ -226,7 +226,7 @@
 .elif ${_SQLITE_VER} == "2"
 LIB_DEPENDS+=	sqlite.${_SQLITE_VER}:${PORTSDIR}/databases/sqlite${_SQLITE_VER}
 .else
-IGNORE=	"Unknown sqlite version: ${_SQLITE_VER}"
+IGNORE=	Unknown sqlite version: ${_SQLITE_VER}
 .endif
 
 .endif # defined(USE_SQLITE)
Index: bsd.gcc.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.gcc.mk,v
retrieving revision 1.5
diff -u -r1.5 bsd.gcc.mk
--- bsd.gcc.mk	31 Jul 2005 17:07:23 -0000	1.5
+++ bsd.gcc.mk	12 Dec 2005 16:36:36 -0000
@@ -77,7 +77,7 @@
 .endfor
 
 .if !defined(_GCCVERSION_OKAY)
-BROKEN=	"Unknown version of GCC specified (USE_GCC=${USE_GCC})"
+BROKEN=	Unknown version of GCC specified (USE_GCC=${USE_GCC})
 .endif
 
 #
@@ -93,7 +93,7 @@
 . endif
 .endfor
 .if !defined(_GCCVERSION)
-BROKEN=		"Couldn't find your current GCCVERSION (OSVERSION=${OSVERSION})"
+BROKEN=		Couldn't find your current GCCVERSION (OSVERSION=${OSVERSION})
 .endif
 
 #
Index: bsd.gnome.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.gnome.mk,v
retrieving revision 1.99
diff -u -r1.99 bsd.gnome.mk
--- bsd.gnome.mk	16 Nov 2005 16:50:15 -0000	1.99
+++ bsd.gnome.mk	12 Dec 2005 16:38:28 -0000
@@ -602,7 +602,7 @@
 .         endif
 .      endif
 .  if ${_USE_GNOME_ALL:M${component}}==""
-BROKEN=	"Unknown component ${component}"
+BROKEN=	Unknown component ${component}
 .  endif
 _USE_GNOME+=	${${component}_USE_GNOME_IMPL} ${component}
 . endfor
@@ -628,7 +628,7 @@
 						done;
 .else
 .if ${USE_GNOME:Mltverhack}!=""
-BROKEN= "${PORTNAME} uses the ltverhack GNOME component but does not use libtool"
+BROKEN= ${PORTNAME} uses the ltverhack GNOME component but does not use libtool
 .endif
 .endif
 
Index: bsd.gstreamer.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.gstreamer.mk,v
retrieving revision 1.12
diff -u -r1.12 bsd.gstreamer.mk
--- bsd.gstreamer.mk	15 Sep 2005 20:22:08 -0000	1.12
+++ bsd.gstreamer.mk	12 Dec 2005 16:36:36 -0000
@@ -237,7 +237,7 @@
 RUN_DEPENDS+=   ${_GST_LIB_BASE}/libgst${ext}.so:${PORTSDIR}/${${ext}_DEPENDS}
 .  endif
 . else
-BROKEN=	"Unknown gstreamer-plugin -- ${ext}"
+BROKEN=	Unknown gstreamer-plugin -- ${ext}
 . endif
 .endfor
 
Index: bsd.kde.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.kde.mk,v
retrieving revision 1.53
diff -u -r1.53 bsd.kde.mk
--- bsd.kde.mk	5 Nov 2005 15:55:24 -0000	1.53
+++ bsd.kde.mk	12 Dec 2005 16:36:36 -0000
@@ -68,7 +68,7 @@
 USE_QT_VER=		3
 PREFIX=			${KDE_PREFIX}
 .else
-BROKEN=			"Unknown value in USE_KDELIBS_VER"
+BROKEN=			Unknown value in USE_KDELIBS_VER
 .endif # ${USE_KDELIBS_VER} == 3
 .endif # defined(USE_KDELIBS_VER)
 
@@ -122,7 +122,7 @@
 CONFIGURE_ENV+=	MOC="${MOC}" CPPFLAGS="${CPPFLAGS} ${QTCPPFLAGS}" LIBS="${QTCFGLIBS}"
 .endif # !defined(QT_NONSTANDARD)
 .else
-BROKEN="Unsupported value of USE_QT_VER"
+BROKEN=Unsupported value of USE_QT_VER
 .endif # defined(USE_QT_VER)
 
 # End of USE_QT_VER section
Index: bsd.php.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.php.mk,v
retrieving revision 1.20
diff -u -r1.20 bsd.php.mk
--- bsd.php.mk	20 Nov 2005 20:35:40 -0000	1.20
+++ bsd.php.mk	12 Dec 2005 16:36:36 -0000
@@ -69,7 +69,7 @@
 .if defined(BROKEN_WITH_PHP)
 .	for VER in ${BROKEN_WITH_PHP}
 .		if ${PHP_VER} == "${VER}"
-BROKEN=		"Doesn't work with PHP version : ${PHP_VER} (Doesn't support PHP ${BROKEN_WITH_PHP})"
+BROKEN=		Doesn't work with PHP version : ${PHP_VER} (Doesn't support PHP ${BROKEN_WITH_PHP})
 .		endif
 .	endfor
 .endif
Index: bsd.port.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.port.mk,v
retrieving revision 1.518
diff -u -r1.518 bsd.port.mk
--- bsd.port.mk	8 Nov 2005 09:02:51 -0000	1.518
+++ bsd.port.mk	12 Dec 2005 16:36:36 -0000
@@ -1171,7 +1171,7 @@
 
 .if defined(PORTVERSION)
 .if ${PORTVERSION:M*[-_,]*}x != x
-BROKEN=			"PORTVERSION ${PORTVERSION} may not contain '-' '_' or ','"
+BROKEN=			PORTVERSION ${PORTVERSION} may not contain '-' '_' or ','
 .endif
 DISTVERSION?=	${PORTVERSION:S/:/::/g}
 .elif defined(DISTVERSION)
@@ -1490,7 +1490,7 @@
 .elif ${WANT_OPENLDAP_VER} == 23
 LIB_DEPENDS+=		ldap-2.3.1:${PORTSDIR}/net/openldap23${_OPENLDAP_FLAVOUR}-client
 .else
-BROKEN=				"unknown OpenLDAP version: ${WANT_OPENLDAP_VER}"
+BROKEN=				unknown OpenLDAP version: ${WANT_OPENLDAP_VER}
 .endif
 .endif
 
@@ -1500,7 +1500,7 @@
 .elif ${WANT_FAM_SYSTEM} == gamin
 LIB_DEPENDS+=	fam.0:${PORTSDIR}/devel/gamin
 .else
-BROKEN=			"unknown FAM system: ${WANT_FAM_SYSTEM}"
+BROKEN=			unknown FAM system: ${WANT_FAM_SYSTEM}
 .endif
 .endif
 
@@ -2770,32 +2770,32 @@
 
 .if !defined(__ARCH_OK)
 .if defined(ONLY_FOR_ARCHS)
-IGNORE=		"is only for ${ONLY_FOR_ARCHS},"
+IGNORE=		is only for ${ONLY_FOR_ARCHS},
 .else # defined(NOT_FOR_ARCHS)
-IGNORE=		"does not run on ${NOT_FOR_ARCHS},"
+IGNORE=		does not run on ${NOT_FOR_ARCHS},
 .endif
-IGNORE+=	"and you are running ${ARCH}"
+IGNORE+=	and you are running ${ARCH}
 .endif
 
 .if !defined(NO_IGNORE)
 .if (defined(IS_INTERACTIVE) && defined(BATCH))
-IGNORE=	"is an interactive port"
+IGNORE=	is an interactive port
 .elif (!defined(IS_INTERACTIVE) && defined(INTERACTIVE))
-IGNORE=	"is not an interactive port"
+IGNORE=	is not an interactive port
 .elif (defined(NO_CDROM) && defined(FOR_CDROM))
-IGNORE=	"may not be placed on a CDROM: ${NO_CDROM}"
+IGNORE=	may not be placed on a CDROM: ${NO_CDROM}
 .elif (defined(RESTRICTED) && defined(NO_RESTRICTED))
-IGNORE=	"is restricted: ${RESTRICTED}"
+IGNORE=	is restricted: ${RESTRICTED}
 .elif defined(BROKEN)
 .if !defined(TRYBROKEN)
-IGNORE=	"is marked as broken: ${BROKEN}"
+IGNORE=	is marked as broken: ${BROKEN}
 .endif
 .elif defined(FORBIDDEN)
-IGNORE=	"is forbidden: ${FORBIDDEN}"
+IGNORE=	is forbidden: ${FORBIDDEN}
 .endif
 
 .if (defined(MANUAL_PACKAGE_BUILD) && defined(PACKAGE_BUILDING) && !defined(PARALLEL_PACKAGE_BUILD))
-IGNORE=	"has to be built manually: ${MANUAL_PACKAGE_BUILD}"
+IGNORE=	has to be built manually: ${MANUAL_PACKAGE_BUILD}
 clean:
 	@${IGNORECMD}
 .endif
@@ -2804,7 +2804,7 @@
 .if defined(IGNORE_SILENT)
 IGNORECMD=	${DO_NADA}
 .else
-IGNORECMD=	${ECHO_MSG} "===>  ${PKGNAME} ${IGNORE}."
+IGNORECMD=	${ECHO_MSG} "===>  ${PKGNAME} "${IGNORE:Q}.
 .endif
 
 .for target in check-sanity fetch checksum extract patch configure all build install reinstall package
@@ -2927,7 +2927,7 @@
 .if defined(IGNORE_SILENT)
 	@${DO_NADA}
 .else
-	@${ECHO_MSG} "===>  ${PKGNAME} may not be packaged: ${NO_PACKAGE}."
+	@${ECHO_MSG} "===>  ${PKGNAME} may not be packaged: "${NO_PACKAGE:Q}.
 .endif
 .endif
 
@@ -2984,7 +2984,7 @@
 	@${ECHO_MSG}
 	@${ECHO_MSG} "This port is deprecated; you may wish to reconsider installing it:"
 	@${ECHO_MSG}
-	@${ECHO_MSG} "${DEPRECATED}."
+	@${ECHO_MSG} ${DEPRECATED:Q}.
 	@${ECHO_MSG}
 .if defined(EXPIRATION_DATE)
 	@${ECHO_MSG} "It is scheduled to be removed on or after ${EXPIRATION_DATE}."
Index: bsd.ruby.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.ruby.mk,v
retrieving revision 1.147
diff -u -r1.147 bsd.ruby.mk
--- bsd.ruby.mk	14 Nov 2005 09:46:15 -0000	1.147
+++ bsd.ruby.mk	12 Dec 2005 16:36:36 -0000
@@ -114,12 +114,12 @@
 
 .if defined(RUBY)
 .if !exists(${RUBY})
-BROKEN=	"You set the variable RUBY to \"${RUBY}\", but it does not seem to exist.  Please specify an already installed ruby executable."
+BROKEN=	You set the variable RUBY to "${RUBY}", but it does not seem to exist.  Please specify an already installed ruby executable.
 .endif
 
 _RUBY_TEST!=		${RUBY} -e 'begin; require "rbconfig"; rescue LoadError; puts "error"; end'
 .if !empty(_RUBY_TEST)
-BROKEN=	"You set the variable RUBY to \"${RUBY}\", but it failed to include rbconfig.  Please specify a properly installed ruby executable."
+BROKEN=	You set the variable RUBY to "${RUBY}", but it failed to include rbconfig.  Please specify a properly installed ruby executable.
 .endif
 
 _RUBY_CONFIG=		${RUBY} -r rbconfig -e 'C = Config::CONFIG' -e
@@ -147,7 +147,7 @@
 RUBY_WRKSRC=		${WRKDIR}/ruby-${RUBY_VERSION}
 #MASTER_SITE_SUBDIR_RUBY=	snapshots
 .elif defined(RUBY_VER) && ${RUBY_VER} == 1.7
-BROKEN=	"Ruby 1.7 is obsolete; set RUBY_VER to 1.8 instead."
+BROKEN=	Ruby 1.7 is obsolete; set RUBY_VER to 1.8 instead.
 .else
 RUBY_VERSION?=		1.6.8
 RUBY_DISTVERSION?=	${RUBY_VERSION}-2004.07.28
Index: bsd.sdl.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.sdl.mk,v
retrieving revision 1.8
diff -u -r1.8 bsd.sdl.mk
--- bsd.sdl.mk	9 Jan 2005 10:12:07 -0000	1.8
+++ bsd.sdl.mk	12 Dec 2005 16:36:36 -0000
@@ -145,7 +145,7 @@
 _USE_SDL=
 .for component in ${USE_SDL}
 . if ${_USE_SDL_ALL:M${component}}==""
-BROKEN=	"Unknown SDL component ${component}"
+BROKEN=	Unknown SDL component ${component}
 . endif
 _USE_SDL+=	${_REQUIRES_${component}} ${component}
 .endfor
Index: bsd.tcl.mk
===================================================================
RCS file: /home/ncvs/ports/Mk/bsd.tcl.mk,v
retrieving revision 1.1
diff -u -r1.1 bsd.tcl.mk
--- bsd.tcl.mk	8 Nov 2005 09:02:51 -0000	1.1
+++ bsd.tcl.mk	12 Dec 2005 16:37:57 -0000
@@ -79,7 +79,7 @@
 .endfor
 
 .if ${_FOUND} == "no"
-IGNORE=		"Unknown TCL version specified: ${USE_TCL}"
+IGNORE=		Unknown TCL version specified: ${USE_TCL}
 .endif
 .endif # defined(USE_TCL)
 
@@ -94,7 +94,7 @@
 TK_VER:=	${USE_TK:S/8/8./}
 
 .if defined(USE_TCL) && ${TCL_VER} != ${TK_VER}
-IGNORE=		"TCL and TK versions must be equal (${TCL_VER} vs ${TK_VER})"
+IGNORE=		TCL and TK versions must be equal (${TCL_VER} vs ${TK_VER})
 .endif
 
 _FOUND=		no
@@ -112,7 +112,7 @@
 .endfor
 
 .if ${_FOUND} == "no"
-IGNORE=		"Unknown TK version specified: ${USE_TK}"
+IGNORE=		Unknown TK version specified: ${USE_TK}
 .endif
 .endif # defined(USE_TK)
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2005-12-19 01:28:23 UTC
State Changed
From-To: feedback->analyzed

Accepted for testing on the cluster.
Comment 5 Mark Linimon freebsd_committer freebsd_triage 2006-01-21 19:53:49 UTC
State Changed
From-To: analyzed->closed

Committed, thanks.