On Fri, 04 Jan 2019 07:02:40 +0100
Michał Górny <mgo...@gentoo.org> wrote:

> On Thu, 2019-01-03 at 21:39 +0000, James Le Cuirot wrote:
> > The previous approach would erroneously match foopython. The new
> > approach requires the match to start the string or be preceeded by a
> > slash, the only two cases we actually want. It does this with slightly
> > less code and allows the replacement of whole path strings that would
> > be problematic when passed to sed. This will be needed when
> > cross-compiling is addressed.
> > 
> > Signed-off-by: James Le Cuirot <ch...@gentoo.org>
> > ---
> >  eclass/python-utils-r1.eclass | 78 ++++++++++++++---------------------
> >  1 file changed, 31 insertions(+), 47 deletions(-)
> > 
> > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > index da76a755fb34..1eca0764a202 100644
> > --- a/eclass/python-utils-r1.eclass
> > +++ b/eclass/python-utils-r1.eclass
> > @@ -1165,8 +1165,7 @@ python_fix_shebang() {
> >             [[ -d ${path} ]] && is_recursive=1
> >  
> >             while IFS= read -r -d '' f; do
> > -                   local shebang i
> > -                   local error= from=
> > +                   local shebang i= error= fix=
> >  
> >                     # note: we can't ||die here since read will fail if file
> >                     # has no newline characters
> > @@ -1175,65 +1174,56 @@ python_fix_shebang() {
> >                     # First, check if it's shebang at all...
> >                     if [[ ${shebang} == '#!'* ]]; then
> >                             local split_shebang=()
> > -                           read -r -a split_shebang <<<${shebang} || die
> > +                           read -r -a split_shebang <<<${shebang#\#\!} || 
> > die  
> 
> Does '!' really need to be escaped?

Seemingly only in an interactive shell.

> >  
> >                             # Match left-to-right in a loop, to avoid 
> > matching random
> >                             # repetitions like 'python2.7 python2'.
> > -                           for i in "${split_shebang[@]}"; do
> > -                                   case "${i}" in
> > -                                           *"${EPYTHON}")
> > +                           for i in $(seq 0 $((${#split_shebang[@]} - 
> > 1))); do  
> 
> for i in "${!split_shebang[@]}"; do

Interesting, didn't know about that.

> > +                                   case "/${split_shebang[${i}]}" in  
> 
> case "/${split_shebang[i]}" in

...or that.

> Also below.
> 
> > +                                           */${EPYTHON})
> >                                                     debug-print 
> > "${FUNCNAME}: in file ${f#${D%/}}"
> >                                                     debug-print 
> > "${FUNCNAME}: shebang matches EPYTHON: ${shebang}"
> >  
> >                                                     # Nothing to do, move 
> > along.
> >                                                     any_correct=1
> > -                                                   from=${EPYTHON}
> > +                                                   continue 2
> > +                                                   ;;
> > +                                           */python)
> > +                                                   fix=1
> >                                                     break
> >                                                     ;;
> > -                                           *python|*python[23])
> > -                                                   debug-print 
> > "${FUNCNAME}: in file ${f#${D%/}}"
> > -                                                   debug-print 
> > "${FUNCNAME}: rewriting shebang: ${shebang}"
> > -
> > -                                                   if [[ ${i} == *python2 
> > ]]; then
> > -                                                           from=python2
> > -                                                           if [[ ! 
> > ${force} ]]; then
> > -                                                                   
> > python_is_python3 "${EPYTHON}" && error=1
> > -                                                           fi
> > -                                                   elif [[ ${i} == 
> > *python3 ]]; then
> > -                                                           from=python3
> > -                                                           if [[ ! 
> > ${force} ]]; then
> > -                                                                   
> > python_is_python3 "${EPYTHON}" || error=1
> > -                                                           fi
> > -                                                   else
> > -                                                           from=python
> > +                                           */python2)
> > +                                                   if [[ ! ${force} ]]; 
> > then
> > +                                                           
> > python_is_python3 "${EPYTHON}" && error=1
> >                                                     fi
> > +                                                   fix=1
> >                                                     break
> >                                                     ;;
> > -                                           
> > *python[23].[0123456789]|*pypy|*pypy3|*jython[23].[0123456789])
> > -                                                   # Explicit mismatch.
> > +                                           */python3)
> >                                                     if [[ ! ${force} ]]; 
> > then
> > -                                                           error=1
> > -                                                   else
> > -                                                           case "${i}" in
> > -                                                                   
> > *python[23].[0123456789])
> > -                                                                           
> > from="python[23].[0123456789]";;
> > -                                                                   *pypy)
> > -                                                                           
> > from="pypy";;
> > -                                                                   *pypy3)
> > -                                                                           
> > from="pypy3";;
> > -                                                                   
> > *jython[23].[0123456789])
> > -                                                                           
> > from="jython[23].[0123456789]";;
> > -                                                                   *)
> > -                                                                           
> > die "${FUNCNAME}: internal error in 2nd pattern match";;
> > -                                                           esac
> > +                                                           
> > python_is_python3 "${EPYTHON}" || error=1
> >                                                     fi
> > +                                                   fix=1
> > +                                                   break
> > +                                                   ;;
> > +                                           
> > */python[2-3].[0-9]|*/pypy|*/pypy3|*/jython[2-3].[0-9])
> > +                                                   # Explicit mismatch.
> > +                                                   [[ ! ${force} ]] && 
> > error=1
> > +                                                   fix=1
> >                                                     break
> >                                                     ;;
> >                                     esac
> >                             done
> >                     fi
> >  
> > -                   if [[ ! ${error} && ! ${from} ]]; then
> > +                   if [[ ${fix} ]]; then  
> 
> Doesn't this mean errors from above code will be ignored?  Diff makes it
> really hard to follow the logic here but I'm pretty sure you set both
> error=1 fix=1, then check for fix first.

