[PATCH] Make sure to use tools as found by configure

[PATCH] Make sure to use tools as found by configure

[Christoph Egger]

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


Hi,

attached patch makes the build process work as described at
http://wiki.xen.org/wiki/Compiling_Xen_From_Source_on_NetBSD

Without this patch compiling the xen kernel fails with
python: No such file or directory

Note, this patch enforces to run configure before you
can compile the xen kernel.
This is already required to build tools and stubdom anyway.

This makes an other patch work as submitted:
http://lists.xen.org/archives/html/xen-devel/2013-04/msg02098.html

Christoph


commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
Author: Christoph Egger <chegger@amazon.de>
Date:   Thu Feb 7 14:29:19 2013 +0000

    Make sure to use tools as found by configure.
    Fold inclusion of Tools.mk into toplevel Config.mk.

    Signed-off-by: Christoph Egger <chegger@amazon.de>
    Reviewed-by: Matthew Wilson <msw@amazon.com>

diff --git a/Config.mk b/Config.mk
index b1f12ec..149f55a 100644
--- a/Config.mk
+++ b/Config.mk
@@ -7,6 +7,7 @@ endif
 # fallback for older make
 realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file))
&& echo "$$PWD/$(notdir $(file))")))

+-include $(XEN_ROOT)/config/Tools.mk
 -include $(XEN_ROOT)/.config

 # A debug build of Xen and tools?
@@ -76,7 +77,6 @@ EXTRA_INCLUDES += $(EXTRA_PREFIX)/include
 EXTRA_LIB += $(EXTRA_PREFIX)/lib
 endif

-PYTHON      ?= python
 PYTHON_PREFIX_ARG ?= --prefix="$(PREFIX)"
 # The above requires that PREFIX contains *no spaces*. This variable is
here
 # to permit the user to set PYTHON_PREFIX_ARG to '' to workaround this bug:
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 3f03a31..7694a7f 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -3,7 +3,6 @@
 # `all' is the default target
 all:

--include $(XEN_ROOT)/config/Tools.mk
 include $(XEN_ROOT)/Config.mk

 export _INSTALL := $(INSTALL)



[-- Attachment #2: patch_usetools.diff --]
[-- Type: text/plain, Size: 1258 bytes --]

commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
Author: Christoph Egger <chegger@amazon.de>
Date:   Thu Feb 7 14:29:19 2013 +0000

    Make sure to use tools as found by configure.
    Fold inclusion of Tools.mk into toplevel Config.mk.
    
    Signed-off-by: Christoph Egger <chegger@amazon.de>

diff --git a/Config.mk b/Config.mk
index b1f12ec..149f55a 100644
--- a/Config.mk
+++ b/Config.mk
@@ -7,6 +7,7 @@ endif
 # fallback for older make
 realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo "$$PWD/$(notdir $(file))")))
 
+-include $(XEN_ROOT)/config/Tools.mk
 -include $(XEN_ROOT)/.config
 
 # A debug build of Xen and tools?
@@ -76,7 +77,6 @@ EXTRA_INCLUDES += $(EXTRA_PREFIX)/include
 EXTRA_LIB += $(EXTRA_PREFIX)/lib
 endif
 
-PYTHON      ?= python
 PYTHON_PREFIX_ARG ?= --prefix="$(PREFIX)"
 # The above requires that PREFIX contains *no spaces*. This variable is here
 # to permit the user to set PYTHON_PREFIX_ARG to '' to workaround this bug:
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 3f03a31..7694a7f 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -3,7 +3,6 @@
 # `all' is the default target
 all:
 
--include $(XEN_ROOT)/config/Tools.mk
 include $(XEN_ROOT)/Config.mk
 
 export _INSTALL := $(INSTALL)

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

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

Re: [PATCH] Make sure to use tools as found by configure

[Ian Campbell]

On Fri, 2013-05-10 at 12:59 +0100, Christoph Egger wrote:
> Hi,
> 
> attached patch makes the build process work as described at
> http://wiki.xen.org/wiki/Compiling_Xen_From_Source_on_NetBSD
> 
> Without this patch compiling the xen kernel fails with
> python: No such file or directory

