Hi Paul and Stefan, Thanks for taking a look at this. I think Stefan has touched upon why he thinks this change might (at least partially) make sense, but let me now try to explain the rationale behind this patch better than I did in the commit message. On Sun, Apr 12, 2015 at 8:02 AM, Stefan Hengelein <stefan.hengelein@xxxxxx> wrote: >> Let's focus, for example, on m32r and FRAME_POINTER. The m32r entry for >> that symbol reads: >> config FRAME_POINTER >> bool "Compile the kernel with frame pointers" >> help >> If you say Y here [...] >> >> 0) If one is building for m32r is that all there's to it? If so, "make >> menuconfig"'s search facility is serving the people building for m32r a >> load of crap. >> >> 1) If it's actually more complicated than that I think that anyone >> reading arch/m32r/Kconfig.debug is being duped. Things look simple but >> actually they are quite complicated. I think that's just wrong. >> >> What am I missing here? > > If you have a look at the definitions, lib/Kconfig.debug is included > before FRAME_POINTER is defined in m32r and the output in the search > facility looks indeed broken > as one "Defined at" is missing but there are somehow Location entries > (-> Kernel hacking and -> Kernel hacking -> compile time checks > and [...]) for both definitions in a weird order (i think (1) and (2) > might indicate both definitions) > > both declarations are valid in kconfig, you have two ways of enabling > the same symbol, one easy without conditions and one with conditions > and both with the same prompt. > > The search facility shows the first one that is found, you see the > complicated depends on but i think the text shown might not be > explicit enough to clarify you don't need to satisfy these complicated > conditions to actually choose a value. > Well, the thing is, you do need to satisfy _one_ of the set of conditions. But unfortunately it's not not possible to see that from the search function because it prints out incomplete information. This is exactly the motivation for this change. When you search for FRAME_POINTER, you only get information on one "Defined at", and the only "Depends" information is from that definition. This is clearly incorrect, as FRAME_POINTER is defined in multiple places, so the information printed does not cover its actual definitions or dependencies. The printed "Selected by" information is, however, correct, because it actually uses the rev_dep expression. This patch changes the "Depends" information to be similar: it will also print the actual expression used by kconfig used to determine whether its direct dependencies are fulfilled. Is this overly complicated or confusing? Perhaps. But it is better than printing out an incomplete set of definitions and dependencies, which is the current behavior. As Stefan mentioned, "FRAME_POINTER is a complicated example, it is selected although it has dependencies or a prompt AND it is redefined in many architectures". I'm pretty new to the Kconfig code, but to me, the multiple symbol behavior is bizarre. This is because: - every definition is independent---dependencies are OR'd together from each definition. - each definition with a prompt creates a new location where that symbol can be changed in menuconfig - each definition _without_ a prompt that has its dependencies fulfilled results in the default value set in that definition being used unconditionally. E.g. for FRAME_POINTER, this means that it is impossible to disable the option on ARM. I have submitted a separate patch to try to fix that for ARM at least, https://lkml.org/lkml/2015/4/8/765 I definitely think more thought needs to be put into cleaning up the multiple definition behavior, as it is very confusing to follow. But the current behavior is flat-out wrong, as the search function does not actually print out the actual set of definitions or dependencies used by kconfig, and that is all that this patch is aiming to fix. Aside: if the issue is in how this is displayed, maybe it would be better to add in some parentheses and/or linebreaks to make it clearer that these are distinct sets, so rather than printing Defined at lib/Kconfig.debug:323, arch/arm/Kconfig.debug:35 Depends on: DEBUG_KERNEL [=y] && (CRIS || M68K || FRV || UML || AVR32 || SUPERH || BLACKFIN || MN10300 || METAG) || ARCH_WANT_FRAME_POINTERS [=n] || !THUMB2_KERNEL [=n] you get something more like Defined at lib/Kconfig.debug:323, arch/arm/Kconfig.debug:35 Depends on: (DEBUG_KERNEL [=y] && (CRIS || M68K || FRV || UML || AVR32 || SUPERH || BLACKFIN || MN10300 || METAG) || ARCH_WANT_FRAME_POINTERS [=n]) || (!THUMB2_KERNEL [=n]) which makes the logic a little bit more obvious. (Also, sorry, the ARM=y in the original before/after was from a separate change I was testing. But it doesn't affect what we get here because THUMB_KERNEL=n anyway). You could do this by iterating over the associated properties rather than just printing out the dir_dep expression of the symbol, but then you'd probably to change rev_dep to do the same. I don't really like this, because it doesn't match up with what kconfig is _actually_ using (i.e. the rev_dep and dir_dep of the associated symbol). Best regards, Gregory -- 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
