On Fri, Oct 28, 2022 at 04:04:12AM +0200, Ævar Arnfjörð Bjarmason wrote:
> > I think this particular case is tricky in that direction, because it's a
> > big set of dependencies that aren't necessarily one-to-one. E.g.,
> > builtin/log.c needs to depend on git-log.txt, but also on git-show.txt
> > and git-format-patch.txt.
>
> I was thinking of just a generated usage-strings.c or whatever. I.e. one
> file with every usage string in it. Then you don't need to hunt down
> which thing goes in which file. We'll just have a variable named
> "builtin_format_patch_usage". Include it in "builtin.h" and it doesn't
> matter if that's in builtin/log.c or builtin/format-patch.c.
Yes, though you have the opposite problem, then: what are the source
files that can produce that usage-strings.c? If your answer is
"Documentation/git-*.txt", then that is a recipe for flaky dependencies.
We already have one such for config-list.h. Try this:
# introduce a new file
printf 'foo.bar::\n\ta fake config var\n' \
>Documentation/config/foo.txt
# it shows up in the output, as expected
make
./git help --config-for-completion | grep foo
# now drop it
rm Documentation/config/foo.txt
# oops; make won't rebuild anything, and it's still there
make
./git help --config-for-completion | grep foo
The bug is that config-list.h depends on a glob. So we notice when a
file changes, but not when one goes away. And this isn't just
hypothetical. Files come and go as you "git checkout" around history (or
bisect). I don't remember the details, but I'm pretty sure I've gotten
false positive test failures out of this before (maybe a topic branch
with a bogus entry that broke t0012 or t9902, and then moving back to
master doesn't fix it?).
So I'd prefer to avoid introducing more flakiness if we can. You might
be able to piggy back on command-list.txt in this case (that's what
makes command-list.h not flaky).
> It does mean you need to rebuild the C code for other built-ins every
> time one of the SYNOPSIS sections changes, but doesn't happen too often.
If it's a c/h combo that only makes the variable names public (not the
contents of the strings they point to), then it would only trigger a
rebuild when a command is added or removed.
> I think I either did or was planning to take it out of cache.h as part
> of that, we put way too much in cache.h.
>
> Even advice.c depends on cache.h for its advice.h *sigh*.
>
> Trying it just now putting advice.h in builtin.h instead leaves 10
> top-level files not-compiling (and they just need the header).
>
> I think it's fine to include common utilties in our "included by
> everything" headers, but if we've got ~500 *.c files and something is
> only needed by ~20 of them (as in this case) we should probably just
> include it in those 20.
Oh, definitely, we should be shrinking cache.h, and not adding more to
it. Especially not generated stuff.
> >> make file-that-does-not-use-generated-header.o
> >>
> >> It sucks a bit to have e.g. every *.o depend on command-list.h, when
> >> we only use it in 1-2 places, ditto the other headers.
> > But that is already true of non-generated headers. If your system
> > doesn't support computed deps, then all objects depend on all headers.
>
> ... this does not build e.g. command-list.h:
>
> make clean
> make grep.o
>
> But this does:
>
> make clean
> make help.o
>
> Because we've manually declared that.
Right, but...does grep.o actually need command-list.h? If it doesn't
(and that seems to be the case), then all is working as intended. If
grep included some other header that included command-list.h, then yeah,
that would be a bug. But that is true whether grep.c includes it
directly or not. Any Makefile dependency needs to take into account
recursive includes.
> > Yes, that sucks for you. But almost nobody actively develops on such a
> > system, and people building Git to use rarely notice (because they are
> > doing an initial build, or jumping versions, both of which generally
> > require recompilation anyway).
>
> I guess I'm "almost nobody" :) Not because I don't use computed deps,
> but because I tend to e.g. do large interactive rebases with:
>
> git rebase -i 'make grep.o'
>
> Or some other subset of our built assets/objects.
I do that all the time, too. But with computed deps, it works (by which
I mean it rebuilds only stuff needed by grep.o).
I'm not even sure what we're talking about anymore. If you are saying
that no, we don't want to just say "everything depends on foo.h that is
generated", I agree. That is wrong to do, and we should specify the
minimal dependencies where appropriate (and take care to keep that set
as small as is feasible using small C interfaces).
If you're saying "for people without computed dependencies, everything
will want to rebuild shell scripts", then I don't care. We decided long
ago that maintaining a manual list of header dependencies was not worth
doing, and people with sub-par compilers will have to suffer.
In the email you're replying to, I was trying to express the second one.
But it sounds like you thought I was trying to argue against the first
one.
-Peff