> 
> Note, this patch enforces to run configure before you
> can compile the xen kernel.

> This is already required to build tools and stubdom anyway.

The hypervisor maintainers have explicitly required that configure not
be required to build the hypervisor. You can be pretty sure they will
NAK this change, which you didn't even CC to them, I have added them
now.

If the instructions above do not work then I think you need to fix them.

> This makes an other patch work as submitted:
> http://lists.xen.org/archives/html/xen-devel/2013-04/msg02098.html

That patch remains wrong. Irrespective of this change the docs subtree
should include its own Perl detection stuff in the configure script,
such that it can be run independently. I explained this in my original
reply to that patch.

Ian.

Re: [PATCH] Make sure to use tools as found by configure

[Tim Deegan]

At 13:14 +0100 on 10 May (1368191687), Ian Campbell wrote:
> On Fri, 2013-05-10 at 12:59 +0100, Christoph Egger wrote:
> > Hi,
> > 
> > attached patch makes the build process work as described at
> > http://wiki.xen.org/wiki/Compiling_Xen_From_Source_on_NetBSD
> > 
> > Without this patch compiling the xen kernel fails with
> > python: No such file or directory
> 
> > 
> > Note, this patch enforces to run configure before you
> > can compile the xen kernel.
> 
> > This is already required to build tools and stubdom anyway.
> 
> The hypervisor maintainers have explicitly required that configure not
> be required to build the hypervisor. You can be pretty sure they will
> NAK this change, which you didn't even CC to them, I have added them
> now.

Indeed, on that basis: Nack.  Please confine autoconf-related goop to
the parts of the tree that use it.

Tim.

Re: [PATCH] Make sure to use tools as found by configure

[Matt Wilson]

On Fri, May 10, 2013 at 01:49:17PM +0100, Tim Deegan wrote:
> At 13:14 +0100 on 10 May (1368191687), Ian Campbell wrote:
> > On Fri, 2013-05-10 at 12:59 +0100, Christoph Egger wrote:
> > > 
> > > Note, this patch enforces to run configure before you
> > > can compile the xen kernel.
> > 
> > The hypervisor maintainers have explicitly required that configure not
> > be required to build the hypervisor. You can be pretty sure they will
> > NAK this change, which you didn't even CC to them, I have added them
> > now.
> 
> Indeed, on that basis: Nack.  Please confine autoconf-related goop to
> the parts of the tree that use it.

I don't want to dog pile, but I agree: Nack, autoconf shouldn't be
required for the hypervisor tree.

--msw

Re: [PATCH] Make sure to use tools as found by configure

[Matt Wilson]

On Fri, May 10, 2013 at 01:59:53PM +0200, Christoph Egger wrote:
> 
> Hi,
> 
> attached patch makes the build process work as described at
> http://wiki.xen.org/wiki/Compiling_Xen_From_Source_on_NetBSD
> 
> Without this patch compiling the xen kernel fails with
> python: No such file or directory
> 
> Note, this patch enforces to run configure before you
> can compile the xen kernel.
> This is already required to build tools and stubdom anyway.
> 
> This makes an other patch work as submitted:
> http://lists.xen.org/archives/html/xen-devel/2013-04/msg02098.html
> 
> Christoph
> 
> 
> commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
> Author: Christoph Egger <chegger@amazon.de>
> Date:   Thu Feb 7 14:29:19 2013 +0000
> 
>     Make sure to use tools as found by configure.
>     Fold inclusion of Tools.mk into toplevel Config.mk.
> 
>     Signed-off-by: Christoph Egger <chegger@amazon.de>
>     Reviewed-by: Matthew Wilson <msw@amazon.com>

To be clear, I Nack'ed this in review and gave two options:

1) rework the patch before posting
2) propose the patch on xen-devel for comment from hypervisor
maintainers

Matt

Re: [PATCH] Make sure to use tools as found by configure

[Ian Campbell]

