Hi,
On 2023-10-18 19:18:13 -0700, Andres Freund wrote:
> On 2023-10-18 21:29:37 -0400, Tom Lane wrote:
> Ah, I see. If I interpret that correctly, the code filters out symbols it
> doesn't find in in some .[chly] file in the *source* directory. This code is,
> uh, barely readable and massively underdocumented.
>
> I guess I need to reimplement that :/. Don't immediately see how this could
> be implemented for in-tree autoconf builds...
>
> > > But in the attached patch I've implemented this slightly differently. If the
> > > tooling to do so is available, the indent-* targets explained above,
> > > use/depend on src/tools/pgindent/typedefs.list.merged (in the build dir),
> > > which is the combination of a src/tools/pgindent/typedefs.list.local generated
> > > for the local binaries/libraries and the source tree
> > > src/tools/pgindent/typedefs.list.
> >
> > Hmm ... that allows indenting your C files, but how do you get from that
> > to a non-noisy patch to commit to typedefs.list?
>
> It doesn't provide that on its own. Being able to painlessly indent the files
> seems pretty worthwhile already. But clearly it'd much better if we can
> automatically update typedefs.list.
With code for that added, things seem to work quite nicely. I added similar
logic to the buildfarm code that builds a list of all tokens in the source
code.
With that, the in-tree typedefs.list can be updated with new tokens found
locally *and* typdefs that aren't used anymore can be removed from the in-tree
typedefs.list (detected by no matching tokens found in the source code).
The only case this approach can't handle is newly referenced typedefs in code
that isn't built locally - which I think isn't particularly common and seems
somewhat fundamental. In those cases typedefs.list still can be updated
manually (and the sorting will still be "fixed" if necessary).
The code is still in a somewhat rough shape and I'll not finish polishing it
today. I've attached the code anyway, don't be too rough :).
The changes from running "ninja update-typedefs indent-tree" on debian and
macos are attached as 0004 - the set of changes looks quite correct to me.
The buildfarm code filtered out a few typedefs manually:
push(@badsyms, 'date', 'interval', 'timestamp', 'ANY');
but I don't really see why? Possibly that was needed with an older
pg_bsd_indent to prevent odd stuff?
Right now building a new unified typedefs.list and copying it to the source
tree are still separate targets, but that probably makes less sense now? Or
perhaps it should be copied to the source tree when reindenting?
I've only handled linux and macos in the typedefs gathering code. But the
remaining OSs should be "just a bit of work" [TM].
Greetings,
Andres Freund