From 05d8dec690e9719ff9a1830f5492864104275b5e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 15 Aug 2016 11:32:09 -0400 Subject: [PATCH] Simplify the process of perltidy'ing our Perl files. Wrap the perltidy invocation into a shell script to reduce the risk of copy-and-paste errors. Include removal of *.bak files in the script, so they don't accidentally get committed. Improve the directions in the README file. --- src/tools/pgindent/README | 148 ++++++++++++++--------- src/tools/pgindent/exclude_file_patterns | 2 +- src/tools/pgindent/pgperltidy | 23 ++++ 3 files changed, 114 insertions(+), 59 deletions(-) create mode 100755 src/tools/pgindent/pgperltidy diff --git a/src/tools/pgindent/README b/src/tools/pgindent/README index f9edbc1cf7..8e91697bd1 100644 --- a/src/tools/pgindent/README +++ b/src/tools/pgindent/README @@ -1,64 +1,104 @@ -pgindent -======== +pgindent'ing the PostgreSQL source tree +======================================= -This can format all PostgreSQL *.c and *.h files, but excludes *.y, and -*.l files. +We run this process at least once in each development cycle, +to maintain uniform layout style in our C and Perl code. -1) Install pg_bsd_indent (see below for details). +You might find this blog post interesting: +http://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html + + +PREREQUISITES: + +1) Install pg_bsd_indent in your PATH (see below for details). 2) Install entab (src/tools/entab/). -3) Change directory to the top of the build tree. +3) Install perltidy. Please be sure it is v20090616 (older and newer + versions make different formatting choices, and we want consistency). -4) Remove all derived files (pgindent has trouble with one of the flex macros): +DOING THE INDENT RUN: - make maintainer-clean +1) Change directory to the top of the source tree. - Or: +2) Remove all derived files (pgindent has trouble with flex files, and it + would be pointless to run it on them anyway): + make maintainer-clean + Or: git clean -fdx -5) Download the typedef file from the buildfarm: +3) Download the latest typedef file from the buildfarm: wget -O src/tools/pgindent/typedefs.list http://buildfarm.postgresql.org/cgi-bin/typedefs.pl - (see http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list for a full list of typedefs, - also http://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html) + (See http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list for a full + list of typedef files, if you want to indent some back branch.) -6) Run pgindent: +4) Run pgindent on the C files: src/tools/pgindent/pgindent -7) Remove any files that generate errors and restore their original - versions. + If any files generate errors, restore their original versions with + "git checkout", and see below for cleanup ideas. + +5) Indent the Perl code using perltidy: + + src/tools/pgindent/pgperltidy -8) Indent the Perl code using perltidy v20090616 (perltidy changes formatting - decisions, so older and newer versions are incompatible): + If you want to use some perltidy version that's not in your PATH, + first set the PERLTIDY environment variable to point to it. - ( - find . -name \*.pl -o -name \*.pm +VALIDATION: - find . -type f -exec file {} \; | - egrep -i ':.*perl[0-9]*\>' | - cut -d: -f1 - ) | - sort -u | - xargs perltidy --profile=src/tools/pgindent/perltidyrc +1) Check for any newly-created files using "git status"; there shouldn't + be any. (perltidy tends to leave *.LOG files behind if it has trouble.) -9) Do a full test build: +2) Do a full test build: - > run configure - # stop is only necessary if it's going to install in a location with an - # already running server - pg_ctl stop - run configure - make -C src install - make -C contrib install - run initdb - pg_ctl start - make installcheck-world + ./configure ... + make -s all # look for unexpected warnings, and errors of course + make check-world + + Your configure switches should include at least --enable-tap-tests + or else much of the Perl code won't get exercised. + +3) If you have the patience, it's worth eyeballing the "git diff" output + for any egregiously ugly changes. See below for cleanup ideas. + + +When you're done, "git commit" everything including the typedefs.list file +you used. -10) Remove Perl backup files after testing (*.bak) + +--------------------------------------------------------------------------- + +Cleaning up in case of failure or ugly output +--------------------------------------------- + +If you don't like the results for any particular file, "git checkout" +that file to undo the changes, patch the file as needed, then repeat +the indent process. + +pgindent will reflow any comment block that's not at the left margin. +If this messes up manual formatting that ought to be preserved, protect +the comment block with some dashes: + + /*---------- + * Text here will not be touched by pgindent. + *---------- + */ + +Odd spacing around typedef names might indicate an incomplete typedefs list. + +pgindent can get confused by #if sequences that look correct to the compiler +but have mismatched braces/parentheses when considered as a whole. Usually +that looks pretty unreadable to humans too, so best practice is to rearrange +the #if tests to avoid it. + +Sometimes, if pgindent or perltidy produces odd-looking output, it's because +of minor bugs like extra commas. Don't hesitate to clean that up while +you're at it. --------------------------------------------------------------------------- @@ -82,33 +122,25 @@ see: --------------------------------------------------------------------------- -Notes about excluded files --------------------------- +Which files are processed +------------------------- + +The pgindent run processes (nearly) all PostgreSQL *.c and *.h files, +but we currently exclude *.y and *.l files. Exceptions are listed +in exclude_file_patterns: src/include/storage/s_lock.h and src/include/port/atomics/ are excluded because they contain assembly code that pgindent tends to mess up. -src/include/snowball/libstemmer/ and src/backend/snowball/libstemmer/ -are excluded because those files are imported from an external project, -not maintained locally, and are machine-generated anyway. - src/interfaces/ecpg/test/expected/ is excluded to avoid breaking the ecpg regression tests. Several *.h files are included in regression output so should not be changed. ---------------------------------------------------------------------------- - -Obsolete typedef list creation instructions -------------------------------------------- - -To use pgindent: - -1) Build the source tree with _debug_ symbols and all possible configure options - -2) Install to /usr/local/pgsql - -3) Install all contrib modules - -4) Save a list of typedefs by running: +src/include/snowball/libstemmer/ and src/backend/snowball/libstemmer/ +are excluded because those files are imported from an external project, +not maintained locally, and are machine-generated anyway. Likewise for +plperl/ppport.h. - src/tools/find_typedef /usr/local/pgsql/bin /usr/local/pgsql/lib > /tmp/pgtypedefs +The perltidy run processes all *.pl and *.pm files, plus a few +executable Perl scripts that are not named that way. See the "find" +rules in pgperltidy for details. diff --git a/src/tools/pgindent/exclude_file_patterns b/src/tools/pgindent/exclude_file_patterns index 1a5866759e..97ba092658 100644 --- a/src/tools/pgindent/exclude_file_patterns +++ b/src/tools/pgindent/exclude_file_patterns @@ -1,4 +1,4 @@ -#list of file patterns to exclude from pgindent runs +#list of file patterns to exclude from pgindent runs, see notes in README /s_lock\.h$ /atomics/ /ecpg/test/expected/ diff --git a/src/tools/pgindent/pgperltidy b/src/tools/pgindent/pgperltidy new file mode 100755 index 0000000000..6098e18428 --- /dev/null +++ b/src/tools/pgindent/pgperltidy @@ -0,0 +1,23 @@ +#!/bin/sh + +# src/tools/pgindent/pgperltidy + +set -e + +# set this to override default perltidy program: +PERLTIDY=${PERLTIDY:-perltidy} + +# locate all Perl files in the tree +( + # take all .pl and .pm files + find . -type f -a \( -name '*.pl' -o -name '*.pm' \) + # take executable files that file(1) thinks are perl files + find . -type f -perm -100 -exec file {} \; | + egrep -i ':.*perl[0-9]*\>' | + cut -d: -f1 +) | +sort -u | +xargs $PERLTIDY --profile=src/tools/pgindent/perltidyrc + +# perltidyrc specifies --backup-and-modify-in-place, so get rid of .bak files +find . -type f -name '*.bak' | xargs rm -- 2.40.0