On 07/05/2024 17:47, John Hubbard wrote: > On 5/7/24 9:34 AM, Ryan Roberts wrote: >> On 07/05/2024 17:19, John Hubbard wrote: >>> On 5/7/24 12:45 AM, Ryan Roberts wrote: >>>> On 04/05/2024 05:43, John Hubbard wrote: >>> ... >>>> Hi John, >>>> >>>> I sent out a similar fix a couple of weeks ago, see [1]. I don't think it got >>>> picked up though. It takes a slightly different approach, explicitly adding >>>> -static-libsan (note no 'a') for clang, instead of relying on its default. >>>> >>>> And it just drops helpers.h from the makefile altogether, on the assumption >>>> that >>>> it was a mistake; its just a header and shouldn't be compiled directly. I'm not >>>> exactly sure what the benefit of adding it to LOCAL_HDRS is? >>> >>> Ah no, you must not drop headers.h. That's a mistake itself, because >>> LOCAL_HDRS adds a Make dependency; that's its purpose. If you touch >>> helpers.h it should cause a rebuild, which won't happen if you remove it >>> from LOCAL_HDRS. >> >> Ahh. I was under the impression that the compiler was configured to output the >> list of dependencies for make to track (something like -M, from memory ?). Since >> helpers.h is included from helpers.c I assumed it would be tracked like this - I >> guess its not that simple? > > This can be done, but it is not automatic with GNU Make. You have to explicitly > run gcc -M, capture the output in a dependencies list, and track it. Which the > Kbuild system does, but kselftest does not. Understood - thanks for the lesson! > > After just now sweeping through kselftest to fix up the clang build, I see a > lot of mistaken or partial use of the kselftest build's Make variables, because > people naturally reason based on what they know about Kbuild, and it doesn't > always translate. And LOCAL_HDRS might need some more documentation too. > I'll keep thinking about how to clarify this, I have a couple early ideas. > >> >> Anyway, on the basis that LOCAL_HDRS is the right way to do this, let's go with >> your version and drop mine: >> >> Reviewed-by: Ryan Roberts <ryan.roberts@xxxxxxx> >> > > Thanks for the review! > > thanks,
