Re: [Xen-devel] [PATCHv6] 03/28] build: use generated Kconfig options for Xen

[Doug Goldstein <cardoe@xxxxxxxxxx>]

On 12/9/15 2:40 AM, Jan Beulich wrote:
>>>> On 08.12.15 at 20:53, <cardoe@xxxxxxxxxx> wrote:
>> On 11/30/15 8:45 AM, Jan Beulich wrote:
>>>>>> On 24.11.15 at 18:51, <cardoe@xxxxxxxxxx> wrote:
>>>> @@ -227,9 +230,14 @@ kconfig := silentoldconfig oldconfig config 
>>>> menuconfig defconfig \
>>>>  $(kconfig):
>>>>    $(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile ARCH=$(XEN_TARGET_ARCH) 
>>>> $@
>>>>  
>>>> -$(BASEDIR)/include/config/%.conf: $(BASEDIR)/include/config/auto.conf.cmd
>>>> +$(BASEDIR)/include/config/%.conf: $(BASEDIR)/include/config/auto.conf.cmd 
>>>> $(BASEDIR)/.config
>>>>    $(Q)$(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile 
>>>> ARCH=$(XEN_TARGET_ARCH) silentoldconfig
>>>>  
>>>>  # Allow people to just run `make` as before and not force them to 
>>>> configure
>>>> -$(BASEDIR)/.config $(BASEDIR)/include/config/auto.conf.cmd: ;
>>>> +$(BASEDIR)/.config:
>>>>    $(Q)$(MAKE) -f $(BASEDIR)/scripts/kconfig/Makefile 
>>>> ARCH=$(XEN_TARGET_ARCH) defconfig
>>>
>>> This should be one of the oldconfig targets now, shouldn't it?
>>
>> oldconfig uses .config. This is the case when the user has checked out
>> the tree fresh. Its there to not change the workflow of "git clone ...
>> && cd xen/xen && make"
> 
> Okay. But then this is meant to only support the simplest possible
> form, i.e. without honoring any make command line or ./.config
> overrides like debug=y or kexec=n? If that understanding of
> mine is correct, at least the comment should mention such a
> limitation (even better would be if in such cases unexpected
> results would be prevented by - as the most trivial solution -
> failing the build with a meaningful error message).

So you would like me to add code into xen/Makefile to detect if someone
ran "make kexec=X" or added "kexec=X" into top level .config and give
them a meaningful error message to tell them to use "make config" instead?

> 
>>>> +# Break the dependency chain for the first run
>>>> +$(BASEDIR)/include/config/auto.conf.cmd: ;
>>>> +
>>>> +-include $(BASEDIR)/include/config/auto.conf.cmd
>>>
>>> The comment is quite a bit different in Linux, and seems to make more
>>> sense. Also note how Linux has an empty rule for $(KCONFIG_CONFIG),
>>> a variable which iirc you defined in an earlier patch and hence perhaps
>>> you should be using here.
>>
>> I don't see where that's defined.
> 
> "That" may have multiple meanings in this context. The empty
> rule is in ./Makefile:

"That" was KCONFIG_CONFIG. Which was not anywhere in this patchset and
was always .config.

> 
> # To avoid any implicit rule to kick in, define an empty command
> $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
> 
> And if I was wrong with seeing you define KCONFIG_CONFIG
> somewhere, it would kind of imply that you probably should be
> inheriting this level of abstraction from Linux.
> 
> Jan
> 

Ok. I'll add a variable called KCONFIG_CONFIG. It'll default to .config.
Arguably someone can override "make menuconfig
KCONFIG_CONFIG=/some/place/I/stored/my/config" in Linux which they call
"Load Alternate Config". Googling searching doesn't show this is a
commonly used option/behavior since people usually copy their .config in.

The other places this variable matters are in the building of the
dependencies in kbuild but Xen doesn't use kbuild.

-- 
Doug Goldstein

Attachment: signature.asc
Description: OpenPGP digital signature

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