It will still hit the error clause below regardless. The debug-print
lines were refactored from further up and they were printed even in
error cases. We could skip the split_shebang lines but it would just
make the code longer.

> > +                           debug-print "${FUNCNAME}: in file ${f#${D%/}}"
> > +                           debug-print "${FUNCNAME}: rewriting shebang: 
> > ${shebang}"
> > +
> > +                           split_shebang[${i}]=/${split_shebang[${i}]}  
> 
> A comment above this hack would be helpful.

Okay.
 
> > +                           split_shebang[${i}]=${split_shebang[${i}]%/*}
> > +                           
> > split_shebang[${i}]=${split_shebang[${i}]#/}${split_shebang[${i}]:+/}${EPYTHON}
> > +                   elif [[ ! ${error} ]]; then
> >                             # Non-Python shebang. Allowed in recursive mode,
> >                             # disallowed when specifying file explicitly.
> >                             [[ ${is_recursive} ]] && continue
> > @@ -1245,13 +1235,7 @@ python_fix_shebang() {
> >                     fi
> >  
> >                     if [[ ! ${error} ]]; then
> > -                           # We either want to match ${from} followed by 
> > space
> > -                           # or at end-of-string.
> > -                           if [[ ${shebang} == *${from}" "* ]]; then
> > -                                   sed -i -e "1s:${from} :${EPYTHON} :" 
> > "${f}" || die
> > -                           else
> > -                                   sed -i -e "1s:${from}$:${EPYTHON}:" 
> > "${f}" || die
> > -                           fi
> > +                           sed -i -e "1c#\!${split_shebang[*]}" "${f}" || 
> > die
> >                             any_fixed=1
> >                     else
> >                             eerror "The file has incompatible shebang:"  
> 
> I'll get to other patches later.
> 


-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

Attachment: pgpFf_ZibN5Fe.pgp
Description: OpenPGP digital signature

Reply via email to