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

[Doug Goldstein <cardoe@cardoe.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3361 bytes --]

On 12/9/15 2:40 AM, Jan Beulich wrote:
>>>> On 08.12.15 at 20:53, <cardoe@cardoe.com> wrote:
>> On 11/30/15 8:45 AM, Jan Beulich wrote:
>>>>>> On 24.11.15 at 18:51, <cardoe@cardoe.com> 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

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

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