On Fri, 2013-05-10 at 18:10 +0100, Matt Wilson wrote:
> On Fri, May 10, 2013 at 01:59:53PM +0200, Christoph Egger wrote:
> > 
> > Hi,
> > 
> > attached patch makes the build process work as described at
> > http://wiki.xen.org/wiki/Compiling_Xen_From_Source_on_NetBSD
> > 
> > Without this patch compiling the xen kernel fails with
> > python: No such file or directory
> > 
> > Note, this patch enforces to run configure before you
> > can compile the xen kernel.
> > This is already required to build tools and stubdom anyway.
> > 
> > This makes an other patch work as submitted:
> > http://lists.xen.org/archives/html/xen-devel/2013-04/msg02098.html
> > 
> > Christoph
> > 
> > 
> > commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
> > Author: Christoph Egger <chegger@amazon.de>
> > Date:   Thu Feb 7 14:29:19 2013 +0000
> > 
> >     Make sure to use tools as found by configure.
> >     Fold inclusion of Tools.mk into toplevel Config.mk.
> > 
> >     Signed-off-by: Christoph Egger <chegger@amazon.de>
> >     Reviewed-by: Matthew Wilson <msw@amazon.com>
> 
> To be clear, I Nack'ed this in review and gave two options:

I was about to query this, thanks for clarifying.

Christoph, please be more careful in future not to misrepresent peoples
review.

In general I would think it a good idea if Reviewed-by tags are posted
publicly by the Reviewer on xen-devel, even if the review was carried
out internally prior to posting, this would help avoid this sort of
issue. Not a rule I don't think, but would help avoid mistakes...

Ian.

Re: [PATCH] Make sure to use tools as found by configure

[Christoph Egger]

On 13.05.13 11:51, Ian Campbell wrote:
> On Fri, 2013-05-10 at 18:10 +0100, Matt Wilson wrote:
>> On Fri, May 10, 2013 at 01:59:53PM +0200, Christoph Egger wrote:
>>>
>>> Hi,
>>>
>>> attached patch makes the build process work as described at
>>> http://wiki.xen.org/wiki/Compiling_Xen_From_Source_on_NetBSD
>>>
>>> Without this patch compiling the xen kernel fails with
>>> python: No such file or directory
>>>
>>> Note, this patch enforces to run configure before you
>>> can compile the xen kernel.
>>> This is already required to build tools and stubdom anyway.
>>>
>>> This makes an other patch work as submitted:
>>> http://lists.xen.org/archives/html/xen-devel/2013-04/msg02098.html
>>>
>>> Christoph
>>>
>>>
>>> commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
>>> Author: Christoph Egger <chegger@amazon.de>
>>> Date:   Thu Feb 7 14:29:19 2013 +0000
>>>
>>>     Make sure to use tools as found by configure.
>>>     Fold inclusion of Tools.mk into toplevel Config.mk.
>>>
>>>     Signed-off-by: Christoph Egger <chegger@amazon.de>
>>>     Reviewed-by: Matthew Wilson <msw@amazon.com>
>>
>> To be clear, I Nack'ed this in review and gave two options:
> 
> I was about to query this, thanks for clarifying.
> 
> Christoph, please be more careful in future not to misrepresent peoples
> review.
> 
> In general I would think it a good idea if Reviewed-by tags are posted
> publicly by the Reviewer on xen-devel, even if the review was carried
> out internally prior to posting, this would help avoid this sort of
> issue. Not a rule I don't think, but would help avoid mistakes...

Understood.

Christoph

Re: [PATCH] Make sure to use tools as found by configure

[Matt Wilson]

On Mon, May 13, 2013 at 10:51:51AM +0100, Ian Campbell wrote:
> On Fri, 2013-05-10 at 18:10 +0100, Matt Wilson wrote:
> > On Fri, May 10, 2013 at 01:59:53PM +0200, Christoph Egger wrote:
> > > 
> > > commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
> > > Author: Christoph Egger <chegger@amazon.de>
> > > Date:   Thu Feb 7 14:29:19 2013 +0000
> > > 
> > >     Make sure to use tools as found by configure.
> > >     Fold inclusion of Tools.mk into toplevel Config.mk.
> > > 
> > >     Signed-off-by: Christoph Egger <chegger@amazon.de>
> > >     Reviewed-by: Matthew Wilson <msw@amazon.com>
> > 
> > To be clear, I Nack'ed this in review and gave two options:
> 
> I was about to query this, thanks for clarifying.
> 
> Christoph, please be more careful in future not to misrepresent peoples
> review.
> 
> In general I would think it a good idea if Reviewed-by tags are posted
> publicly by the Reviewer on xen-devel, even if the review was carried
> out internally prior to posting, this would help avoid this sort of
> issue. Not a rule I don't think, but would help avoid mistakes...

