Hi Masahiro, Le 27/03/2017 à 07:26, Masahiro Yamada a écrit : > Hi Nocolas, > > > 2017-03-24 18:03 GMT+09:00 Nicolas Dichtel <nicolas.dichtel@xxxxxxxxx>: [snip] > > > As a whole, this series is amazing. Thanks for your great work! Thank you. And thank you for taking time to review it. > > > I added some comments, but they are trivial. > > > > > I wanted to leave comments/questions on 10/11, > but I could not find 10/11 in my mailbox. I do not know why. Note that you can download the mail from the kbuild patchwork, open it with your email client and do a reply ;-) > > > I am leaving comments on the cover-letter, > the following are related to 10/11. > > > > [1] > >> mandatory-y += $(foreach hdr,$(opt-header), \ >> $(if \ >> $(wildcard \ >> $(srctree)/arch/$(SRCARCH)/include/uapi/asm/$(hdr) \ >> $(srctree)/arch/$(SRCARCH)/include/asm/$(hdr) \ >> ), \ >> $(hdr) \ >> )) > > What is this actually checking? > > If ARCH has its own (uapi/)asm/{kvm.h,kvm_para.h,a.out.h}, > they are added to mandatory-y, then they are checked if they exist. > But, we know they exist. Yes, you're right. With english words : 'those files are mandatory only if they exist', thus they are not mandatory at all :) > > > This check reminds us only when we added asm/*.h > but forgot to add uapi/asm/*.h > > $(srctree)/arch/$(SRCARCH)/include/uapi/asm/$(hdr) seems unneeded at least. > (perhaps, the whole hunk might be unneeded.) I think we can remove the whole hunk (see also [2]). > > > > [2] > >> ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/uapi/asm/a.out.h \ >> $(srctree)/arch/$(SRCARCH)/include/asm/a.out.h),) >> header-n += a.out.h >> endif >> >> ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/uapi/asm/kvm.h \ >> $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h),) >> header-n += kvm.h >> endif >> >> ifeq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/uapi/asm/kvm_para.h \ >> $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h),) >> header-n += kvm_para.h >> endif > > This series intends all headers are exported from uapi/, correct? > Do we still need to check $(srctree)/arch/$(SRCARCH)/include/asm/*.h ? > (related to [1]) No you're right, uapi/asm/*.h is enough. Those files should be exported only if the uapi/asm/ counterpart exists. > > > > [3] > >> --- 7.1 header-n >> >> header-n is essentially used by include/uapi/linux/Kbuild to avoid >> exporting specific headers (e.g. kvm.h) on architectures that do not >> support it. It should be avoided as much as possible. > > > Going forward, header-y will be never used > because uapi/ is exported by default. > > So, I wonder if we could rename this into something clearer. > > Kbuild supports "no-clean-files". > (Please see ./Kbuild for its usage) > I guess this notation seems clearer > when we want to negate the default behavior. > > Can you consider "no-export", "no-export-files", "no-export-headers" > or whatever you like? No problem, let's use no-export-headers. Thank you, Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html