Horton

Thread

Commits

  1. Remove pgrminclude and associated scripts

  2. Remove pgrminclude annotations

  1. remove pgrminclude?

    Peter Eisentraut <peter@eisentraut.org> — 2024-12-09T07:14:54Z

    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 think the include-what-you-use tool that I've been using lately is a 
better tool and can adequately replace pgrminclude.

I'm sending separately a patch to add some IWYU annotations, but these 
don't seem to correspond very strongly to pgrminclude annotations, so I 
don't see any value in keeping the old ones even for a transition.

Here are the scripts to remove:
1. pgcheckdefines -- This could still be useful???
2. pgcominclude -- redundant with headerscheck
3. pgdefine -- used internally only
4. pgfixinclude -- Could still be useful principle?
5. pgrminclude -- obsolete

  2. Re: remove pgrminclude?

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-12-09T17:20:49Z

    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 -0400

    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 sillier;
    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 had
    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 particular
    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 sanely
    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

  3. Re: remove pgrminclude?

    Peter Eisentraut <peter@eisentraut.org> — 2024-12-16T11:20:39Z

    On 09.12.24 18:20, Tom Lane wrote:
>      walsender.h should depend on xlog.h, not vice versa.  (Actually, the
>      inclusion was circular until a couple hours ago, which was even sillier;
>      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 had
>      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 particular
>      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 sanely
>      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?

IWYU is built on compiler infrastructure and tracks where things are 
declared and where they are used and then suggests you to include 
exactly the header files where the things you use are declared (rather 
than some other header file that happens to pull in the one you actually 
need) and suggests to remove the includes that don't provide any 
declarations that you use.  So it is not really a heuristic, it is 
perfectly accurate, barring bugs or complicated edge cases (cough, 
cough, CppAsString2()), assuming one agrees with that goal.

If you have two headers that circularly include each other, and you have 
the normal multiple-inclusion-guards, then one of the includes won't 
contribute anything to the overall set of declared things, so it would 
most likely be suggested for removal.  That's not exactly the same thing 
as actual circular include detection, but it will indirectly tell you 
about it.

  4. Re: remove pgrminclude?

    Peter Eisentraut <peter@eisentraut.org> — 2024-12-24T13:09:35Z

    On 09.12.24 18:20, Tom Lane wrote:
> 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.

committed