git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, John Cai <johncai86@gmail.com>
Subject: Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids
Date: Thu, 27 Oct 2022 21:23:03 -0400	[thread overview]
Message-ID: <Y1su99Kc0ex1W7bX@coredump.intra.peff.net> (raw)
In-Reply-To: <221026.8635badbz5.gmgdl@evledraar.gmail.com>

On Wed, Oct 26, 2022 at 01:35:43PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > (I had a similar thought after getting hit on the head by the recent
> > t0450-txt-doc-vs-help.sh).
> 
> Sorry about that. FWIW I've wanted to assert that for a while, and to do
> it by e.g. having the doc *.txt blurbs generated from running "$buildin
> -h" during the build.
> 
> But when I suggested that I gathered That Junio wasn't up for that
> approach, it does have its downsides thorugh. E.g. to build the docs
> you'd now have to compile C code, and e.g. that git-scm.com publisher
> written in Ruby would have to compile the code to generate its docs.

Yes, it would definitely break the git-scm.com importer. It might be
possible to make it work by actually running "make man" (or maybe even
some partial targets) in a local repo. The nightly update job pulls all
of the data using GitHub's API, but it does run in a heroku dyno that
has git and gcc. Doing a shallow clone of the tag to build is probably
more expensive, but the cost of an actual build isn't that important.
99% of the scheduled job runs are noops because there's no new version
of Git to build manpages for; as long as those remain cheap-ish, we're
OK.

I also think in the long run that the site should move to a system that
builds off a local repo, which we can trigger manually or via GitHub
Actions. But that doesn't exist yet, and I don't think anyone's actively
working on it.

I also think it would be nice if the git-scm.com system relied more on
"make man USE_ASCIIDOCTOR=1", possibly by hooking into
asciidoctor-extensions.rb or similar, rather than munging files in an
ad-hoc manner. But that's also a big change that nobody is actively
working on.

All of which is to say that yes, having docs depend on C code will cause
work on the site, but it may be work that takes us in the right
direction.

> Or we could do it the other way around, and scape the *.txt for the *.c
> generation, but then we need to run a new script for building
> builtin/*.o. Also possible, and I think eventually both are better than
> what t0450-txt-doc-vs-help.sh's doing now, but that was a less invasive
> change than both...

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.

> >> +# Unfortunately our dependency management of generated headers used
> >> +# from within other headers suck, so we'll need to list every user of
> >> +# fsck.h here, but not too bad...
> >> +FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects
> >> +$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h
> >> +FSCK_MSGIDS_H_LIBS = fetch-pack fsck
> >> +$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h
> >
> > I don't understand the "used within other headers" part here. Computed
> > dependencies will get this right. It's only the initial build (before we
> > have any computed dependencies) that needs to know that the C files in
> > question depend on the generated file. But that is true whether they do
> > so directly or indirectly.
> 
> I forgot the full story, but luckily I wrote it down in the WIP commits
> :) FWIW if you want to scour that it's mainly:
> 
> 	https://github.com/avar/git/commit/a00f1cb9ea5 # add advice-type.h
> 	https://github.com/avar/git/commit/9e080923a11 # generate 'struct advice

That seems like the same problem to me. It's just that if something is
in cache.h, then it's needed by basically everything, and so the
dependency list is big.

> Anyway, in partial summary, why (and sorry this got so long):
> 
>  * Yes, once we build e.g. fsck.o we have the full dependency tree,
>    yay....
>  * ...but only for gcc & clang, but we support other compilers.
>  * ...for other compilers (or gcc & clang without the dep detection
>    enabled) what should this do:
> 
> 	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.
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).

That all comes from b8ba629264 (Makefile: fold MISC_H into LIB_H,
2012-06-20).

>  * We also have e.g. "make hdr-check", which works just fine now, but
>    it's becausue we have no *.h file that uses another generated *.h
>    file.

I'm not that surprised. Probably it should depend on $(GENERATED_H), and
possibly even be checking those files too.

>  * I may be wrong (I'm just looking at some old "here be dragons" note
>    of mine), but I think there's also an edge case where the dependency
>    graph of .depend.d/* is correct *once you've always had it*, but if a
>    file now e.g. depends on foo.h, and I make foo.h include a new
>    foo-generated.h things go boom.

That should work because the timestamp on foo.h will have been updated,
causing anything that includes it to rebuild (and thus updating its
computed dependencies to include foo-generated.h).

-Peff

  reply	other threads:[~2022-10-28  1:23 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-24 15:00 [PATCH 0/2] Document fsck msg ids John Cai via GitGitGadget
2022-10-24 15:00 ` [PATCH 1/2] fsck: remove the unused BAD_TAG_OBJECT John Cai via GitGitGadget
2022-10-24 16:57   ` Junio C Hamano
2022-10-24 18:16     ` Junio C Hamano
2022-10-24 18:33       ` John Cai
2022-10-24 23:39         ` Jeff King
2022-10-24 15:00 ` [PATCH 2/2] fsck: document msg-id John Cai via GitGitGadget
2022-10-24 17:33   ` Junio C Hamano
2022-10-25  9:41     ` Ævar Arnfjörð Bjarmason
2022-10-25 16:07       ` Junio C Hamano
2022-10-24 18:51 ` [PATCH 0/2] Document fsck msg ids Junio C Hamano
2022-10-25  3:17 ` [PATCH v2 " John Cai via GitGitGadget
2022-10-25  3:17   ` [PATCH v2 1/2] fsck: remove the unused BAD_TAG_OBJECT John Cai via GitGitGadget
2022-10-25  3:17   ` [PATCH v2 2/2] fsck: document msg-id John Cai via GitGitGadget
2022-10-25  4:30   ` [PATCH v2 0/2] Document fsck msg ids Junio C Hamano
2022-10-25  4:40     ` Junio C Hamano
2022-10-25  5:12     ` [PATCH] Documentation: add lint-fsck-msgids Junio C Hamano
2022-10-25 22:42   ` [PATCH v3 0/4] document fsck error message ids Junio C Hamano
2022-10-25 22:42     ` [PATCH v3 1/4] fsck: remove the unused BAD_TAG_OBJECT Junio C Hamano
2022-10-25 22:42     ` [PATCH v3 2/4] fsck: remove the unused MISSING_TREE_OBJECT Junio C Hamano
2022-10-25 22:42     ` [PATCH v3 3/4] fsck: document msg-id Junio C Hamano
2022-10-25 22:42     ` [PATCH v3 4/4] Documentation: add lint-fsck-msgids Junio C Hamano
2022-10-26  2:43       ` Ævar Arnfjörð Bjarmason
2022-10-26  5:34         ` Jeff King
2022-10-26  6:46           ` Junio C Hamano
2022-10-26 11:35           ` Ævar Arnfjörð Bjarmason
2022-10-28  1:23             ` Jeff King [this message]
2022-10-28  2:04               ` Ævar Arnfjörð Bjarmason
2022-10-28  5:32                 ` Jeff King
2022-10-28 10:41                   ` Ævar Arnfjörð Bjarmason
2022-10-28  3:02             ` John Cai
2022-10-28  3:11               ` Ævar Arnfjörð Bjarmason
2022-10-28  5:32                 ` Junio C Hamano
2022-10-28  5:37                   ` Jeff King
2022-10-28  5:35                 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y1su99Kc0ex1W7bX@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johncai86@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).