From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter@eisentraut.org>
cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: remove pgrminclude?
In-reply-to: <2d4dc7b2-cb2e-49b1-b8ca-ba5f7024f05b@eisentraut.org>
References: <2d4dc7b2-cb2e-49b1-b8ca-ba5f7024f05b@eisentraut.org>
Comments: In-reply-to Peter Eisentraut <peter@eisentraut.org>
	message dated "Mon, 09 Dec 2024 08:14:54 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3926003.1733764849.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Mon, 09 Dec 2024 12:20:49 -0500
Message-ID: <3926004.1733764849@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/3926004.1733764849%40sss.pgh.pa.us>
Precedence: bulk

Peter Eisentraut <peter@eisentraut.org> writes:
> I propose to remove the pgrminclude scripts and annotations.  AFAICT, =

> per git log, the last time someone tried to do something with it was =

> around 2011.  Also, many (not all) of the "pgrminclude ignore" =

> annotations are of a newer date but seem to have just been copied around=
 =

> during refactorings and file moves and don't seem to reflect an actual =

> need anymore.

I agree with dropping pgrminclude --- as you say, it's not been used
since 2011 and there seems little appetite for ever using it again.
But I think there might be some lessons for us now in why it ended up
in such bad odor.  The relevant git history is at 1609797c2:

    Author: Tom Lane <tgl@sss.pgh.pa.us>
    Branch: master Release: REL9_2_BR [1609797c2] 2011-09-04 01:13:16 -040=
0

    Clean up the #include mess a little.
    =

    walsender.h should depend on xlog.h, not vice versa.  (Actually, the
    inclusion was circular until a couple hours ago, which was even sillie=
r;
    but Bruce broke it in the expedient rather than logically correct
    direction.)  Because of that poor decision, plus blind application of
    pgrminclude, we had a situation where half the system was depending on
    xlog.h to include such unrelated stuff as array.h and guc.h.  Clean up
    the header inclusion, and manually revert a lot of what pgrminclude ha=
d
    done so things build again.
    =

    This episode reinforces my feeling that pgrminclude should not be run
    without adult supervision.  Inclusion changes in header files in parti=
cular
    need to be reviewed with great care.  More generally, it'd be good if =
we
    had a clearer notion of module layering to dictate which headers can s=
anely
    include which others ... but that's a big task for another day.

In short, pgrminclude turned a small human error into a giant mess
that required half a day's work to clean up, because it had no idea
which of some redundant-looking includes were reasonable to get
rid of and which weren't.

I am worried that IWYU might be prone to similar mess-amplification.
Perhaps it has better heuristics as a result of doing more thorough
semantic analysis, but heuristics are still only heuristics.

One thing that I would look favorably on, given the mistakes we made
in 2011, is automatic detection of circular #include's.  Do you happen
to know whether IWYU complains about that?

			regards, tom lane