On Thu, Sep 17, 2026 at 11:42:50AM -0700, Kees Cook wrote:
> On Thu, Sep 17, 2026 at 05:06:19PM +0100, Lorenzo Stoakes (ARM) wrote:
> > This can be done faster in C, so implement scripts/basic/depcheck to do so.
> >
> > It does as little work as possible, reading the .cmd files from a
> > directory's targets and running stat on each dependency only a single time.
>
> Doesn't this run the risk of missing transitive deps?
>
> C.cmd: C.o depends on A.c and B.c and B.h
> B.cmd: B.h depends on B.data and B.script
>
> depcheck looks at C.o and B.h's times and is happy so it drops C.cmd file,
> but then see B.script has changed compared to B.h, so it keeps B.cmd,
> and then the build runs and doesn't have the C.o dep list any more,
> and C.o goes unbuilt?
No, B.h's .cmd file is still in the fragment with the full dependency list, so
make remakes B.h from B.script and C.o's saved command line is there too.
But, as you mention in the follow up :), it screws up on next bit - C.o
depending on B.h with B.h just remade. Will reply to your follow-up on that!
>
> Also, I don't think this handles if_changed commands at all? The kbuild
> rebuild condition is timestamps plus if_changed's command-line check.
It does, the fragment always has the saved command line, only the dependency
list is dropped, so if_changed compares the same as before.
>
> > diff --git a/scripts/basic/depcheck.c b/scripts/basic/depcheck.c
> > new file mode 100644
> > index 000000000000..622101c33650
> > --- /dev/null
> > +++ b/scripts/basic/depcheck.c
> > @@ -0,0 +1,441 @@
> > [...]
> > +/* What fixdep writes, see above. */
> > +#define DEPS_PREFIX "deps_"
> > +#define DEPS_RULE_PREFIX "$(" DEPS_PREFIX
> > +#define RULE_SUFFIX ":"
> > +#define LINE_CONTINUATION " \\"
> > +#define WILDCARD_OPEN "$(wildcard "
> > +#define WILDCARD_CLOSE ")"
> > +#define CMD_SUFFIX ".cmd"
> > [...]
> > +static bool is_blank(char chr)
> > [...]
> > +static bool str_ends_with(const char *str, const char *suffix)
> > [...]
> > +static bool line_starts_with(const struct line *line, const char *prefix)
> > [...]
> > +static bool line_ends_with(const struct line *line, const char *suffix)
> > [...]
> > +static bool line_is_blank(const struct line *line)
> > [...]
> > +static bool line_is_continued(const struct line *line)
> > [...]
> > +static void line_strip_continuation(struct line *line)
> > [...]
> > +static void line_trim(struct line *line)
> > [...]
> > +static bool next_line(const char **pos, const char *end, struct line *line)
>
> There is a lot of sting handling in here. I know you're going for speed,
> but it just feels like a python script doing all this would be much more
> readable without wrecking speed compared to C much. Yes, C will win out,
> but the maintainability of this helper does not fill me with joy. :P
Well I had the LLM look at that and for an allmodconfig python's startup time
would take 40s of CPU time _alone_ and it'd kill the no op improvement. So C it
has to be :)
>
> > [...]
> > +static const struct dep *lookup_dep(const char *path)
> > +{
> > + const unsigned int key = hash_str(path);
> > + struct dep *dep;
> > + struct stat st;
> > +
> > + hash_for_each_possible(dep_table, dep, hnode, key) {
> > + if (!strcmp(dep->path, path))
> > + return dep;
> > + }
> > +
> > + dep = xmalloc(sizeof(*dep) + strlen(path) + 1);
> > + strcpy(dep->path, path);
> > + dep->exists = !stat(path, &st);
> > + if (dep->exists)
> > + dep->mtime = st.st_mtim;
>
> I realize you only check "mtime" after an "exists" check, but I bristle
> at leaving a time-stamp uninitialized. Can we just add an "else
> dep->mtime = 0" here?
Ack, done for v4!
>
> -Kees
>
> --
> Kees Cook
--
Cheers, Lorenzo
