2018-07-10 20:34 GMT+09:00 Dirk Gouders <d...@gouders.net>:
> Masahiro Yamada <yamada.masah...@socionext.com> writes:
>
>> The main motivation of this patch series is to suppress the syncconfig
>> during running installation targets.
>>
>> V1 consisted of only two patches:
>> https://patchwork.kernel.org/patch/10468105/
>> https://patchwork.kernel.org/patch/10468103/
>>
>> I noticed that installation targets would continue running
>> even if the source tree is not configured at all
>> because the inclusion of include/config/auto.conf was optional.
>>
>> So, I added one more patch in V2:
>> https://patchwork.kernel.org/patch/10483637/
>>
>> However, kbuild test robot reported a new warning message was displayed:
>>
>> Makefile:592: include/config/auto.conf: No such file or directory
>>
>> This warning is displayed only for Make 4.1 or older.
>>
>> To fix this annoying warning, I changed Kconfig too,
>> which leaded to more clean-up, improvements in Kconfig.
>>
>> So, V3 is a big patch series.
>
> Hello Masahiro,
>
> I tested your series for a while, now. I did not notice real issues
> with it but want to leave some remarks about what I noticed in
> the surroundings of your patches.
>
>
>> Masahiro Yamada (12):
>> kconfig: rename file_write_dep and move it to confdata.c
>
> I might be missing some trivial use-case, but when looking at this
> patch, I noticed an inconsistency with the file names auto.conf and
> auto.conf.cmd.
>
> The first can be modified by an environment variable but when this
> happens, auto.conf.cmd remains as is. I noticed that only the
> Documentation mentions that KCONFIG_AUTOCONFIG exists and confdata.c
> uses it to serve the file name -- no other use anywhere.
Indeed.
I had also noticed this.
Probably, replacing the hardcoded 'include/config/auto.conf.cmd'
with get_env('KCONFIG_AUTOCONFIG') + 'cmd' will be nice.
I did not touch it since it thought it was less important
for this patch set.
If you are willing to contribute to this,
a patch is welcome (after this series).
> Now, I am wondering if I just don't see an important case when the
> use of KCONFIG_AUTOCONFIG is really helpful or even mandatory.
I do not know the historical background,
but I guess predecessors wanted to implement Kconfig
as generic/flexible as possible.
>
>> kconfig: split out helpers to check file/directory, create directory
>> kconfig: remove unneeded directory generation from local*config
>> kconfig: create directories needed for syncconfig by itself
>> kconfig: make syncconfig update .config regardless of sym_change_count
>
> For this patch, I already mentioned that `conf --help' perhaps could be
> updated.
What do you want 'conf --help' to look like ?
> On the other side, none of the entries there tells us such
> details, so there is probably no need for syncconfig to do so.
>
>> kconfig: allow all config targets to write auto.conf if missing
>> kbuild: use 'include' directive to load auto.conf from top Makefile
>> kbuild: add .DELETE_ON_ERROR special target
>> kbuild: do not update config when running install targets
>> kbuild: do not update config for 'make kernelrelease'
>> kbuild: remove auto.conf and tristate.conf from prerequisites
>
> In the surrounding of this patch I noticed -include of auto.conf and
> tristate.conf in scripts/Makfile.modbuildin. I tried it in some ways
> but was not able to trigger that file being used with a missing
> auto.conf.
Right. auto.conf and tristate.conf are mandatory there.
'-include' should be replaced with 'include'.
Cater to send a patch?
> On the other hand, if I now manually remove tristate.conf,
> that would not be fixed or even noticed, because of -include and I
> wonder if it is safer to also change the -includes in that file.
>
> It seems, if one of those files is missing, one must have done it
> manually or some other serious issue is present that we probably want to
> notice.
You are right. If somebody removes tristate.conf on purpose,
it is not self-healing.
I should be fixed by itself, or at least it should fail
with clear message.
I will reconsider this.
Thanks.
> Dirk
>
>> kbuild: replace include/config/%.conf with include/config/auto.conf
>>
>> Makefile | 46 +++++++++------
>> scripts/Kbuild.include | 3 +
>> scripts/kconfig/Makefile | 16 ++---
>> scripts/kconfig/conf.c | 39 +++++++------
>> scripts/kconfig/confdata.c | 139
>> +++++++++++++++++++++++++++++++++++++-------
>> scripts/kconfig/gconf.c | 1 +
>> scripts/kconfig/lkc.h | 1 -
>> scripts/kconfig/lkc_proto.h | 2 +-
>> scripts/kconfig/mconf.c | 1 +
>> scripts/kconfig/nconf.c | 1 +
>> scripts/kconfig/qconf.cc | 2 +
>> scripts/kconfig/util.c | 30 ----------
>> 12 files changed, 182 insertions(+), 99 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Masahiro Yamada