I think it could help avoid mistakes, but it might cause some
complications. I asked Christoph to post patches with the appropriate
Reviewed-by:/Acked-by:/etc. line because I'm in a timezone that's
fairly far from him and most committers. I sometimes get behind on
xen-devel mail and it's possible that a committer might commit a
posted patch before I reply on the list, and we'd lose a valuable bit
of audit trail in the history.

--msw

Re: [PATCH] Make sure to use tools as found by configure

[Ian Campbell]

On Mon, 2013-05-13 at 21:32 +0100, Matt Wilson wrote:
> On Mon, May 13, 2013 at 10:51:51AM +0100, Ian Campbell wrote:
> > On Fri, 2013-05-10 at 18:10 +0100, Matt Wilson wrote:
> > > On Fri, May 10, 2013 at 01:59:53PM +0200, Christoph Egger wrote:
> > > > 
> > > > commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
> > > > Author: Christoph Egger <chegger@amazon.de>
> > > > Date:   Thu Feb 7 14:29:19 2013 +0000
> > > > 
> > > >     Make sure to use tools as found by configure.
> > > >     Fold inclusion of Tools.mk into toplevel Config.mk.
> > > > 
> > > >     Signed-off-by: Christoph Egger <chegger@amazon.de>
> > > >     Reviewed-by: Matthew Wilson <msw@amazon.com>
> > > 
> > > To be clear, I Nack'ed this in review and gave two options:
> > 
> > I was about to query this, thanks for clarifying.
> > 
> > Christoph, please be more careful in future not to misrepresent peoples
> > review.
> > 
> > In general I would think it a good idea if Reviewed-by tags are posted
> > publicly by the Reviewer on xen-devel, even if the review was carried
> > out internally prior to posting, this would help avoid this sort of
> > issue. Not a rule I don't think, but would help avoid mistakes...
> 
> I think it could help avoid mistakes, but it might cause some
> complications. I asked Christoph to post patches with the appropriate
> Reviewed-by:/Acked-by:/etc. line

I'm confused, you Nacked this patch and then asked Christoph to post it
with your Reviewed-by anyway?

>  because I'm in a timezone that's
> fairly far from him and most committers. I sometimes get behind on
> xen-devel mail and it's possible that a committer might commit a
> posted patch before I reply on the list, and we'd lose a valuable bit
> of audit trail in the history.

I'm not sure what you are worried about here, we could always revert if
when you catch up you aren't happy with the commit.

Ian.

Re: [PATCH] Make sure to use tools as found by configure

[Christoph Egger]

On 14.05.13 10:44, Ian Campbell wrote:
> On Mon, 2013-05-13 at 21:32 +0100, Matt Wilson wrote:
>> On Mon, May 13, 2013 at 10:51:51AM +0100, Ian Campbell wrote:
>>> On Fri, 2013-05-10 at 18:10 +0100, Matt Wilson wrote:
>>>> On Fri, May 10, 2013 at 01:59:53PM +0200, Christoph Egger wrote:
>>>>>
>>>>> commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
>>>>> Author: Christoph Egger <chegger@amazon.de>
>>>>> Date:   Thu Feb 7 14:29:19 2013 +0000
>>>>>
>>>>>     Make sure to use tools as found by configure.
>>>>>     Fold inclusion of Tools.mk into toplevel Config.mk.
>>>>>
>>>>>     Signed-off-by: Christoph Egger <chegger@amazon.de>
>>>>>     Reviewed-by: Matthew Wilson <msw@amazon.com>
>>>>
>>>> To be clear, I Nack'ed this in review and gave two options:
>>>
>>> I was about to query this, thanks for clarifying.
>>>
>>> Christoph, please be more careful in future not to misrepresent peoples
>>> review.
>>>
>>> In general I would think it a good idea if Reviewed-by tags are posted
>>> publicly by the Reviewer on xen-devel, even if the review was carried
>>> out internally prior to posting, this would help avoid this sort of
>>> issue. Not a rule I don't think, but would help avoid mistakes...
>>
>> I think it could help avoid mistakes, but it might cause some
>> complications. I asked Christoph to post patches with the appropriate
>> Reviewed-by:/Acked-by:/etc. line
> 
> I'm confused, you Nacked this patch and then asked Christoph to post it
> with your Reviewed-by anyway?

