[PATCH] tools: python portability fixes

[PATCH] tools: python portability fixes

[Christoph Egger]

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


Hi!

Attached patch applies portability fixes for NetBSD:

- remove useless get-path. It is a bash specific script which tries to
  gain information the build system already has.
- make install-wrap work with bourne shell on NetBSD
- pass `which $(PYTHON)` to install-wrap. It is safe to assume
  this always works because in case it doesn't the build system
  errors out very early in tools/check/check_python

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_python.diff --]
[-- Type: text/x-diff, Size: 3058 bytes --]

diff -r bc3aca17cb88 tools/misc/Makefile
--- a/tools/misc/Makefile	Thu Jul 23 09:01:30 2009 +0100
+++ b/tools/misc/Makefile	Thu Jul 23 15:45:48 2009 +0200
@@ -25,8 +25,7 @@ INSTALL_BIN := $(INSTALL_BIN-y)
 INSTALL_SBIN-y := xm xen-bugtool xen-python-path xend xenperf xsview xenpm xen-tmem-list-parse gtraceview gtracestat
 INSTALL_SBIN := $(INSTALL_SBIN-y)
 
-DEFAULT_PYTHON_PATH := $(shell $(XEN_ROOT)/tools/python/get-path)
-PYTHON_PATH ?= $(DEFAULT_PYTHON_PATH)
+PYTHON_PATH := $(shell which $(PYTHON))
 INSTALL_PYTHON_PROG = $(XEN_ROOT)/tools/python/install-wrap \
 "$(PYTHON_PATH)" $(INSTALL_PROG)
 
diff -r bc3aca17cb88 tools/python/get-path
--- a/tools/python/get-path	Thu Jul 23 09:01:30 2009 +0100
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,22 +0,0 @@
-#! /usr/bin/env bash
-set -e
-
-check () {
-	set +e
-	p=`type -p python$v`
-	r=$?
-	set -e
-	if [ $r = 0 ]; then
-		echo >&2 "${0##*/}: will use #!$p for python programs"
-		printf "%s\n" "$p"
-		exit 0
-	fi
-}
-
-v="$(python -V 2>&1)"
-v="${v#* }"
-check
-v="${v%.*}"
-check
-echo >&2 'python version not determined, will use env to find python at runtime'
-printf "/usr/bin/env python\n"
diff -r bc3aca17cb88 tools/python/install-wrap
--- a/tools/python/install-wrap	Thu Jul 23 09:01:30 2009 +0100
+++ b/tools/python/install-wrap	Thu Jul 23 15:45:48 2009 +0200
@@ -1,44 +1,60 @@
-#! /usr/bin/env bash
+#!/bin/sh
 # usage:
 #  .../install-wrap $(PYTHON_PATH) install <options-to-install> <src>... <dest>
 # where
 #  PYTHON_PATH is what to put after #! and may be `/usr/bin/env python'
 #
-# Used via $(INSTALL_PYTHON_PROG) in Rules.mk; PYTHON_PATH comes from
-# .../get-path alongside this script
+# Used via $(INSTALL_PYTHON_PROG) in Rules.mk; PYTHON_PATH comes from $(PYTHON)
 
 set -e
-if [ $# -lt 2 ]; then echo >&2 "${0##*/}: too few arguments"; exit 1; fi
-pythonpath="$1"; shift
+if test $# -lt 2; then
+	echo >&2 "${0##*/}: too few arguments"
+	exit 1
+fi
 
-install=("$1"); shift
-srcs=()
+pythonpath="$1"
+shift
+
+install="$1"
+shift
+srcs=""
 
 while [ $# != 0 ]; do
 	case "$1" in
-	-|--)	install=("${install[@]}" "$1"); shift; break ;;
-	-*)	install=("${install[@]}" "$1"); shift ;;
-	*)	break ;;
+	-|--)	install=`echo "${install} $1"`
+		shift
+		break
+		;;
+	-*)	install=`echo "${install} $1"`
+		shift
+		;;
+	*)	break
+		;;
 	esac
 done
-while [ $# -gt 1 ]; do
-	srcs=("${srcs[@]}" "$1"); shift
+
+while test $# -gt 1; do
+	srcs=`echo "${srcs} $1"`
+	shift
 done
-dest="$1"; shift
+
+dest="$1"
+shift
 
 destf="$dest"
-for srcf in "${srcs[@]}"; do
+for srcf in ${srcs}; do
 	if test -d "$dest"; then
-		destf="$dest/${srcf%%*/}";
+		destf="$dest/${srcf%%*/}"
 	fi
 	org="$(sed -n '2q; /^#! *\/usr\/bin\/env python *$/p' $srcf)"
-	if [ "x$org" = x ]; then
-		"${install[@]}" "$srcf" "$destf"
+	if test "x$org" = x; then
+		eval "${install} $srcf $destf"
 		continue
 	fi
 	tmpf="$destf.tmp"
-	"${install[@]}" "$srcf" "$tmpf"
+	eval "${install} $srcf $tmpf"
 	printf >"$tmpf" "#!%s\n" "$pythonpath"
 	sed -e 1d "$srcf" >>"$tmpf"
 	mv -f "$tmpf" "$destf"
 done
+exit 0

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] tools: python portability fixes

