[gentoo-dev] [PATCH] cdrom.eclass: Near rewrite

[gentoo-dev] [PATCH] cdrom.eclass: Near rewrite

[James Le Cuirot]

If you've been wondering why I've been quiet of late (you have,
right?!) then this is partly why. I'm not sure why I spent so long on
an eclass that hardly anyone uses but it's utilised by many of my old
favourite games.

The main bug that it fixes is one I filed a report for 10 years ago!
There were also several duplicates. If you want something done...

I realised part way through that I didn't have any multi-disc games
that use this eclass any more. A former friend of mine lost my second
UT99 disc with the high resolution textures. I couldn't rewrite the
eclass without testing it thoroughly so I wrote a brand new "comi"
(Curse of Monkey Island) ebuild for use with ScummVM! I'll get this
committed when these changes go in. I've also revised all the
Descent-related packages.

[gentoo-dev] [PATCH 01/14] cdrom.eclass: Detect case-insensitively and handle special characters

[James Le Cuirot]

This eclass previously used "find -iname" but it only checked the file
case-insensitively and not the directories. There is "find -ipath" but
this does not intelligently skip non-matching paths, making it
slow. Globbing is used here instead.

The : character has always been used to delimit paths given to
cdrom_get_cds, which makes sense because : generally isn't allowed on
CDs, while whitespace is. Despite that, whitespace was not being
handled properly and neither were wildcard characters. Now all special
characters are automatically escaped.
---
 eclass/cdrom.eclass | 48 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 41488d2446c2..de72f15563db 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -79,12 +79,13 @@ cdrom_get_cds() {
 		export CDROM_ROOT=${CD_ROOT_1:-${CD_ROOT}}
 		einfo "Found CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
 		export CDROM_SET=-1
-		for f in ${CDROM_CHECK_1//:/ } ; do
+		IFS=:
+		for f in ${CDROM_CHECK_1} ; do
+			unset IFS
 			((++CDROM_SET))
-			[[ -e ${CDROM_ROOT}/${f} ]] && break
+			export CDROM_MATCH=$(_cdrom_glob_match "${CDROM_ROOT}" "${f}")
+			[[ -n ${CDROM_MATCH} ]] && return
 		done
-		export CDROM_MATCH=${f}
-		return
 	fi
 
 	# User didn't help us out so lets make sure they know they can
@@ -181,28 +182,24 @@ _cdrom_locate_file_on_cd() {
 	local showedmsg=0 showjolietmsg=0
 
 	while [[ -z ${CDROM_ROOT} ]] ; do
-		local i=0
-		local -a cdset=(${*//:/ })
+		local i=0 cdset
+		IFS=: read -a cdset <<< "${*}"
+
 		if [[ -n ${CDROM_SET} ]] ; then
-			cdset=(${cdset[${CDROM_SET}]})
+			cdset=( "${cdset[${CDROM_SET}]}" )
 		fi
 
 		while [[ -n ${cdset[${i}]} ]] ; do
-			local dir=$(dirname ${cdset[${i}]})
-			local file=$(basename ${cdset[${i}]})
-
 			local point= node= fs= foo=
 			while read point node fs foo ; do
 				[[ " cd9660 iso9660 udf " != *" ${fs} "* ]] && \
 					! [[ ${fs} == "subfs" && ",${opts}," == *",fs=cdfss,"* ]] \
 					&& continue
 				point=${point//\040/ }
-				[[ ! -d ${point}/${dir} ]] && continue
-				[[ -z $(find "${point}/${dir}" -maxdepth 1 -iname "${file}") ]] \
-					&& continue
+				export CDROM_MATCH=$(_cdrom_glob_match "${point}" "${cdset[${i}]}")
+				[[ -z ${CDROM_MATCH} ]] && continue
 				export CDROM_ROOT=${point}
 				export CDROM_SET=${i}
-				export CDROM_MATCH=${cdset[${i}]}
 				return
 			done <<< "$(get_mounts)"
 
@@ -243,4 +240,27 @@ _cdrom_locate_file_on_cd() {
 	done
 }
 
+# @FUNCTION: _cdrom_glob_match
+# @USAGE: <root directory> <path>
+# @INTERNAL
+# @DESCRIPTION:
+# Locates the given path ($2) within the given root directory ($1)
+# case-insensitively and returns the first actual matching path. This
+# eclass previously used "find -iname" but it only checked the file
+# case-insensitively and not the directories. There is "find -ipath" but
+# this does not intelligently skip non-matching paths, making it
+# slow. Case-insensitive matching can only be applied to patterns so
+# extended globbing is used to turn regular strings into patterns. All
+# special characters are escaped so don't worry about breaking this. The
+# first person to make this work without an eval wins a cookie.
+_cdrom_glob_match() {
+	local p=\?\($(sed -e 's:[^A-Za-z0-9/]:\\\0:g' -e 's:/:)/?(:g' <<< "$2" || die)\)
+	(
+		cd "$1" 2>/dev/null || return
+		shopt -s extglob nocaseglob nullglob || die
+		eval "ARRAY=( ${p} )"
+		echo ${ARRAY[0]}
+	)
+}
+
 fi
-- 
2.11.0

[gentoo-dev] [PATCH 02/14] cdrom.eclass: Simplify printing of CD_ROOT_# variable names

[James Le Cuirot]

---
 eclass/cdrom.eclass | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index de72f15563db..f1839b189ae9 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -122,12 +122,7 @@ cdrom_get_cds() {
 		einfo "If you do not have the CDs, but have the data files"
 		einfo "mounted somewhere on your filesystem, just export"
 		einfo "the following variables so they point to the right place:"
-		einfon ""
-		cdcnt=0
-		while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do
-			((++cdcnt))
-			echo -n " CD_ROOT_${cdcnt}"
-		done
+		einfo $(printf "CD_ROOT_%d " $(seq ${#}))
 		echo
 		einfo "Or, if you have all the files in the same place, or"
 		einfo "you only have one cdrom, you can export CD_ROOT"
-- 
2.11.0

[gentoo-dev] [PATCH 03/14] cdrom.eclass: Rename CDROM_NAME_SET array to CDROM_NAMES

[James Le Cuirot]

vapier seemed confused about what he wanted this variable to do as can
be seen in bug #139196. The eclass used it for the names of each disc,
regardless of the set, while ebuilds used it for the name of each
single-disc set. This was not helped by the fact that the set feature
has been totally undocumented. The former behaviour makes more sense
so let's rename the array to something less confusing.

This will not break ebuilds already using CDROM_NAME_SET. As they all
use just a single disc, they currently do not display the names given
in this variable anyway.
---
 eclass/cdrom.eclass | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index f1839b189ae9..72250556e624 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -44,7 +44,7 @@ fi
 # etc...  If you want to give the cds better names, then just export
 # the appropriate CDROM_NAME variable before calling cdrom_get_cds().
 # Use CDROM_NAME for one cd, or CDROM_NAME_# for multiple cds.  You can
-# also use the CDROM_NAME_SET bash array.
+# also use the CDROM_NAMES bash array.
 #
 # For those multi cd ebuilds, see the cdrom_load_next_cd() function.
 cdrom_get_cds() {
@@ -102,12 +102,12 @@ cdrom_get_cds() {
 		einfo "export CD_ROOT=/mnt/cdrom"
 		echo
 	else
-		if [[ -n ${CDROM_NAME_SET} ]] ; then
-			# Translate the CDROM_NAME_SET array into CDROM_NAME_#
+		if [[ -n ${CDROM_NAMES} ]] ; then
+			# Translate the CDROM_NAMES array into CDROM_NAME_#
 			cdcnt=0
 			while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do
 				((++cdcnt))
-				export CDROM_NAME_${cdcnt}="${CDROM_NAME_SET[$((${cdcnt}-1))]}"
+				export CDROM_NAME_${cdcnt}="${CDROM_NAMES[$((${cdcnt}-1))]}"
 			done
 		fi
 
-- 
2.11.0

[gentoo-dev] [PATCH 04/14] cdrom.eclass: Allow CDROM_NAMES changes before each cdrom_load_next_cd

[James Le Cuirot]

This works around the lack of per-set disc names. Once the first disc
has been detected, ebuilds can adjust CDROM_NAMES to contain just the
names from the matched CDROM_SET.
---
 eclass/cdrom.eclass | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 72250556e624..9724c66ca2ce 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -102,15 +102,7 @@ cdrom_get_cds() {
 		einfo "export CD_ROOT=/mnt/cdrom"
 		echo
 	else
-		if [[ -n ${CDROM_NAMES} ]] ; then
-			# Translate the CDROM_NAMES array into CDROM_NAME_#
-			cdcnt=0
-			while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do
-				((++cdcnt))
-				export CDROM_NAME_${cdcnt}="${CDROM_NAMES[$((${cdcnt}-1))]}"
-			done
-		fi
-
+		_cdrom_set_names
 		einfo "This package will need access to ${CDROM_TOTAL_CDS} cds."
 		cdcnt=0
 		while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do
@@ -152,6 +144,8 @@ cdrom_load_next_cd() {
 	local var
 	((++CDROM_CURRENT_CD))
 
+	_cdrom_set_names
+
 	unset CDROM_ROOT
 	var=CD_ROOT_${CDROM_CURRENT_CD}
 	[[ -z ${!var} ]] && var="CD_ROOT"
@@ -258,4 +252,17 @@ _cdrom_glob_match() {
 	)
 }
 
+# @FUNCTION: _cdrom_set_names
+# @INTERNAL
+# @DESCRIPTION:
+# Populate CDROM_NAME_# variables with the CDROM_NAMES array.
+_cdrom_set_names() {
+	if [[ -n ${CDROM_NAMES} ]] ; then
+		local i
+		for i in $(seq ${#CDROM_NAMES[@]}); do
+			export CDROM_NAME_${i}="${CDROM_NAMES[$((${i} - 1))]}"
+		done
+	fi
+}
+
 fi
-- 
2.11.0

[gentoo-dev] [PATCH 05/14] cdrom.eclass: Remove ye olde Submount check

[James Le Cuirot]

Submount was last-rited in 2007 and was already dead long before that.
---
 eclass/cdrom.eclass | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 9724c66ca2ce..681683f9328c 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -181,9 +181,7 @@ _cdrom_locate_file_on_cd() {
 		while [[ -n ${cdset[${i}]} ]] ; do
 			local point= node= fs= foo=
 			while read point node fs foo ; do
-				[[ " cd9660 iso9660 udf " != *" ${fs} "* ]] && \
-					! [[ ${fs} == "subfs" && ",${opts}," == *",fs=cdfss,"* ]] \
-					&& continue
+				[[ " cd9660 iso9660 udf " != *" ${fs} "* ]] && continue
 				point=${point//\040/ }
 				export CDROM_MATCH=$(_cdrom_glob_match "${point}" "${cdset[${i}]}")
 				[[ -z ${CDROM_MATCH} ]] && continue
-- 
2.11.0

[gentoo-dev] [PATCH 06/14] cdrom.eclass: Simplify loop with seq

[James Le Cuirot]

---
 eclass/cdrom.eclass | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 681683f9328c..b8fdb03ac535 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -104,10 +104,9 @@ cdrom_get_cds() {
 	else
 		_cdrom_set_names
 		einfo "This package will need access to ${CDROM_TOTAL_CDS} cds."
-		cdcnt=0
-		while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do
-			((++cdcnt))
-			var="CDROM_NAME_${cdcnt}"
+		local cdcnt
+		for cdcnt in $(seq ${#}); do
+			local var=CDROM_NAME_${cdcnt}
 			[[ ! -z ${!var} ]] && einfo " CD ${cdcnt}: ${!var}"
 		done
 		echo
-- 
2.11.0

[gentoo-dev] [PATCH 07/14] cdrom.eclass: We don't know for sure how many discs will be needed

[James Le Cuirot]

The number of discs may vary between sets and ebuilds may not call
cdrom_load_next_cd() for every argument depending on USE flags and
other conditional factors.
---
 eclass/cdrom.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index b8fdb03ac535..67f9b15d21e4 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -103,7 +103,7 @@ cdrom_get_cds() {
 		echo
 	else
 		_cdrom_set_names
-		einfo "This package will need access to ${CDROM_TOTAL_CDS} cds."
+		einfo "This package may need access to ${CDROM_TOTAL_CDS} cds."
 		local cdcnt
 		for cdcnt in $(seq ${#}); do
 			local var=CDROM_NAME_${cdcnt}
-- 
2.11.0

[gentoo-dev] [PATCH 08/14] cdrom.eclass: Fix important typo in the multiple disc instructions

[James Le Cuirot]

If you have all the files within the same directory tree then you
should set CD_ROOT, not CD_ROOT_1.
---
 eclass/cdrom.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 67f9b15d21e4..56032e084d01 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -121,7 +121,7 @@ cdrom_get_cds() {
 		einfo "for all the CDs."
 		echo
 		einfo "For example:"
-		einfo "export CD_ROOT_1=/mnt/cdrom"
+		einfo "export CD_ROOT=/mnt/cdrom"
 		echo
 	fi
 
-- 
2.11.0

[gentoo-dev] [PATCH 09/14] cdrom.eclass: Change CDROM_CHECK_# variables to a CDROM_CHECKS array

[James Le Cuirot]

---
 eclass/cdrom.eclass | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 56032e084d01..10a19551161a 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -52,12 +52,8 @@ cdrom_get_cds() {
 	# the # of files they gave us
 	local cdcnt=0
 	local f=
-	for f in "$@" ; do
-		((++cdcnt))
-		export CDROM_CHECK_${cdcnt}="$f"
-	done
 	export CDROM_TOTAL_CDS=${cdcnt}
-	export CDROM_CURRENT_CD=1
+	export CDROM_CURRENT_CD=1 CDROM_CHECKS=( "${@}" )
 
 	# now we see if the user gave use CD_ROOT ...
 	# if they did, let's just believe them that it's correct
@@ -80,7 +76,7 @@ cdrom_get_cds() {
 		einfo "Found CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
 		export CDROM_SET=-1
 		IFS=:
-		for f in ${CDROM_CHECK_1} ; do
+		for f in ${CDROM_CHECKS[0]} ; do
 			unset IFS
 			((++CDROM_SET))
 			export CDROM_MATCH=$(_cdrom_glob_match "${CDROM_ROOT}" "${f}")
@@ -149,8 +145,7 @@ cdrom_load_next_cd() {
 	var=CD_ROOT_${CDROM_CURRENT_CD}
 	[[ -z ${!var} ]] && var="CD_ROOT"
 	if [[ -z ${!var} ]] ; then
-		var="CDROM_CHECK_${CDROM_CURRENT_CD}"
-		_cdrom_locate_file_on_cd ${!var}
+		_cdrom_locate_file_on_cd "${CDROM_CHECKS[${CDROM_CURRENT_CD}]}"
 	else
 		export CDROM_ROOT=${!var}
 	fi
-- 
2.11.0

[gentoo-dev] [PATCH 10/14] cdrom.eclass: The CDROM_TOTAL_CDS variable is redundant now

[James Le Cuirot]

This was never formally declared by the eclass or used by ebuilds.
---
 eclass/cdrom.eclass | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 10a19551161a..95bf48829e14 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -52,7 +52,6 @@ cdrom_get_cds() {
 	# the # of files they gave us
 	local cdcnt=0
 	local f=
-	export CDROM_TOTAL_CDS=${cdcnt}
 	export CDROM_CURRENT_CD=1 CDROM_CHECKS=( "${@}" )
 
 	# now we see if the user gave use CD_ROOT ...
@@ -60,7 +59,7 @@ cdrom_get_cds() {
 	if [[ -n ${CD_ROOT}${CD_ROOT_1} ]] ; then
 		local var=
 		cdcnt=0
-		while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do
+		while [[ ${cdcnt} -lt ${#} ]] ; do
 			((++cdcnt))
 			var="CD_ROOT_${cdcnt}"
 			[[ -z ${!var} ]] && var="CD_ROOT"
@@ -68,7 +67,7 @@ cdrom_get_cds() {
 				eerror "You must either use just the CD_ROOT"
 				eerror "or specify ALL the CD_ROOT_X variables."
 				eerror "In this case, you will need" \
-					"${CDROM_TOTAL_CDS} CD_ROOT_X variables."
+					"${#} CD_ROOT_X variables."
 				die "could not locate CD_ROOT_${cdcnt}"
 			fi
 		done
@@ -86,7 +85,7 @@ cdrom_get_cds() {
 
 	# User didn't help us out so lets make sure they know they can
 	# simplify the whole process ...
-	if [[ ${CDROM_TOTAL_CDS} -eq 1 ]] ; then
+	if [[ ${#} -eq 1 ]] ; then
 		einfo "This ebuild will need the ${CDROM_NAME:-cdrom for ${PN}}"
 		echo
 		einfo "If you do not have the CD, but have the data files"
@@ -99,7 +98,7 @@ cdrom_get_cds() {
 		echo
 	else
 		_cdrom_set_names
-		einfo "This package may need access to ${CDROM_TOTAL_CDS} cds."
+		einfo "This package may need access to ${#} cds."
 		local cdcnt
 		for cdcnt in $(seq ${#}); do
 			local var=CDROM_NAME_${cdcnt}
@@ -189,7 +188,7 @@ _cdrom_locate_file_on_cd() {
 
 		echo
 		if [[ ${showedmsg} -eq 0 ]] ; then
-			if [[ ${CDROM_TOTAL_CDS} -eq 1 ]] ; then
+			if [[ ${#CDROM_CHECKS[@]} -eq 1 ]] ; then
 				if [[ -z ${CDROM_NAME} ]] ; then
 					einfo "Please insert+mount the cdrom for ${PN} now !"
 				else
-- 
2.11.0

[gentoo-dev] [PATCH 11/14] cdrom.eclass: Make CD_ROOT less of a special case, fixes #195868

[James Le Cuirot]

CD_ROOT was intended to be a power user feature so the eclass didn't
try very hard to check the validity of the given location. This
difference in behaviour ultimately made the eclass larger and more
confusing.

It now uses the same matching loop as the regular case, making it
simpler and more consistent. The only differences are that it doesn't
show information or prompts about inserting discs and it dies
immediately if a match cannot be found.
---
 eclass/cdrom.eclass | 126 +++++++++++++++++++---------------------------------
 1 file changed, 46 insertions(+), 80 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 95bf48829e14..f4ea2ff36400 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -48,44 +48,16 @@ fi
 #
 # For those multi cd ebuilds, see the cdrom_load_next_cd() function.
 cdrom_get_cds() {
-	# first we figure out how many cds we're dealing with by
-	# the # of files they gave us
-	local cdcnt=0
-	local f=
-	export CDROM_CURRENT_CD=1 CDROM_CHECKS=( "${@}" )
+	unset CDROM_SET
+	export CDROM_CURRENT_CD=0 CDROM_CHECKS=( "${@}" )
 
-	# now we see if the user gave use CD_ROOT ...
-	# if they did, let's just believe them that it's correct
+	# If the user has set CD_ROOT or CD_ROOT_1, don't bother informing
+	# them about which discs are needed as they presumably already know.
 	if [[ -n ${CD_ROOT}${CD_ROOT_1} ]] ; then
-		local var=
-		cdcnt=0
-		while [[ ${cdcnt} -lt ${#} ]] ; do
-			((++cdcnt))
-			var="CD_ROOT_${cdcnt}"
-			[[ -z ${!var} ]] && var="CD_ROOT"
-			if [[ -z ${!var} ]] ; then
-				eerror "You must either use just the CD_ROOT"
-				eerror "or specify ALL the CD_ROOT_X variables."
-				eerror "In this case, you will need" \
-					"${#} CD_ROOT_X variables."
-				die "could not locate CD_ROOT_${cdcnt}"
-			fi
-		done
-		export CDROM_ROOT=${CD_ROOT_1:-${CD_ROOT}}
-		einfo "Found CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
-		export CDROM_SET=-1
-		IFS=:
-		for f in ${CDROM_CHECKS[0]} ; do
-			unset IFS
-			((++CDROM_SET))
-			export CDROM_MATCH=$(_cdrom_glob_match "${CDROM_ROOT}" "${f}")
-			[[ -n ${CDROM_MATCH} ]] && return
-		done
-	fi
+		:
 
-	# User didn't help us out so lets make sure they know they can
-	# simplify the whole process ...
-	if [[ ${#} -eq 1 ]] ; then
+	# Single disc info.
+	elif [[ ${#} -eq 1 ]] ; then
 		einfo "This ebuild will need the ${CDROM_NAME:-cdrom for ${PN}}"
 		echo
 		einfo "If you do not have the CD, but have the data files"
@@ -96,6 +68,8 @@ cdrom_get_cds() {
 		einfo "For example:"
 		einfo "export CD_ROOT=/mnt/cdrom"
 		echo
+
+	# Multi disc info.
 	else
 		_cdrom_set_names
 		einfo "This package may need access to ${#} cds."
@@ -120,8 +94,7 @@ cdrom_get_cds() {
 		echo
 	fi
 
-	export CDROM_SET=""
-	export CDROM_CURRENT_CD=0
+	# Scan for the first disc.
 	cdrom_load_next_cd
 }
 
@@ -135,57 +108,48 @@ cdrom_get_cds() {
 # in the CD list, so make sure you only call this function when you're
 # done using the current CD.
 cdrom_load_next_cd() {
-	local var
-	((++CDROM_CURRENT_CD))
-
-	_cdrom_set_names
+	local showedmsg=0 showjolietmsg=0
 
 	unset CDROM_ROOT
-	var=CD_ROOT_${CDROM_CURRENT_CD}
-	[[ -z ${!var} ]] && var="CD_ROOT"
-	if [[ -z ${!var} ]] ; then
-		_cdrom_locate_file_on_cd "${CDROM_CHECKS[${CDROM_CURRENT_CD}]}"
-	else
-		export CDROM_ROOT=${!var}
-	fi
-
-	einfo "Found CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
-}
-
-# this is used internally by the cdrom_get_cds() and cdrom_load_next_cd()
-# functions.  this should *never* be called from an ebuild.
-# all it does is try to locate a give file on a cd ... if the cd isn't
-# found, then a message asking for the user to insert the cdrom will be
-# displayed and we'll hang out here until:
-# (1) the file is found on a mounted cdrom
-# (2) the user hits CTRL+C
-_cdrom_locate_file_on_cd() {
-	local mline=""
-	local showedmsg=0 showjolietmsg=0
+	((++CDROM_CURRENT_CD))
 
-	while [[ -z ${CDROM_ROOT} ]] ; do
-		local i=0 cdset
-		IFS=: read -a cdset <<< "${*}"
+	_cdrom_set_names
 
-		if [[ -n ${CDROM_SET} ]] ; then
-			cdset=( "${cdset[${CDROM_SET}]}" )
-		fi
+	while true ; do
+		local i cdset
+		: CD_ROOT_${CDROM_CURRENT_CD}
+		export CDROM_ROOT=${CD_ROOT:-${!_}}
+		IFS=: read -a cdset <<< "${CDROM_CHECKS[$((${CDROM_CURRENT_CD} - 1))]}"
+
+		for i in $(seq ${CDROM_SET:-0} ${CDROM_SET:-$((${#cdset[@]} - 1))}); do
+			local f=${cdset[${i}]} point= node= fs= opts=
+
+			if [[ -z ${CDROM_ROOT} ]] ; then
+				while read point node fs opts ; do
+					has "${fs}" cd9660 iso9660 udf || continue
+					point=${point//\040/ }
+					export CDROM_MATCH=$(_cdrom_glob_match "${point}" "${f}")
+					[[ -z ${CDROM_MATCH} ]] && continue
+					export CDROM_ROOT=${point}
+				done <<< "$(get_mounts)"
+			else
+				export CDROM_MATCH=$(_cdrom_glob_match "${CDROM_ROOT}" "${f}")
+			fi
 
-		while [[ -n ${cdset[${i}]} ]] ; do
-			local point= node= fs= foo=
-			while read point node fs foo ; do
-				[[ " cd9660 iso9660 udf " != *" ${fs} "* ]] && continue
-				point=${point//\040/ }
-				export CDROM_MATCH=$(_cdrom_glob_match "${point}" "${cdset[${i}]}")
-				[[ -z ${CDROM_MATCH} ]] && continue
-				export CDROM_ROOT=${point}
+			if [[ -n ${CDROM_MATCH} ]] ; then
+				export CDROM_ABSMATCH=${CDROM_ROOT}/${CDROM_MATCH}
 				export CDROM_SET=${i}
-				return
-			done <<< "$(get_mounts)"
-
-			((++i))
+				break 2
+			fi
 		done
 
+		# If we get here then we were unable to locate a match. If
+		# CDROM_ROOT is non-empty then this implies that a CD_ROOT
+		# variable was given and we should therefore abort immediately.
+		if [[ -n ${CDROM_ROOT} ]] ; then
+			die "unable to locate CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
+		fi
+
 		echo
 		if [[ ${showedmsg} -eq 0 ]] ; then
 			if [[ ${#CDROM_CHECKS[@]} -eq 1 ]] ; then
@@ -218,6 +182,8 @@ _cdrom_locate_file_on_cd() {
 		fi
 		read || die "something is screwed with your system"
 	done
+
+	einfo "Found CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
 }
 
 # @FUNCTION: _cdrom_glob_match
-- 
2.11.0

[gentoo-dev] [PATCH 12/14] cdrom.eclass: Use consistent terminology in prompts and messages

[James Le Cuirot]

---
 eclass/cdrom.eclass | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index f4ea2ff36400..36d967d2f4cd 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -58,7 +58,7 @@ cdrom_get_cds() {
 
 	# Single disc info.
 	elif [[ ${#} -eq 1 ]] ; then
-		einfo "This ebuild will need the ${CDROM_NAME:-cdrom for ${PN}}"
+		einfo "This ebuild will need the ${CDROM_NAME:-CD for ${PN}}"
 		echo
 		einfo "If you do not have the CD, but have the data files"
 		einfo "mounted somewhere on your filesystem, just export"
@@ -72,7 +72,7 @@ cdrom_get_cds() {
 	# Multi disc info.
 	else
 		_cdrom_set_names
-		einfo "This package may need access to ${#} cds."
+		einfo "This package may need access to ${#} CDs."
 		local cdcnt
 		for cdcnt in $(seq ${#}); do
 			local var=CDROM_NAME_${cdcnt}
@@ -85,7 +85,7 @@ cdrom_get_cds() {
 		einfo $(printf "CD_ROOT_%d " $(seq ${#}))
 		echo
 		einfo "Or, if you have all the files in the same place, or"
-		einfo "you only have one cdrom, you can export CD_ROOT"
+		einfo "you only have one CD, you can export CD_ROOT"
 		einfo "and that place will be used as the same data source"
 		einfo "for all the CDs."
 		echo
@@ -150,31 +150,27 @@ cdrom_load_next_cd() {
 			die "unable to locate CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
 		fi
 
-		echo
 		if [[ ${showedmsg} -eq 0 ]] ; then
 			if [[ ${#CDROM_CHECKS[@]} -eq 1 ]] ; then
-				if [[ -z ${CDROM_NAME} ]] ; then
-					einfo "Please insert+mount the cdrom for ${PN} now !"
-				else
-					einfo "Please insert+mount the ${CDROM_NAME} cdrom now !"
-				fi
+				einfo "Please insert+mount the ${CDROM_NAME:-CD for ${PN}} now !"
 			else
-				if [[ -z ${CDROM_NAME_1} ]] ; then
-					einfo "Please insert+mount cd #${CDROM_CURRENT_CD}" \
-						"for ${PN} now !"
+				local var="CDROM_NAME_${CDROM_CURRENT_CD}"
+				if [[ -z ${!var} ]] ; then
+					einfo "Please insert+mount CD #${CDROM_CURRENT_CD} for ${PN} now !"
 				else
-					local var="CDROM_NAME_${CDROM_CURRENT_CD}"
-					einfo "Please insert+mount the ${!var} cdrom now !"
+					einfo "Please insert+mount the ${!var} now !"
 				fi
 			fi
 			showedmsg=1
 		fi
-		einfo "Press return to scan for the cd again"
+
+		einfo "Press return to scan for the CD again"
 		einfo "or hit CTRL+C to abort the emerge."
-		echo
+
 		if [[ ${showjolietmsg} -eq 0 ]] ; then
 			showjolietmsg=1
 		else
+			echo
 			ewarn "If you are having trouble with the detection"
 			ewarn "of your CD, it is possible that you do not have"
 			ewarn "Joliet support enabled in your kernel.  Please"
-- 
2.11.0

[gentoo-dev] [PATCH 13/14] cdrom.eclass: Update and improve documentation following changes

[James Le Cuirot]

---
 eclass/cdrom.eclass | 121 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 96 insertions(+), 25 deletions(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index 36d967d2f4cd..c95bb80325e9 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -6,13 +6,13 @@
 # games@gentoo.org
 # @BLURB: Functions for CD-ROM handling
 # @DESCRIPTION:
-# Acquire cd(s) for those lovely cd-based emerges.  Yes, this violates
-# the whole 'non-interactive' policy, but damnit I want CD support!
+# Acquire CD(s) for those lovely CD-based emerges.  Yes, this violates
+# the whole "non-interactive" policy, but damnit I want CD support!
 #
-# With these cdrom functions we handle all the user interaction and
-# standardize everything.  All you have to do is call cdrom_get_cds()
-# and when the function returns, you can assume that the cd has been
-# found at CDROM_ROOT.
+# Do not call these functions in pkg_* phases like pkg_setup as they
+# should not be used for binary packages.  Most packages using this
+# eclass will require RESTRICT="bindist" but the point still stands.
+# The functions are generally called in src_unpack.
 
 if [[ -z ${_CDROM_ECLASS} ]]; then
 _CDROM_ECLASS=1
@@ -24,8 +24,8 @@ inherit portability
 # @DESCRIPTION:
 # By default, the eclass sets PROPERTIES="interactive" on the assumption
 # that people will be using these.  If your package optionally supports
-# disc based installed, then set this to "yes", and we'll set things
-# conditionally based on USE=cdinstall.
+# disc-based installs then set this to "yes" and we'll set things
+# conditionally based on USE="cdinstall".
 if [[ ${CDROM_OPTIONAL} == "yes" ]] ; then
 	IUSE="cdinstall"
 	PROPERTIES="cdinstall? ( interactive )"
@@ -34,19 +34,41 @@ else
 fi
 
 # @FUNCTION: cdrom_get_cds
-# @USAGE: <file on cd1> [file on cd2] [file on cd3] [...]
+# @USAGE: <cd1 file>[:alt cd1 file] [cd2 file[:alt cd2 file]] [...]
 # @DESCRIPTION:
-# The function will attempt to locate a cd based upon a file that is on
-# the cd.  The more files you give this function, the more cds the cdrom
-# functions will handle.
+# Attempt to locate a CD based upon a file that is on the CD.
 #
-# Normally the cdrom functions will refer to the cds as 'cd #1', 'cd #2',
-# etc...  If you want to give the cds better names, then just export
-# the appropriate CDROM_NAME variable before calling cdrom_get_cds().
-# Use CDROM_NAME for one cd, or CDROM_NAME_# for multiple cds.  You can
-# also use the CDROM_NAMES bash array.
+# If the data spans multiple discs then additional arguments can be
+# given to check for more files.  Call cdrom_load_next_cd() to scan for
+# the next disc in the set.
 #
-# For those multi cd ebuilds, see the cdrom_load_next_cd() function.
+# Sometimes it is necessary to support alternative CD "sets" where the
+# contents differ.  Alternative files for each disc can be appended to
+# each argument, separated by the : character.  This feature is
+# frequently used to support installing from an existing installation.
+# Note that after the first disc is detected, the set is locked so
+# cdrom_load_next_cd() will only scan for files in that specific set on
+# subsequent discs.
+#
+# The given files can be within named subdirectories.  It is not
+# necessary to specify different casings of the same filename as
+# matching is done case-insensitively.  Filenames can include special
+# characters such as spaces.  Only : is not allowed.
+#
+# If you don't want each disc to be referred to as "CD #1", "CD #2",
+# etc. then you can optionally provide your own names.  Set CDROM_NAME
+# for a single disc, CDROM_NAMES as an array for multiple discs, or
+# individual CDROM_NAME_# variables for each disc starting from 1.
+#
+# Despite what you may have seen in older ebuilds, it has never been
+# possible to provide per-set disc names.  This would not make sense as
+# all the names are initially displayed before the first disc has been
+# detected.  As a workaround, you can redefine the name variable(s)
+# after the first disc has been detected.
+#
+# This function ends with a cdrom_load_next_cd() call to scan for the
+# first disc.  For more details about variables read and written by this
+# eclass, see that function's description.
 cdrom_get_cds() {
 	unset CDROM_SET
 	export CDROM_CURRENT_CD=0 CDROM_CHECKS=( "${@}" )
@@ -100,13 +122,62 @@ cdrom_get_cds() {
 
 # @FUNCTION: cdrom_load_next_cd
 # @DESCRIPTION:
-# Some packages are so big they come on multiple CDs.  When you're done
-# reading files off a CD and want access to the next one, just call this
-# function.  Again, all the messy details of user interaction are taken
-# care of for you.  Once this returns, just read the variable CDROM_ROOT
-# for the location of the mounted CD.  Note that you can only go forward
-# in the CD list, so make sure you only call this function when you're
-# done using the current CD.
+# If multiple arguments were given to cdrom_get_cds() then you can call
+# this function to scan for the next disc.  This function is also called
+# implicitly to scan for the first disc.
+#
+# The file(s) given to cdrom_get_cds() are scanned for on any mounted
+# filesystem that resembles optical media.  If no match is found then
+# the user is prompted to insert and mount the disc and press enter to
+# rescan.  This will loop continuously until a match is found or the
+# user aborts with Ctrl+C.
+#
+# The user can override the scan location by setting CD_ROOT for a
+# single disc, CD_ROOT if multiple discs are merged into the same
+# directory tree (useful for existing installations), or individual
+# CD_ROOT_# variables for each disc starting from 1.  If no match is
+# found then the function dies with an error as a rescan will not help
+# in this instance.
+#
+# Users wanting to set CD_ROOT or CD_ROOT_# for specific packages
+# persistently can do so using Portage's /etc/portage/env feature.
+#
+# Regardless of which scanning method is used, several variables are set
+# by this function for you to use:
+#
+#  CDROM_ROOT: Root path of the detected disc.
+#  CDROM_MATCH: Path of the matched file, relative to CDROM_ROOT.
+#  CDROM_ABSMATCH: Absolute path of the matched file.
+#  CDROM_SET: The matching set number, starting from 0.
+#
+# The casing of CDROM_MATCH may not be the same as the argument given to
+# cdrom_get_cds() as matching is done case-insensitively.  You should
+# therefore use this variable (or CDROM_ABSMATCH) when performing file
+# operations to ensure the file is found.  Use newins rather than doins
+# to keep the final result consistent and take advantage of Bash
+# case-conversion features like ${FOO,,}.
+#
+# Chances are that you'll need more than just the matched file from each
+# disc though.  You should not assume the casing of these files either
+# but dealing with this goes beyond the scope of this ebuild.  For a
+# good example, see games-action/descent2-data, which combines advanced
+# globbing with advanced tar features to concisely deal with
+# case-insensitive matching, case conversion, file moves, and
+# conditional exclusion.
+#
+# Copying directly from a mounted disc using doins/newins will remove
+# any read-only permissions but be aware of these when copying to an
+# intermediate directory first.  Attempting to clean a build directory
+# containing read-only files as a non-root user will result in an error.
+# If you're using tar as suggested above then you can easily work around
+# this with --mode=u+w.
+#
+# Note that you can only go forwards in the disc list, so make sure you
+# only call this function when you're done using the current disc.
+#
+# If you cd to any location within CDROM_ROOT then remember to leave the
+# directory before calling this function again, otherwise the user won't
+# be able to unmount the current disc.
 cdrom_load_next_cd() {
 	local showedmsg=0 showjolietmsg=0
 
-- 
2.11.0

[gentoo-dev] [PATCH 14/14] cdrom.eclass: Update copyright year

[James Le Cuirot]

---
 eclass/cdrom.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
index c95bb80325e9..baf566ea415e 100644
--- a/eclass/cdrom.eclass
+++ b/eclass/cdrom.eclass
@@ -1,4 +1,4 @@
-# Copyright 1999-2014 Gentoo Foundation
+# Copyright 1999-2017 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
 # @ECLASS: cdrom.eclass
-- 
2.11.0

Re: [gentoo-dev] [PATCH] cdrom.eclass: Near rewrite

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

>>>>> On Mon, 17 Apr 2017, James Le Cuirot wrote:

> If you've been wondering why I've been quiet of late (you have,
> right?!) then this is partly why. I'm not sure why I spent so long
> on an eclass that hardly anyone uses but it's utilised by many of my
> old favourite games.

Wouldn't this be a good time to rethink the whole concept? By all our
standards, ebuilds shouldn't be interactive. AFAICS, cdrom.eclass is
the last remnant in the tree using PROPERTIES="interactive".

Maybe the eclass could be replaced by a utility that extracts the ISO
image and places it into DISTDIR, so that ebuilds could use regular
non-interactive unpacking? The additional disk space used shouldn't be
an argument any more with today's large disks.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] [PATCH 01/14] cdrom.eclass: Detect case-insensitively and handle special characters

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 4065 bytes --]

On pon, 2017-04-17 at 22:53 +0100, James Le Cuirot wrote:
> This eclass previously used "find -iname" but it only checked the file
> case-insensitively and not the directories. There is "find -ipath" but
> this does not intelligently skip non-matching paths, making it
> slow. Globbing is used here instead.
> 
> The : character has always been used to delimit paths given to
> cdrom_get_cds, which makes sense because : generally isn't allowed on
> CDs, while whitespace is. Despite that, whitespace was not being
> handled properly and neither were wildcard characters. Now all special
> characters are automatically escaped.
> ---
>  eclass/cdrom.eclass | 48 ++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
> index 41488d2446c2..de72f15563db 100644
> --- a/eclass/cdrom.eclass
> +++ b/eclass/cdrom.eclass
> @@ -79,12 +79,13 @@ cdrom_get_cds() {
>  		export CDROM_ROOT=${CD_ROOT_1:-${CD_ROOT}}
>  		einfo "Found CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
>  		export CDROM_SET=-1
> -		for f in ${CDROM_CHECK_1//:/ } ; do
> +		IFS=:

'local', please.

> +		for f in ${CDROM_CHECK_1} ; do
> +			unset IFS
>  			((++CDROM_SET))
> -			[[ -e ${CDROM_ROOT}/${f} ]] && break
> +			export CDROM_MATCH=$(_cdrom_glob_match "${CDROM_ROOT}" "${f}")
> +			[[ -n ${CDROM_MATCH} ]] && return
>  		done
> -		export CDROM_MATCH=${f}
> -		return
>  	fi
>  
>  	# User didn't help us out so lets make sure they know they can
> @@ -181,28 +182,24 @@ _cdrom_locate_file_on_cd() {
>  	local showedmsg=0 showjolietmsg=0
>  
>  	while [[ -z ${CDROM_ROOT} ]] ; do
> -		local i=0
> -		local -a cdset=(${*//:/ })
> +		local i=0 cdset
> +		IFS=: read -a cdset <<< "${*}"

-r to avoid handling escapes; -d '' to avoid finishing on newline.

> +
>  		if [[ -n ${CDROM_SET} ]] ; then
> -			cdset=(${cdset[${CDROM_SET}]})
> +			cdset=( "${cdset[${CDROM_SET}]}" )
>  		fi
>  
>  		while [[ -n ${cdset[${i}]} ]] ; do
> -			local dir=$(dirname ${cdset[${i}]})
> -			local file=$(basename ${cdset[${i}]})
> -
>  			local point= node= fs= foo=
>  			while read point node fs foo ; do
>  				[[ " cd9660 iso9660 udf " != *" ${fs} "* ]] && \
>  					! [[ ${fs} == "subfs" && ",${opts}," == *",fs=cdfss,"* ]] \
>  					&& continue
>  				point=${point//\040/ }
> -				[[ ! -d ${point}/${dir} ]] && continue
> -				[[ -z $(find "${point}/${dir}" -maxdepth 1 -iname "${file}") ]] \
> -					&& continue
> +				export CDROM_MATCH=$(_cdrom_glob_match "${point}" "${cdset[${i}]}")
> +				[[ -z ${CDROM_MATCH} ]] && continue
>  				export CDROM_ROOT=${point}
>  				export CDROM_SET=${i}
> -				export CDROM_MATCH=${cdset[${i}]}
>  				return
>  			done <<< "$(get_mounts)"
>  
> @@ -243,4 +240,27 @@ _cdrom_locate_file_on_cd() {
>  	done
>  }
>  
> +# @FUNCTION: _cdrom_glob_match
> +# @USAGE: <root directory> <path>
> +# @INTERNAL
> +# @DESCRIPTION:
> +# Locates the given path ($2) within the given root directory ($1)
> +# case-insensitively and returns the first actual matching path. This
> +# eclass previously used "find -iname" but it only checked the file
> +# case-insensitively and not the directories. There is "find -ipath" but
> +# this does not intelligently skip non-matching paths, making it
> +# slow. Case-insensitive matching can only be applied to patterns so
> +# extended globbing is used to turn regular strings into patterns. All
> +# special characters are escaped so don't worry about breaking this. The
> +# first person to make this work without an eval wins a cookie.
> +_cdrom_glob_match() {
> +	local p=\?\($(sed -e 's:[^A-Za-z0-9/]:\\\0:g' -e 's:/:)/?(:g' <<< "$2" || die)\)

Explanatory comment needed, i.e. what gets converted into what, and why.

> +	(
> +		cd "$1" 2>/dev/null || return
> +		shopt -s extglob nocaseglob nullglob || die
> +		eval "ARRAY=( ${p} )"
> +		echo ${ARRAY[0]}
> +	)
> +}
> +
>  fi

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH] cdrom.eclass: Near rewrite

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On wto, 2017-04-18 at 07:51 +0200, Ulrich Mueller wrote:
> > > > > > On Mon, 17 Apr 2017, James Le Cuirot wrote:
> > If you've been wondering why I've been quiet of late (you have,
> > right?!) then this is partly why. I'm not sure why I spent so long
> > on an eclass that hardly anyone uses but it's utilised by many of my
> > old favourite games.
> 
> Wouldn't this be a good time to rethink the whole concept? By all our
> standards, ebuilds shouldn't be interactive. AFAICS, cdrom.eclass is
> the last remnant in the tree using PROPERTIES="interactive".
> 
> Maybe the eclass could be replaced by a utility that extracts the ISO
> image and places it into DISTDIR, so that ebuilds could use regular
> non-interactive unpacking? The additional disk space used shouldn't be
> an argument any more with today's large disks.
> 

I think the eclass supports multiple different CD variants (releases),
including poorly made copies, copy-protected media... I don't think you
can create a file with stable checksum out of them in any sane way.

-- 
Best regards,
Michał Górny

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: [gentoo-dev] [PATCH] cdrom.eclass: Near rewrite

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

On Tue, 18 Apr 2017 07:51:58 +0200
Ulrich Mueller <ulm@gentoo.org> wrote:

> >>>>> On Mon, 17 Apr 2017, James Le Cuirot wrote:  
> 
> > If you've been wondering why I've been quiet of late (you have,
> > right?!) then this is partly why. I'm not sure why I spent so long
> > on an eclass that hardly anyone uses but it's utilised by many of my
> > old favourite games.  
> 
> Wouldn't this be a good time to rethink the whole concept? By all our
> standards, ebuilds shouldn't be interactive. AFAICS, cdrom.eclass is
> the last remnant in the tree using PROPERTIES="interactive".

mgorny makes good points, it is indeed not quite that simple.

I didn't actually notice the --accept-properties=-interactive feature
until just now, that's pretty cool.

Although I agree it should be avoided, there may be other uses for it
in future. I'd still like to go ahead with my lgogdownloader plan
(probably via a new src_fetch) and that may need it for entering
credentials on rare occasions though there are other possibilities.

> Maybe the eclass could be replaced by a utility that extracts the ISO
> image and places it into DISTDIR, so that ebuilds could use regular
> non-interactive unpacking? The additional disk space used shouldn't be
> an argument any more with today's large disks.

Don't assume everyone has such huge disks. ;) My main system isn't bad
but that doesn't mean I want to waste the space on something like this.
Many have written optical media off but I still have two big flight
cases full of discs of various kinds nearby.

No one is forced to use this stuff and it is possible to use it in a
non-interactive manner similar to how you suggest. You can copy the
files from the disc(s) and point CD_ROOT to this location in a
per-package env file.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

Re: [gentoo-dev] [PATCH 01/14] cdrom.eclass: Detect case-insensitively and handle special characters

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On Tue, 18 Apr 2017 08:08:44 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> On pon, 2017-04-17 at 22:53 +0100, James Le Cuirot wrote:
> > diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
> > index 41488d2446c2..de72f15563db 100644
> > --- a/eclass/cdrom.eclass
> > +++ b/eclass/cdrom.eclass
> > @@ -79,12 +79,13 @@ cdrom_get_cds() {
> >  		export CDROM_ROOT=${CD_ROOT_1:-${CD_ROOT}}
> >  		einfo "Found CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
> >  		export CDROM_SET=-1
> > -		for f in ${CDROM_CHECK_1//:/ } ; do
> > +		IFS=:  
> 
> 'local', please.

This line disappears later in the series but I've amended it for the
history anyway.

> > @@ -181,28 +182,24 @@ _cdrom_locate_file_on_cd() {
> >  	local showedmsg=0 showjolietmsg=0
> >  
> >  	while [[ -z ${CDROM_ROOT} ]] ; do
> > -		local i=0
> > -		local -a cdset=(${*//:/ })
> > +		local i=0 cdset
> > +		IFS=: read -a cdset <<< "${*}"  
> 
> -r to avoid handling escapes; -d '' to avoid finishing on newline.

Good call.

> > @@ -243,4 +240,27 @@ _cdrom_locate_file_on_cd() {
> >  	done
> >  }
> >  
> > +# @FUNCTION: _cdrom_glob_match
> > +# @USAGE: <root directory> <path>
> > +# @INTERNAL
> > +# @DESCRIPTION:
> > +# Locates the given path ($2) within the given root directory ($1)
> > +# case-insensitively and returns the first actual matching path. This
> > +# eclass previously used "find -iname" but it only checked the file
> > +# case-insensitively and not the directories. There is "find -ipath" but
> > +# this does not intelligently skip non-matching paths, making it
> > +# slow. Case-insensitive matching can only be applied to patterns so
> > +# extended globbing is used to turn regular strings into patterns. All
> > +# special characters are escaped so don't worry about breaking this. The
> > +# first person to make this work without an eval wins a cookie.
> > +_cdrom_glob_match() {
> > +	local p=\?\($(sed -e 's:[^A-Za-z0-9/]:\\\0:g' -e 's:/:)/?(:g' <<< "$2" || die)\)  
> 
> Explanatory comment needed, i.e. what gets converted into what, and why.

I'll add this:

# The following line turns this:
#  foo*foo/bar bar/baz/file.zip
#
# Into this:
#  ?(foo\*foo)/?(bar\ bar)/?(baz)/?(file\.zip)
#
# This turns every path component into an escaped extended glob
# pattern to allow case-insensitive matching. Globs cannot span
# directories so each component becomes an individual pattern.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

Re: [gentoo-dev] [PATCH 01/14] cdrom.eclass: Detect case-insensitively and handle special characters

[Michał Górny]

Dnia 18 kwietnia 2017 23:31:31 CEST, James Le Cuirot <chewi@gentoo.org> napisał(a):
>On Tue, 18 Apr 2017 08:08:44 +0200
>Michał Górny <mgorny@gentoo.org> wrote:
>
>> On pon, 2017-04-17 at 22:53 +0100, James Le Cuirot wrote:
>> > diff --git a/eclass/cdrom.eclass b/eclass/cdrom.eclass
>> > index 41488d2446c2..de72f15563db 100644
>> > --- a/eclass/cdrom.eclass
>> > +++ b/eclass/cdrom.eclass
>> > @@ -79,12 +79,13 @@ cdrom_get_cds() {
>> >  		export CDROM_ROOT=${CD_ROOT_1:-${CD_ROOT}}
>> >  		einfo "Found CD #${CDROM_CURRENT_CD} root at ${CDROM_ROOT}"
>> >  		export CDROM_SET=-1
>> > -		for f in ${CDROM_CHECK_1//:/ } ; do
>> > +		IFS=:  
>> 
>> 'local', please.
>
>This line disappears later in the series but I've amended it for the
>history anyway.
>
>> > @@ -181,28 +182,24 @@ _cdrom_locate_file_on_cd() {
>> >  	local showedmsg=0 showjolietmsg=0
>> >  
>> >  	while [[ -z ${CDROM_ROOT} ]] ; do
>> > -		local i=0
>> > -		local -a cdset=(${*//:/ })
>> > +		local i=0 cdset
>> > +		IFS=: read -a cdset <<< "${*}"  
>> 
>> -r to avoid handling escapes; -d '' to avoid finishing on newline.
>
>Good call.
>
>> > @@ -243,4 +240,27 @@ _cdrom_locate_file_on_cd() {
>> >  	done
>> >  }
>> >  
>> > +# @FUNCTION: _cdrom_glob_match
>> > +# @USAGE: <root directory> <path>
>> > +# @INTERNAL
>> > +# @DESCRIPTION:
>> > +# Locates the given path ($2) within the given root directory ($1)
>> > +# case-insensitively and returns the first actual matching path.
>This
>> > +# eclass previously used "find -iname" but it only checked the
>file
>> > +# case-insensitively and not the directories. There is "find
>-ipath" but
>> > +# this does not intelligently skip non-matching paths, making it
>> > +# slow. Case-insensitive matching can only be applied to patterns
>so
>> > +# extended globbing is used to turn regular strings into patterns.
>All
>> > +# special characters are escaped so don't worry about breaking
>this. The
>> > +# first person to make this work without an eval wins a cookie.
>> > +_cdrom_glob_match() {
>> > +	local p=\?\($(sed -e 's:[^A-Za-z0-9/]:\\\0:g' -e 's:/:)/?(:g' <<<
>"$2" || die)\)  
>> 
>> Explanatory comment needed, i.e. what gets converted into what, and
>why.
>
>I'll add this:
>
># The following line turns this:
>#  foo*foo/bar bar/baz/file.zip
>#
># Into this:
>#  ?(foo\*foo)/?(bar\ bar)/?(baz)/?(file\.zip)
>#
># This turns every path component into an escaped extended glob
># pattern to allow case-insensitive matching. Globs cannot span
># directories so each component becomes an individual pattern.

Why do you escape pattern chars? Wasn't the variable supposed to be a pattern in the first place?

-- 
Best regards,
Michał Górny (by phone)
-- 
Best regards,
Michał Górny (by phone)

Re: [gentoo-dev] [PATCH 01/14] cdrom.eclass: Detect case-insensitively and handle special characters

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 2933 bytes --]

On Wed, 19 Apr 2017 05:14:34 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> >> > @@ -243,4 +240,27 @@ _cdrom_locate_file_on_cd() {
> >> >  	done
> >> >  }
> >> >  
> >> > +# @FUNCTION: _cdrom_glob_match
> >> > +# @USAGE: <root directory> <path>
> >> > +# @INTERNAL
> >> > +# @DESCRIPTION:
> >> > +# Locates the given path ($2) within the given root directory ($1)
> >> > +# case-insensitively and returns the first actual matching path.  
> >This  
> >> > +# eclass previously used "find -iname" but it only checked the  
> >file  
> >> > +# case-insensitively and not the directories. There is "find  
> >-ipath" but  
> >> > +# this does not intelligently skip non-matching paths, making it
> >> > +# slow. Case-insensitive matching can only be applied to patterns  
> >so  
> >> > +# extended globbing is used to turn regular strings into patterns.  
> >All  
> >> > +# special characters are escaped so don't worry about breaking  
> >this. The  
> >> > +# first person to make this work without an eval wins a cookie.
> >> > +_cdrom_glob_match() {
> >> > +	local p=\?\($(sed -e 's:[^A-Za-z0-9/]:\\\0:g' -e 's:/:)/?(:g' <<<  
> >"$2" || die)\)    
> >> 
> >> Explanatory comment needed, i.e. what gets converted into what, and  
> >why.
> >
> >I'll add this:
> >
> ># The following line turns this:
> >#  foo*foo/bar bar/baz/file.zip
> >#
> ># Into this:
> >#  ?(foo\*foo)/?(bar\ bar)/?(baz)/?(file\.zip)
> >#
> ># This turns every path component into an escaped extended glob
> ># pattern to allow case-insensitive matching. Globs cannot span
> ># directories so each component becomes an individual pattern.  
> 
> Why do you escape pattern chars? Wasn't the variable supposed to be a
> pattern in the first place?

If you mean in the eclass before I changed it then no. In the non
CD_ROOT case, the basename was passed to "find -iname" but this was not
documented. In the CD_ROOT case, the whole thing was passed to
[[ -e ]] so patterns wouldn't have worked here.

You wouldn't want to use a pattern anyway as you're trying to uniquely
identify the disc using a very specific filename. Conversely, I relaxed
case-sensitivity because this can vary depending on whether we're
dealing with the original disc, a copy of some kind, or an existing
installation that may come from Windows.

The Curse of Monkey Island turned out to be a great example. Both discs
have some files in common like COMI.LA0, however, when mounted with the
default options, it appears upper-cased on the first disc but
lower-cased on the second. Why? The second disc doesn't use Joliet as
all the filenames have the old 8.3 format. Linux normalises these to
lower-case. The first disc does use Joliet because of a single file,
"Curse of Monkey Island - Manual.pdf" so all the other 8.3 filename are
left as upper-case by Linux.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]

Re: [gentoo-dev] [PATCH] cdrom.eclass: Near rewrite

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]

On Mon, 17 Apr 2017 22:53:45 +0100
James Le Cuirot <chewi@gentoo.org> wrote:

> If you've been wondering why I've been quiet of late (you have,
> right?!) then this is partly why. I'm not sure why I spent so long on
> an eclass that hardly anyone uses but it's utilised by many of my old
> favourite games.
> 
> The main bug that it fixes is one I filed a report for 10 years ago!
> There were also several duplicates. If you want something done...
> 
> I realised part way through that I didn't have any multi-disc games
> that use this eclass any more. A former friend of mine lost my second
> UT99 disc with the high resolution textures. I couldn't rewrite the
> eclass without testing it thoroughly so I wrote a brand new "comi"
> (Curse of Monkey Island) ebuild for use with ScummVM! I'll get this
> committed when these changes go in. I've also revised all the
> Descent-related packages.

Merged with the suggested fixes.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 981 bytes --]