public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
From: "Michał Górny" <mgorny@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Cc: James Le Cuirot <chewi@gentoo.org>
Subject: Re: [gentoo-dev] [PATCH 01/14] cdrom.eclass: Detect case-insensitively and handle special characters
Date: Tue, 18 Apr 2017 08:08:44 +0200	[thread overview]
Message-ID: <1492495724.1167.1.camel@gentoo.org> (raw)
In-Reply-To: <20170417215359.30641-2-chewi@gentoo.org>

[-- 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 --]

  reply	other threads:[~2017-04-18  6:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-17 21:53 [gentoo-dev] [PATCH] cdrom.eclass: Near rewrite James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 01/14] cdrom.eclass: Detect case-insensitively and handle special characters James Le Cuirot
2017-04-18  6:08   ` Michał Górny [this message]
2017-04-18 21:31     ` James Le Cuirot
2017-04-19  3:14       ` Michał Górny
2017-04-19 21:50         ` James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 02/14] cdrom.eclass: Simplify printing of CD_ROOT_# variable names James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 03/14] cdrom.eclass: Rename CDROM_NAME_SET array to CDROM_NAMES James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 04/14] cdrom.eclass: Allow CDROM_NAMES changes before each cdrom_load_next_cd James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 05/14] cdrom.eclass: Remove ye olde Submount check James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 06/14] cdrom.eclass: Simplify loop with seq James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 07/14] cdrom.eclass: We don't know for sure how many discs will be needed James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 08/14] cdrom.eclass: Fix important typo in the multiple disc instructions James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 09/14] cdrom.eclass: Change CDROM_CHECK_# variables to a CDROM_CHECKS array James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 10/14] cdrom.eclass: The CDROM_TOTAL_CDS variable is redundant now James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 11/14] cdrom.eclass: Make CD_ROOT less of a special case, fixes #195868 James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 12/14] cdrom.eclass: Use consistent terminology in prompts and messages James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 13/14] cdrom.eclass: Update and improve documentation following changes James Le Cuirot
2017-04-17 21:53 ` [gentoo-dev] [PATCH 14/14] cdrom.eclass: Update copyright year James Le Cuirot
2017-04-18  5:51 ` [gentoo-dev] [PATCH] cdrom.eclass: Near rewrite Ulrich Mueller
2017-04-18  6:18   ` Michał Górny
2017-04-18 21:01   ` James Le Cuirot
2017-04-27 21:55 ` James Le Cuirot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1492495724.1167.1.camel@gentoo.org \
    --to=mgorny@gentoo.org \
    --cc=chewi@gentoo.org \
    --cc=gentoo-dev@lists.gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox