[PATCH] yet another package check (take 2)

[PATCH] yet another package check (take 2)

[Jan Beulich]

As before, the file added to tools/check/ must be given exec permission.

Now using the approach suggested by Christoph Egger <Christoph.Egger@amd.com>.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

Index: 2006-12-11/tools/Makefile
===================================================================
--- 2006-12-11.orig/tools/Makefile	2006-12-04 08:49:57.000000000 +0100
+++ 2006-12-11/tools/Makefile	2006-12-14 14:25:02.000000000 +0100
@@ -36,7 +36,7 @@ all: check
 	$(MAKE) ioemu
 
 .PHONY: install
-install: check
+install: check_install
 	@set -e; for subdir in $(SUBDIRS-y); do \
 		$(MAKE) -C $$subdir $@; \
 	done
@@ -58,6 +58,10 @@ distclean: clean
 check:
 	$(MAKE) -C check
 
+.PHONY: check_install
+check_install: check
+	$(MAKE) -C check install
+
 .PHONY: check_clean
 check_clean:
 	$(MAKE) -C check clean
Index: 2006-12-11/tools/check/check_python_xml
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ 2006-12-11/tools/check/check_python_xml	2006-12-14 17:04:04.000000000 +0100
@@ -0,0 +1,19 @@
+#!/bin/sh
+# CHECK-INSTALL
+
+RC=0
+
+python -c '
+import os.path, sys
+for p in sys.path:
+	if os.path.exists(p + "/xml/dom/minidom.py"):
+		sys.exit(0)
+sys.exit(1)
+' || RC=1
+
+if test ${RC} -ne 0; then
+	echo
+	echo " *** Check for python development environment FAILED"
+fi
+
+exit ${RC}

Re: [PATCH] yet another package check (take 2)

[Ewan Mellor]

On Thu, Dec 14, 2006 at 04:23:32PM +0000, Jan Beulich wrote:

> As before, the file added to tools/check/ must be given exec permission.
> 
> Now using the approach suggested by Christoph Egger <Christoph.Egger@amd.com>.
>
> [Snip]
>
> Index: 2006-12-11/tools/check/check_python_xml
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ 2006-12-11/tools/check/check_python_xml	2006-12-14 17:04:04.000000000 +0100
> @@ -0,0 +1,19 @@
> +#!/bin/sh
> +# CHECK-INSTALL
> +
> +RC=0
> +
> +python -c '
> +import os.path, sys
> +for p in sys.path:
> +	if os.path.exists(p + "/xml/dom/minidom.py"):
> +		sys.exit(0)
> +sys.exit(1)
> +' || RC=1
> +
> +if test ${RC} -ne 0; then
> +	echo
> +	echo " *** Check for python development environment FAILED"
> +fi
> +
> +exit ${RC}

How about

if ! python -c 'import xml.dom.minidom' 2>/dev/null ; then
  echo
  echo " *** Check for python development environment FAILED"
  exit 1
else
  exit 0
fi

Ewan.

Re: [PATCH] yet another package check (take 2)

[Jan Beulich]

>How about
>
>if ! python -c 'import xml.dom.minidom' 2>/dev/null ; then
>  echo
>  echo " *** Check for python development environment FAILED"
>  exit 1
>else
>  exit 0
>fi

Why not - I didn't invent the other method, I just cloned what we had for
python-devel (which in turn I created based on suggestions from you and
others). I'm not a python person at all, I just want understandable error
messages when dependencies are missing instead of cryptic ones.

Jan

Re: [PATCH] yet another package check (take 2)

[Ewan Mellor]

On Thu, Dec 14, 2006 at 04:59:36PM +0000, Jan Beulich wrote:

> >How about
> >
> >if ! python -c 'import xml.dom.minidom' 2>/dev/null ; then
> >  echo
> >  echo " *** Check for python development environment FAILED"
> >  exit 1
> >else
> >  exit 0
> >fi
> 
> Why not - I didn't invent the other method, I just cloned what we had for
> python-devel (which in turn I created based on suggestions from you and
> others). I'm not a python person at all, I just want understandable error
> messages when dependencies are missing instead of cryptic ones.

I'll put something like that in then.  I think SuSE are the only ones who
split minidom.py out from the main Python package, which is why you're having
the problem ;-)

Ewan.

Re: [PATCH] yet another package check (take 2)

[Ewan Mellor]

On Thu, Dec 14, 2006 at 04:23:32PM +0000, Jan Beulich wrote:

> As before, the file added to tools/check/ must be given exec permission.
> 
> Now using the approach suggested by Christoph Egger <Christoph.Egger@amd.com>.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> Index: 2006-12-11/tools/Makefile
> ===================================================================
> --- 2006-12-11.orig/tools/Makefile	2006-12-04 08:49:57.000000000 +0100
> +++ 2006-12-11/tools/Makefile	2006-12-14 14:25:02.000000000 +0100
> @@ -36,7 +36,7 @@ all: check
>  	$(MAKE) ioemu
>  
>  .PHONY: install
> -install: check
> +install: check_install
>  	@set -e; for subdir in $(SUBDIRS-y); do \
>  		$(MAKE) -C $$subdir $@; \
>  	done
> @@ -58,6 +58,10 @@ distclean: clean
>  check:
>  	$(MAKE) -C check
>  
> +.PHONY: check_install
> +check_install: check
> +	$(MAKE) -C check install
> +
>  .PHONY: check_clean
>  check_clean:
>  	$(MAKE) -C check clean

What's this change for?  In tools/check/Makefile, it says not to run make -C
check install as part of make install.

Ewan.

Re: [PATCH] yet another package check (take 2)

[Jan Beulich]

>>> Ewan Mellor <ewan@xensource.com> 14.12.06 18:11 >>>
>On Thu, Dec 14, 2006 at 04:23:32PM +0000, Jan Beulich wrote:
>
>> As before, the file added to tools/check/ must be given exec permission.
>> 
>> Now using the approach suggested by Christoph Egger <Christoph.Egger@amd.com>.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>> 
>> Index: 2006-12-11/tools/Makefile
>> ===================================================================
>> --- 2006-12-11.orig/tools/Makefile	2006-12-04 08:49:57.000000000 +0100
>> +++ 2006-12-11/tools/Makefile	2006-12-14 14:25:02.000000000 +0100
>> @@ -36,7 +36,7 @@ all: check
>>  	$(MAKE) ioemu
>>  
>>  .PHONY: install
>> -install: check
>> +install: check_install
>>  	@set -e; for subdir in $(SUBDIRS-y); do \
>>  		$(MAKE) -C $$subdir $@; \
>>  	done
>> @@ -58,6 +58,10 @@ distclean: clean
>>  check:
>>  	$(MAKE) -C check
>>  
>> +.PHONY: check_install
>> +check_install: check
>> +	$(MAKE) -C check install
>> +
>>  .PHONY: check_clean
>>  check_clean:
>>  	$(MAKE) -C check clean
>
>What's this change for?  In tools/check/Makefile, it says not to run make -C
>check install as part of make install.

Hmm, I didn't realize this was intentionally not used. For me it simply means I
can't run Xen tools directly out of the build area (since install.sh does nothing
but copying things around, there's little point in using it for other than an 
install into the default (root=/) location. But obviously, when run directly out
of the build area, the install checks will never get done and I'm ending up
with a cryptic error message again.
As I can't think of a smarter way, I think taking this change will depend on
whether it's acceptable to the majority to require all install prerequisites to
be present on build-only machines, and whether it is a reasonable
expectation that at some future point it'll be possible to run multiple Xen
versions on the same machine (and, as per above comment, directly out of
the build area).

Jan

Re: [PATCH] yet another package check (take 2)

[Ewan Mellor]

On Fri, Dec 15, 2006 at 08:22:06AM +0000, Jan Beulich wrote:

> >>> Ewan Mellor <ewan@xensource.com> 14.12.06 18:11 >>>
> >On Thu, Dec 14, 2006 at 04:23:32PM +0000, Jan Beulich wrote:
> >
> >> As before, the file added to tools/check/ must be given exec permission.
> >> 
> >> Now using the approach suggested by Christoph Egger <Christoph.Egger@amd.com>.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> >> 
> >> Index: 2006-12-11/tools/Makefile
> >> ===================================================================
> >> --- 2006-12-11.orig/tools/Makefile	2006-12-04 08:49:57.000000000 +0100
> >> +++ 2006-12-11/tools/Makefile	2006-12-14 14:25:02.000000000 +0100
> >> @@ -36,7 +36,7 @@ all: check
> >>  	$(MAKE) ioemu
> >>  
> >>  .PHONY: install
> >> -install: check
> >> +install: check_install
> >>  	@set -e; for subdir in $(SUBDIRS-y); do \
> >>  		$(MAKE) -C $$subdir $@; \
> >>  	done
> >> @@ -58,6 +58,10 @@ distclean: clean
> >>  check:
> >>  	$(MAKE) -C check
> >>  
> >> +.PHONY: check_install
> >> +check_install: check
> >> +	$(MAKE) -C check install
> >> +
> >>  .PHONY: check_clean
> >>  check_clean:
> >>  	$(MAKE) -C check clean
> >
> >What's this change for?  In tools/check/Makefile, it says not to run make -C
> >check install as part of make install.
> 
> Hmm, I didn't realize this was intentionally not used. For me it simply means I
> can't run Xen tools directly out of the build area (since install.sh does nothing
> but copying things around, there's little point in using it for other than an 
> install into the default (root=/) location. But obviously, when run directly out
> of the build area, the install checks will never get done and I'm ending up
> with a cryptic error message again.

Just run make -C tools/check install.

> As I can't think of a smarter way, I think taking this change will depend on
> whether it's acceptable to the majority to require all install prerequisites to
> be present on build-only machines,

That's certainly not acceptable.  We won't have the bridge-utils on our build
boxes, for instance.

> and whether it is a reasonable
> expectation that at some future point it'll be possible to run multiple Xen
> versions on the same machine (and, as per above comment, directly out of
> the build area).

Why do you want to run them straight out of the build area?  Just do

export DISTDIR=/usr/local/xen-3.0.3
make dist

and you've got everything that you need, right there.  You'll probably have to
fix a few hardcoded paths, but that should be easy.

Ewan.

Re: [PATCH] yet another package check (take 2)

[Keir Fraser]

On 15/12/06 09:33, "Ewan Mellor" <ewan@xensource.com> wrote:

>> As I can't think of a smarter way, I think taking this change will depend on
>> whether it's acceptable to the majority to require all install prerequisites
>> to
>> be present on build-only machines,
> 
> That's certainly not acceptable.  We won't have the bridge-utils on our build
> boxes, for instance.

Perhaps we should have different behaviour between 'make install' and 'make
dist' in this respect? The latter shouldn't run install checks, certainly,
but arguably the former should.

Anyhow, I'll back out the changes to the Makefile for 3.0.4 (which means in
-unstable, for now) and we can revisit this argument for 3.0.5.

 -- Keir

Re: [PATCH] yet another package check (take 2)

[Jan Beulich]

>Why do you want to run them straight out of the build area?  Just do
>
>export DISTDIR=/usr/local/xen-3.0.3
>make dist
>
>and you've got everything that you need, right there.  You'll probably have to
>fix a few hardcoded paths, but that should be easy.

I can't generally see the value of 'installing' if all that means is just copying. I'm
doing this with basically all other components I use non-distro versions of, without
problems. Xen is the first one to cause me real headaches here.

Also, the 'few hardcoded paths' seem to be scattered all around the various tools,
so it doesn't look like a rather simple job to (a) spot them all and (b) fix them.

Jan

Re: [PATCH] yet another package check (take 2)

[Keir Fraser]

On 15/12/06 09:50, "Jan Beulich" <jbeulich@novell.com> wrote:

> I can't generally see the value of 'installing' if all that means is just
> copying. I'm
> doing this with basically all other components I use non-distro versions of,
> without
> problems. Xen is the first one to cause me real headaches here.
> 
> Also, the 'few hardcoded paths' seem to be scattered all around the various
> tools,
> so it doesn't look like a rather simple job to (a) spot them all and (b) fix
> them.

It's been pointed out that the check script is there for you to run from
your own installation wrappers if you want to. Any change to default
behaviour will require us to distinguish 'make dist' and 'make install'
inside the tools directory. That's a 3.0.5 build-system issue imo.

 -- Keir