[Keir Fraser]

On 23/07/2009 14:58, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> Attached patch applies portability fixes for NetBSD:
> 
> - remove useless get-path. It is a bash specific script which tries to
>   gain information the build system already has.
> - make install-wrap work with bourne shell on NetBSD
> - pass `which $(PYTHON)` to install-wrap. It is safe to assume
>   this always works because in case it doesn't the build system
>   errors out very early in tools/check/check_python
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

Ian's on holiday until the end of next week so don't expect a swift ack/nack
on this one.

 -- Keir

Re: [PATCH] tools: python portability fixes

[Ian Jackson]

Christoph Egger writes ("[PATCH] tools: python portability fixes"):
> Attached patch applies portability fixes for NetBSD:

Hi.  As Keir says, I'm away, but:

> - remove useless get-path. It is a bash specific script which tries to
>   gain information the build system already has.
> - make install-wrap work with bourne shell on NetBSD
> - pass `which $(PYTHON)` to install-wrap. It is safe to assume
>   this always works because in case it doesn't the build system
>   errors out very early in tools/check/check_python

Isn't the effect of this to stop substituting #!/usr/bin/python2.5 (or
whatever) for #!/usr/bin/python ?  In which case it's wrong as I've
already explained.

Ian.

Re: [PATCH] tools: python portability fixes

[Christoph Egger]

On Monday 27 July 2009 20:57:28 Ian Jackson wrote:
> Christoph Egger writes ("[PATCH] tools: python portability fixes"):
> > Attached patch applies portability fixes for NetBSD:
>
> Hi.  As Keir says, I'm away, but:
> > - remove useless get-path. It is a bash specific script which tries to
> >   gain information the build system already has.
> > - make install-wrap work with bourne shell on NetBSD
> > - pass `which $(PYTHON)` to install-wrap. It is safe to assume
> >   this always works because in case it doesn't the build system
> >   errors out very early in tools/check/check_python
>
> Isn't the effect of this to stop substituting #!/usr/bin/python2.5 (or
> whatever) for #!/usr/bin/python ?  In which case it's wrong as I've
> already explained.

No, the effect is this:

When $(PYTHON) is python you get #!/usr/bin/python,
when $(PYTHON) is python2.5 you get #!/usr/bin/python2.5,
when $(PYTHON) is python2.6 you get #!/usr/bin/python2.6,
when $(PYTHON) is /usr/bin/python2.4 you get #!/usr/bin/python2.4,
etc.

When you install python on NetBSD, you don't have a "python"
binary. You always have pythonN.M where N.M is the version number
so you can have multiple python versions installed.
On NetBSD, you actually get #!/usr/pkg/bin/python2.5.
The get-path is a bash specific script which tries to get the information
`which $(PYTHON)` already provides but doesn't work for the case
where "python" doesn't exist.

The current approach is doomed to always fail on NetBSD.

On Linux you have a python symlink to pythonN.M.
Remove the symlink manually, build and install xen with
gmake PYTHON=pythonN.M and you can reproduce the issue on Linux.

The patch fixes it.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] tools: python portability fixes

[Ian Jackson]

Christoph Egger writes ("Re: [PATCH] tools: python portability fixes"):
> [stuff]

Ah, I see.  I don't think this is right.

> When $(PYTHON) is python you get #!/usr/bin/python,

This, in particularly, is wrong.  But I don't have time right now to
investigate this properly.  Can it wait until I'm back in the office
(this coming Monda) ?

> The get-path is a bash specific script which tries to get the information
                    ^^^^^^^^^^^^^^^^^^^^
While I'm here, though: you keep mentioning this as if there was
something wrong with this.  I think having the Xen build system depend
on bash being installed is absolutely fine.

Ian.

Re: [PATCH] tools: python portability fixes

[Christoph Egger]

On Tuesday 28 July 2009 11:55:59 Ian Jackson wrote:
> Christoph Egger writes ("Re: [PATCH] tools: python portability fixes"):
> > [stuff]
>
> Ah, I see.  I don't think this is right.
>
> > When $(PYTHON) is python you get #!/usr/bin/python,
>
> This, in particularly, is wrong.  But I don't have time right now to
> investigate this properly.  Can it wait until I'm back in the office
> (this coming Monda) ?

yes.

