Re: [PATCH 4/9] build: include Tools.mk first in tools/Rules.mk

[Roger Pau Monne <roger.pau@citrix.com>]

Ian Campbell wrote:
> On Wed, 2012-07-25 at 10:32 +0100, Roger Pau Monne wrote:
>> Ian Campbell wrote:
>>> On Wed, 2012-07-11 at 11:23 +0100, Roger Pau Monne wrote:
>>>> Tools.mk should be included first, or PREFIX is not honoured in the
>>>> other conf/ files that define the paths of several tools.
>>> Isn't it a bug for anything which is included/defined via
>>> $(XEN_ROOT)/Config.mk to depend on the contents of
>>> $(XEN_ROOT)/config/Tools.mk since for anything non-tools Tools.mk would
>>> never be included at all?
>> $(XEN_ROOT)/Config.mk includes config/{Linux.mk/NetBSD.mk} and
>> config/StdGNU.mk, which contain a bunch of paths, specially
>> config/StdGNU.mk. If the tools config file is not loaded before, all
>> this paths get set to the default value (which is probably fine in
>> Linux), but not desirable in NetBSD if the user has specified a custom
>> prefix.
>>
>> So the main problem is that the tools Makefile should include
>> config/Tools.mk before config/StdGNU.mk, but since config/StdGNU.mk is
>> included in $(XEN_ROOT)/Config.mk we have to either include Tools.mk
>> before $(XEN_ROOT)/Config.mk or modify $(XEN_ROOT)/Config.mk to include
>> Tools.mk (which is not desirable at all). I don't care that
>> $(XEN_ROOT)/Config.mk uses the default paths when building the xen
>> kernel, but we should honour the user set paths when installing the tools.
> 
> My point was that really all of these paths should move to autoconf and
> Tools.mk.in. But for 4.2 I guess your proposed change is more
> appropriate.

Yes, this is just a bandaid to hide a much bigger problem we have with
paths & makefiles in general.

>> One effect of this patch is that etc and var is installed inside of
>> $(PREFIX)/etc, which is normal in BSD systems, but I'm quite sure it is
>> not correct on Linux, so I should add the following chunk:
> 
> StdGNU explicitly has:
>         ifeq ($(PREFIX),/usr)
>         CONFIG_DIR = /etc
>         XEN_LOCK_DIR = /var/lock
>         XEN_RUN_DIR = /var/run/xen
>         XEN_PAGING_DIR = /var/lib/xen/xenpaging
>         else
>         CONFIG_DIR = $(PREFIX)/etc
>         XEN_LOCK_DIR = $(PREFIX)/var/lock
>         XEN_RUN_DIR = $(PREFIX)/var/run/xen
>         XEN_PAGING_DIR = $(PREFIX)/var/lib/xen/xenpaging
>         endif
> so it looks like this was at least deliberate.

Yes, this was the behaviour we had before the autoconf patch, when
PREFIX was specified in .config (which I find strange in a Linux
system). So I'm not going to add this chunk and leave the patch as-is.

Thanks.

> 
>> --- a/config/Linux.mk
>> +++ b/config/Linux.mk
>> @@ -6,3 +6,8 @@ KERNELS ?=
>>  XKERNELS := $(foreach kernel, $(KERNELS), \
>>                $(patsubst buildconfigs/mk.%,%, \
>>                  $(wildcard buildconfigs/mk.$(kernel))) )
>> +
>> +CONFIG_DIR = /etc
>> +XEN_LOCK_DIR = /var/lock
>> +XEN_RUN_DIR = /var/run/xen
>> +XEN_PAGING_DIR = /var/lib/xen/xenpaging
>>
>>>> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
>>>> ---
>>>>  tools/Rules.mk |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>>>> index 1e928b7..2dd8ed3 100644
>>>> --- a/tools/Rules.mk
>>>> +++ b/tools/Rules.mk
>>>> @@ -3,8 +3,8 @@
>>>>  # `all' is the default target
>>>>  all:
>>>>  
>>>> -include $(XEN_ROOT)/Config.mk
>>>>  -include $(XEN_ROOT)/config/Tools.mk
>>>> +include $(XEN_ROOT)/Config.mk
>>>>  
>>>>  export _INSTALL := $(INSTALL)
>>>>  INSTALL = $(XEN_ROOT)/tools/cross-install
>>>
> 
>