Message ID | 20100121084406.GA3644@bee.dooz.org |
---|---|
State | New |
Headers | show |
Loïc Minier <lool@dooz.org> wrote: > On Wed, Jan 20, 2010, Måns Rullgård wrote: >> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe. >> Likewise if you set the value first. > > Ok; see attached patches I still think that path_of is a complication that we don't really need. "type" command exist in posix, and althought its output is not defined, if the output of a real command don't show the path that we need (in this case /usr/bin/install), then type is pretty wrong in my humble opinion. With respect of the rest of the patch. Only real difference is that mine uses "prog_exists" vs yours use "has" name. I obviosly think that prog_exists is a more descriptive name, but I really don't care one way or another. Can we agree that we don't need path_of, and then just stop playing with IFS and friends? Later, Juan.
Juan Quintela <quintela@redhat.com> writes: > Loïc Minier <lool@dooz.org> wrote: >> On Wed, Jan 20, 2010, Måns Rullgård wrote: >>> If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe. >>> Likewise if you set the value first. >> >> Ok; see attached patches > > I still think that path_of is a complication that we don't really need. > > "type" command exist in posix, and althought its output is not defined, > if the output of a real command don't show the path that we need (in > this case /usr/bin/install), then type is pretty wrong in my humble > opinion. I think that entire test is wrong, in fact. It is perfectly possible for someone on Solaris to install a working "install" command in /usr/bin. It is better, if possible, to test whatever "install" command is in the path, and complain only if it actually fails.
On Thu, Jan 21, 2010, Måns Rullgård wrote: > I think that entire test is wrong, in fact. It is perfectly possible > for someone on Solaris to install a working "install" command in > /usr/bin. It is better, if possible, to test whatever "install" > command is in the path, and complain only if it actually fails. As I said, I prefer not changing this without access to a Solaris platform; would you mind committing the patch as is, and requesting whoever is interested in the Solaris specific code in qemu to fix the remainder? The patch is a net improvement over what's in ./configure. Thanks
On Tue, Jan 26, 2010 at 6:47 PM, Loïc Minier <lool@dooz.org> wrote: > On Thu, Jan 21, 2010, Måns Rullgård wrote: >> I think that entire test is wrong, in fact. It is perfectly possible >> for someone on Solaris to install a working "install" command in >> /usr/bin. It is better, if possible, to test whatever "install" >> command is in the path, and complain only if it actually fails. > > As I said, I prefer not changing this without access to a Solaris > platform; would you mind committing the patch as is, and requesting > whoever is interested in the Solaris specific code in qemu to fix the > remainder? The patch is a net improvement over what's in ./configure. The patches didn't apply. Also please send only one patch per message, git am can't handle multiple patches.
On Tue, Jan 26, 2010, Blue Swirl wrote: > The patches didn't apply. Also please send only one patch per message, > git am can't handle multiple patches. They applied fine here, perhaps you didn't apply the sdl-config patch first? I rebased them and resent them.
On Wed, Jan 27, 2010 at 12:41 PM, Loïc Minier <lool@dooz.org> wrote: > On Tue, Jan 26, 2010, Blue Swirl wrote: >> The patches didn't apply. Also please send only one patch per message, >> git am can't handle multiple patches. > > They applied fine here, perhaps you didn't apply the sdl-config patch > first? I rebased them and resent them. That must've been it. But I get this on Milax: config-host.mak is out-of-date, running configure /bin/ginstall: cannot stat `=': No such file or directory Configuration and build succeeds though.
From 430b06dcc84e82987d0146ef92dddbe838d6a117 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org> Date: Wed, 20 Jan 2010 12:35:54 +0100 Subject: [PATCH 2/2] Solaris: test for presence of commands with has() --- configure | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/configure b/configure index eb02de3..f59f3e2 100755 --- a/configure +++ b/configure @@ -799,21 +799,19 @@ fi # Solaris specific configure tool chain decisions # if test "$solaris" = "yes" ; then - solinst=`path_of $install` - if test -z "$solinst" ; then + if ! has $install; then echo "Solaris install program not found. Use --install=/usr/ucb/install or" echo "install fileutils from www.blastwave.org using pkg-get -i fileutils" echo "to get ginstall which is used by default (which lives in /opt/csw/bin)" exit 1 fi - if test "$solinst" = "/usr/sbin/install" ; then + if "`path_of $install`" = "/usr/sbin/install" ; then echo "Error: Solaris /usr/sbin/install is not an appropriate install program." echo "try ginstall from the GNU fileutils available from www.blastwave.org" echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install" exit 1 fi - sol_ar=`path_of ar` - if test -z "$sol_ar" ; then + if ! has ar; then echo "Error: No path includes ar" if test -f /usr/ccs/bin/ar ; then echo "Add /usr/ccs/bin to your path and rerun configure" -- 1.6.5