I think, this is my fault. We agreed on moving the discussion about the
patch to xen-devel and not the patch itself.

One reason for my mistake is that I didn't understand the design of
the build system. I was thinking in: there is a hypervisor and a
tools part. But in real there is a hypervisor part, a tools part,
a stubdom part and a docs part and each part has its own configure
except the hypervisor part.

Christoph

> 
>>  because I'm in a timezone that's
>> fairly far from him and most committers. I sometimes get behind on
>> xen-devel mail and it's possible that a committer might commit a
>> posted patch before I reply on the list, and we'd lose a valuable bit
>> of audit trail in the history.
> 
> I'm not sure what you are worried about here, we could always revert if
> when you catch up you aren't happy with the commit.
> 
> Ian.
> 
> 

Re: [PATCH] Make sure to use tools as found by configure

[Matt Wilson]

On Tue, May 14, 2013 at 09:44:23AM +0100, Ian Campbell wrote:
> On Mon, 2013-05-13 at 21:32 +0100, Matt Wilson wrote:
> > On Mon, May 13, 2013 at 10:51:51AM +0100, Ian Campbell wrote:
> > > On Fri, 2013-05-10 at 18:10 +0100, Matt Wilson wrote:
> > > > On Fri, May 10, 2013 at 01:59:53PM +0200, Christoph Egger wrote:
> > > > > 
> > > > > commit 7172e6e0020328d14638a0bbb66a52c905cb4b0b
> > > > > Author: Christoph Egger <chegger@amazon.de>
> > > > > Date:   Thu Feb 7 14:29:19 2013 +0000
> > > > > 
> > > > >     Make sure to use tools as found by configure.
> > > > >     Fold inclusion of Tools.mk into toplevel Config.mk.
> > > > > 
> > > > >     Signed-off-by: Christoph Egger <chegger@amazon.de>
> > > > >     Reviewed-by: Matthew Wilson <msw@amazon.com>
> > > > 
> > > > To be clear, I Nack'ed this in review and gave two options:
> > > 
> > > I was about to query this, thanks for clarifying.
> > > 
> > > Christoph, please be more careful in future not to misrepresent peoples
> > > review.
> > > 
> > > In general I would think it a good idea if Reviewed-by tags are posted
> > > publicly by the Reviewer on xen-devel, even if the review was carried
> > > out internally prior to posting, this would help avoid this sort of
> > > issue. Not a rule I don't think, but would help avoid mistakes...
> > 
> > I think it could help avoid mistakes, but it might cause some
> > complications. I asked Christoph to post patches with the appropriate
> > Reviewed-by:/Acked-by:/etc. line
> 
> I'm confused, you Nacked this patch and then asked Christoph to post it
> with your Reviewed-by anyway?

No, that was a misunderstanding by Christoph. I asked that he post the
patches with either "Acked-by:" or "Reviewed-by:" when those are given
in our internal review. I gave neither in this case.

> >  because I'm in a timezone that's
> > fairly far from him and most committers. I sometimes get behind on
> > xen-devel mail and it's possible that a committer might commit a
> > posted patch before I reply on the list, and we'd lose a valuable bit
> > of audit trail in the history.
> 
> I'm not sure what you are worried about here, we could always revert if
> when you catch up you aren't happy with the commit.

It's not the case when a patch needs to be rolled back that I'm
concerned about. I think that it's useful to have all signoffs in the
history so that if there's ever a question in the future about a
change an the primary author is unavailable, others can be identified
to answer questions, defend the change, etc.

Does that make sense?
--msw