Xen devel - May 2013 - [PATCH] Make sure to use tools as found by configure

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)




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

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.

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.

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

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

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.

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

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

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.

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.
> 
>

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