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@arm.com
Thanks for the review!
thanks,