>
> > The get-path is a bash specific script which tries to get the information
>
>                     ^^^^^^^^^^^^^^^^^^^^
> While I'm here, though: you keep mentioning this as if there was
> something wrong with this.  I think having the Xen build system depend
> on bash being installed is absolutely fine.

On NetBSD, bash is a third-party package.
scripts having #!/bin/bash fail with "/bin/bash: No such file or directory"
The bourne shell (#!/bin/sh) is mighty enough to not require bashism.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] tools: python portability fixes

[Ian Jackson]

Christoph Egger writes ("Re: [PATCH] tools: python portability fixes"):
> On Tuesday 28 July 2009 11:55:59 Ian Jackson wrote:
> > While I'm here, though: you keep mentioning this as if there was
> > something wrong with this.  I think having the Xen build system depend
> > on bash being installed is absolutely fine.
> 
> On NetBSD, bash is a third-party package.
> scripts having #!/bin/bash fail with "/bin/bash: No such file or directory"
> The bourne shell (#!/bin/sh) is mighty enough to not require bashism.

I agree that it is sensible not to make scripts bash-specific for no
reason.  However, there are quite a few things that are much easier to
do in bash and require enormous amounts of pratting about to do in
POSIX sh.

Also, if we write #!/bin/sh at the top of the script, we get
complaints from Solaris people if our script is not portable to the
astonishingly ancient and crusty Solaris /bin/sh.

I don't want to get into NetBSD's decision to make bash be `a third
party package' (whatever that means), but surely there are plenty of
things in the Xen build system that are `third party packages'.  You
can't build Xen without hg and git and texinfo and GNU make and so on.
It seems to me that bash is much less of an imposition.

Ian.

Re: [PATCH] tools: python portability fixes

[Christoph Egger]

On Tuesday 04 August 2009 12:32:41 Ian Jackson wrote:
> Christoph Egger writes ("Re: [PATCH] tools: python portability fixes"):
> > On Tuesday 28 July 2009 11:55:59 Ian Jackson wrote:
> > > While I'm here, though: you keep mentioning this as if there was
> > > something wrong with this.  I think having the Xen build system depend
> > > on bash being installed is absolutely fine.
> >
> > On NetBSD, bash is a third-party package.
> > scripts having #!/bin/bash fail with "/bin/bash: No such file or
> > directory" The bourne shell (#!/bin/sh) is mighty enough to not require
> > bashism.
>
> I agree that it is sensible not to make scripts bash-specific for no
> reason.  However, there are quite a few things that are much easier to
> do in bash and require enormous amounts of pratting about to do in
> POSIX sh.
>
> Also, if we write #!/bin/sh at the top of the script, we get
> complaints from Solaris people if our script is not portable to the
> astonishingly ancient and crusty Solaris /bin/sh.

The build system should call shell scripts via $(SHELL).
On Solaris, $(SHELL) points to /bin/bash .

> I don't want to get into NetBSD's decision to make bash be `a third
> party package' (whatever that means),

that means, it's not part of the base system.

> but surely there are plenty of things in the Xen build system
> that are `third party packages'. You can't build Xen without
> hg and git and texinfo and GNU make and so on. 
> It seems to me that bash is much less of an imposition.

Actually the opposite is the case.
bash as a shell is special in how it is used.

You don't type /bin/hg or /bin/git but you type #!/bin/bash in scripts
and that is where it fails.

The portable way is to make it work with POSIX sh and call it with $(SHELL)  
(see above).

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] tools: python portability fixes

[Ian Jackson]

Christoph Egger writes ("Re: [PATCH] tools: python portability fixes"):
> You don't type /bin/hg or /bin/git but you type #!/bin/bash in scripts
> and that is where it fails.

If you actually look at the script you're complaining about, it starts
with
  #! /usr/bin/env bash
which should work just fine.  Right ?

Ian.

Re: [PATCH] tools: python portability fixes

[Christoph Egger]

On Tuesday 04 August 2009 15:03:30 Ian Jackson wrote:
> Christoph Egger writes ("Re: [PATCH] tools: python portability fixes"):
> > You don't type /bin/hg or /bin/git but you type #!/bin/bash in scripts
> > and that is where it fails.
>
> If you actually look at the script you're complaining about, it starts
> with
>   #! /usr/bin/env bash
> which should work just fine.  Right ?

Yes, given that bash is installed. The non-portable part in this line is the 
path to env. In Solaris, env is in /bin.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: Re: [PATCH] tools: python portability fixes

[John Levon]

On Tue, Aug 04, 2009 at 03:40:01PM +0200, Christoph Egger wrote:

> > If you actually look at the script you're complaining about, it starts
> > with
> >   #! /usr/bin/env bash
> > which should work just fine.  Right ?
> 
> Yes, given that bash is installed. The non-portable part in this line is the 
> path to env. In Solaris, env is in /bin.

/usr/bin/env works fine, since /bin is a symlink to /usr/bin

regards
john