Skip to content

Conversation

@jasonyb
Copy link

@jasonyb jasonyb commented May 13, 2025

A number of commits to run pgindent.

jaki and others added 2 commits May 13, 2025 00:41
Take the state of src/tools/pg_bsd_indent as of commit
be31ac2.  The next commit that touches
src/tools/pg_bsd_indent (3691edf)
breaks build, so this commit is chosen.

Cherry-picking each individual commit leading up to this stage is
nontrivial due to interaction with not-yet-supported Meson and the
touching of many files for trivial style updates, so this way is much
simpler.  Followups will take care of any other changes needed outside
of this directory.
YB: discard changes to
- src/meson.build since meson is unrelated at this point (use make
  instead)
- src/tools/pg_bsd_indent since it is already up-to-date as of later
  commit be31ac2.

Update the Makefile and build directions for in-tree build,
and add Meson build infrastructure.  Also convert the ad-hoc
test target into a TAP test.

Currently, the Make build system will not build pg_bsd_indent
by default, while the Meson system will.  Both will test it
during "make check-world" or "ninja test".  Neither will install
it automatically.  (We might change some of these decisions later.)

Also fix a few portability nits noted during early testing.

Also, exclude pg_bsd_indent from pgindent's purview; at least for
now, we'll leave it formatted similarly to the FreeBSD original.

Tom Lane and Andres Freund

Discussion: https://postgr.es/m/3935719.1675967430@sss.pgh.pa.us
Discussion: https://postgr.es/m/20200812223409.6di3y2qsnvynao7a@alap3.anarazel.de
(cherry picked from commit 156c049)
@jasonyb jasonyb requested a review from hari90 May 13, 2025 07:50
tglsfdc and others added 4 commits May 13, 2025 12:45
I missed updating this when we pulled pg_bsd_indent into the tree.

(cherry picked from commit 2e6ba13)
If a variable has an initialization expression that wraps onto the
next line(s), pg_bsd_indent will now indent the continuation lines
one stop, instead of aligning them flush with the variable declaration.

We've been holding off applying this until the last v16 CF finished,
but now it's time.

Thomas Munro and Tom Lane

Discussion: https://postgr.es/m/20230120013137.7ky7nl4e4zjorrfa@awork3.anarazel.de
(cherry picked from commit 064750a)
Do a test run of pgindent, manually find types that should be in
typedefs.list, exclude any that do not also belong in the typedefs.list
of recent master (commit 8ede692 at the
time of writing), then add those types to typedefs.list.

Note the following types that were intentionally not included in the
list:

- ResultRelInfoExtra: introduced by commit
  3706cc9, this type only exists in the
  back-patch branch and not master.
- EPQStateExtra: introduced by commit
  4729d1e, this type only exists in the
  back-patch branch and not master.
- LWLockWaitState: this type is not properly added to typedefs.list in
  master as well.
- pgoff_t: this is #define'd, and it appears that those typically do not
  show up in typedefs.list.  Closest example I found was LOCKMODE, which
  is #define'd, but it is also typedef'd, giving it a fair excuse to
  exist in typedefs.list.  pgoff_t does not exist in master's
  typedefs.list, and there is an oddly formatted line

      src/bin/pg_dump/pg_backup_archiver.c:2055:ReadOffset(ArchiveHandle *AH, pgoff_t * o)

  as a result.  So the best choice here appears to be to not add it to
  typedefs.list, and once pgindent is run again, it will produce another
  oddly formatted line due to commit
  6b6901a being different from master's
  version of it.
Back-patch branches generally do not run pgindent.  Back-patch branches
tend to come from commits in master.  Since master periodically runs
pgindent, by the time of the next major version PG merge, running
pgindent in this back-patch branch as well reduces merge conflicts.

Also, YB may periodically run pgindent as well, and in order to reduce
the difference between YB and PG, pgindent should be run on both sides.

See some previous commits for additional details.

Run pgindent like so:

    src/tools/pgindent/pgindent --indent=src/tools/pg_bsd_indent/pg_bsd_indent
Copy link

@timothy-e timothy-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 0d8210c (cherry pick of 064750a) doesn't include the changes to io.c or args.c. Can you explain that?

Otherwise the commits look good to me

@jasonyb
Copy link
Author

jasonyb commented May 21, 2025

Commit 0d8210c (cherry pick of 064750a) doesn't include the changes to io.c or args.c. Can you explain that?

Otherwise the commits look good to me

because 8c9f7c1 already has it

@jasonyb jasonyb merged commit 7c4b9bd into yugabyte:yb-pg15 May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants