pgsql: pgindent run for 9.4

Started by Bruce Momjianover 11 years ago16 messages

bruce@momjian.us

over 11 years ago

pgindent run for 9.4

This includes removing tabs after periods in C comments, which was
applied to back branches, so this change should not effect backpatching.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/0a7832005792fa6dad171f9cadb8d587fe0dd800

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Kevin Grittner

kgrittn@ymail.com

over 11 years ago

In reply to: Bruce Momjian (#1)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Bruce Momjian <bruce@momjian.us> wrote:

pgindent run for 9.4

This includes removing tabs after periods in C comments, which was
applied to back branches, so this change should not effect backpatching.

http://git.postgresql.org/pg/commitdiff/0a7832005792fa6dad171f9cadb8d587fe0dd800

.../test/expected/compat_informix-test_informix2.c |    2 +-
src/interfaces/ecpg/test/expected/preproc-init.c |    2 +-
src/interfaces/ecpg/test/expected/sql-array.c      |    2 +-
src/interfaces/ecpg/test/expected/sql-code100.c    |    2 +-
src/interfaces/ecpg/test/expected/sql-copystdout.c |    2 +-
src/interfaces/ecpg/test/expected/sql-define.c    |    2 +-
src/interfaces/ecpg/test/expected/sql-dynalloc.c |    2 +-
src/interfaces/ecpg/test/expected/sql-dynalloc2.c |    2 +-
src/interfaces/ecpg/test/expected/sql-dyntest.c    |    2 +-
src/interfaces/ecpg/test/expected/sql-indicators.c |    2 +-
src/interfaces/ecpg/test/expected/thread-alloc.c |    2 +-
.../ecpg/test/expected/thread-descriptor.c        |    2 +-
src/interfaces/ecpg/test/expected/thread-prep.c    |    2 +-

The 13 tests above are broken by this commit. Probably the
directory should be excluded from pgindent processing.

While I haven't checked, I assume all other branches are affected.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Kevin Grittner (#2)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Kevin Grittner <kgrittn@ymail.com> writes:

Bruce Momjian <bruce@momjian.us> wrote:

pgindent run for 9.4

The 13 tests above are broken by this commit.� Probably the
directory should be excluded from pgindent processing.

What's broken? The buildfarm isn't complaining, and "make installcheck"
in src/interfaces/ecpg/test passes for me.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Kevin Grittner

kgrittn@ymail.com

over 11 years ago

In reply to: Tom Lane (#3)

1 attachment(s)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Tom Lane <tgl@sss.pgh.pa.us> wrote:

Kevin Grittner <kgrittn@ymail.com> writes:

Bruce Momjian <bruce@momjian.us> wrote:

pgindent run for 9.4

The 13 tests above are broken by this commit. Probably the
directory should be excluded from pgindent processing.

What's broken? The buildfarm isn't complaining, and "make installcheck"
in src/interfaces/ecpg/test passes for me.

On "make check-world" I get the attached. After the period, the
trailing tabs in the comment have been changed to spaces in
"expected", but are still tabs in "results".

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachments:

regression.diffsapplication/octet-stream; name=regression.diffsDownload

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/compat_informix-test_informix2.c	2014-05-06 12:45:26.769114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/compat_informix-test_informix2.c	2014-05-06 12:51:35.685113562 -0500
***************
*** 57,63 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 57,63 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/preproc-init.c	2014-05-06 12:45:26.769114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/preproc-init.c	2014-05-06 12:51:35.849113562 -0500
***************
*** 51,57 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 51,57 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/sql-array.c	2014-05-06 12:45:26.769114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/sql-array.c	2014-05-06 12:51:35.901113562 -0500
***************
*** 59,65 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 59,65 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/sql-code100.c	2014-05-06 12:45:26.769114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/sql-code100.c	2014-05-06 12:51:35.921113562 -0500
***************
*** 51,57 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 51,57 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/sql-copystdout.c	2014-05-06 12:45:26.769114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/sql-copystdout.c	2014-05-06 12:51:35.933113562 -0500
***************
*** 53,59 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 53,59 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/sql-define.c	2014-05-06 12:45:26.769114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/sql-define.c	2014-05-06 12:51:35.941113562 -0500
***************
*** 51,57 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 51,57 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/sql-dynalloc.c	2014-05-06 12:45:26.769114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/sql-dynalloc.c	2014-05-06 12:51:35.989113562 -0500
***************
*** 52,58 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 52,58 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/sql-dynalloc2.c	2014-05-06 12:45:26.773114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/sql-dynalloc2.c	2014-05-06 12:51:36.001113562 -0500
***************
*** 52,58 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 52,58 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/sql-dyntest.c	2014-05-06 12:45:26.773114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/sql-dyntest.c	2014-05-06 12:51:36.009113562 -0500
***************
*** 105,111 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 105,111 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/sql-indicators.c	2014-05-06 12:45:26.773114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/sql-indicators.c	2014-05-06 12:51:36.057113562 -0500
***************
*** 53,59 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 53,59 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/thread-prep.c	2014-05-06 12:45:26.773114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/thread-prep.c	2014-05-06 12:51:36.177113560 -0500
***************
*** 74,80 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 74,80 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/thread-alloc.c	2014-05-06 12:45:26.773114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/thread-alloc.c	2014-05-06 12:51:36.229113560 -0500
***************
*** 74,80 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 74,80 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

*** /home/kgrittn/pg/master/src/interfaces/ecpg/test/expected/thread-descriptor.c	2014-05-06 12:45:26.773114421 -0500
--- /home/kgrittn/pg/master/src/interfaces/ecpg/test/results/thread-descriptor.c	2014-05-06 12:51:36.265113560 -0500
***************
*** 65,71 ****
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.             */
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred
--- 65,71 ----
  	/* Element 0: set to 'W' if at least one other is 'W'	*/
  	/* 1: if 'W' at least one character string		*/
  	/* value was truncated when it was			*/
! 	/* stored into a host variable.				*/
  
  	/*
  	 * 2: if 'W' a (hopefully) non-fatal notice occurred

======================================================================

Bruce Momjian

bruce@momjian.us

over 11 years ago

In reply to: Kevin Grittner (#4)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

On Tue, May 6, 2014 at 12:38:41PM -0700, Kevin Grittner wrote:

Tom Lane <tgl@sss.pgh.pa.us> wrote:

Kevin Grittner <kgrittn@ymail.com> writes:

Bruce Momjian <bruce@momjian.us> wrote:

pgindent run for 9.4

The 13 tests above are broken by this commit.ï¿½ Probably the
directory should be excluded from pgindent processing.

What's broken?ï¿½ The buildfarm isn't complaining, and "make installcheck"
in src/interfaces/ecpg/test passes for me.

On "make check-world" I get the attached.ï¿½ After the period, the
trailing tabs in the comment have been changed to spaces in
"expected", but are still tabs in "results".

Yes, I had to modify those lines before I pushed the pgindent changes so
'make installcheck-world' would pass. It passes here for me now. Did
you do 'make maintainer-clean' before running the tests? That might
help.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Kevin Grittner

kgrittn@ymail.com

over 11 years ago

In reply to: Bruce Momjian (#5)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Bruce Momjian <bruce@momjian.us> wrote:

On Tue, May 6, 2014 at 12:38:41PM -0700, Kevin Grittner wrote:

Tom Lane <tgl@sss.pgh.pa.us> wrote:

Kevin Grittner <kgrittn@ymail.com> writes:

Bruce Momjian <bruce@momjian.us> wrote:

pgindent run for 9.4

The 13 tests above are broken by this commit. Probably the
directory should be excluded from pgindent processing.

What's broken? The buildfarm isn't complaining, and "make installcheck"
in src/interfaces/ecpg/test passes for me.

On "make check-world" I get the attached. After the period, the
trailing tabs in the comment have been changed to spaces in
"expected", but are still tabs in "results".

Yes, I had to modify those lines before I pushed the pgindent changes so
'make installcheck-world' would pass. It passes here for me now. Did
you do 'make maintainer-clean' before running the tests? That might
help.

It occurred to me after my last post that I had just done a "make
world" without any cleanup when I pulled that, and had started a
full build from "make maintainer-clean" when you sent that. :-)

I'll let you know either way when I get results from that.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Kevin Grittner

kgrittn@ymail.com

over 11 years ago

In reply to: Kevin Grittner (#6)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Kevin Grittner <kgrittn@ymail.com> wrote:

It occurred to me after my last post that I had just done a "make
world" without any cleanup when I pulled that, and had started a
full build from "make maintainer-clean" when you sent that. :-)

I'll let you know either way when I get results from that.

Yeah, after "make maintainer-clean" it's fine. Another case where
--enable-depend doesn't handle things. I don't know whether we can
or should try to fix that.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Kevin Grittner (#6)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Kevin Grittner <kgrittn@ymail.com> writes:

Bruce Momjian <bruce@momjian.us> wrote:

Yes, I had to modify those lines before I pushed the pgindent changes so
'make installcheck-world' would pass.� It passes here for me now.� Did
you do 'make maintainer-clean' before running the tests?� That might
help.

It occurred to me after my last post that I had just done a "make
world" without any cleanup when I pulled that, and had started a
full build from "make maintainer-clean" when you sent that.� :-)

FWIW, I did a "make distclean" before pulling the update, which is
my usual habit, and it worked fine. A look at the make rules for
ecpg suggests that "make clean" is enough to get rid of all the
derived files for the tests.

But having said that, if this didn't work then there's something broken
about the make rules for the ecpg tests. I'm a bit suspicious of commit
69e9768e7b183d4b276d0e067a5a0000689580eb.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Bruce Momjian

bruce@momjian.us

over 11 years ago

In reply to: Tom Lane (#8)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

On Tue, May 6, 2014 at 03:54:24PM -0400, Tom Lane wrote:

Kevin Grittner <kgrittn@ymail.com> writes:

Bruce Momjian <bruce@momjian.us> wrote:

Yes, I had to modify those lines before I pushed the pgindent changes so
'make installcheck-world' would pass.� It passes here for me now.� Did
you do 'make maintainer-clean' before running the tests?� That might
help.

It occurred to me after my last post that I had just done a "make
world" without any cleanup when I pulled that, and had started a
full build from "make maintainer-clean" when you sent that.� :-)

FWIW, I did a "make distclean" before pulling the update, which is
my usual habit, and it worked fine. A look at the make rules for
ecpg suggests that "make clean" is enough to get rid of all the
derived files for the tests.

But having said that, if this didn't work then there's something broken
about the make rules for the ecpg tests. I'm a bit suspicious of commit
69e9768e7b183d4b276d0e067a5a0000689580eb.

What _is_ odd is that I had to change these files after the pgindent run
in head, but _not_ in the back branches when I removed the tabs from
comments. I assume there is something new in 9.4 about they way they
are built.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Bruce Momjian (#9)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Bruce Momjian <bruce@momjian.us> writes:

On Tue, May 6, 2014 at 03:54:24PM -0400, Tom Lane wrote:

But having said that, if this didn't work then there's something broken
about the make rules for the ecpg tests. I'm a bit suspicious of commit
69e9768e7b183d4b276d0e067a5a0000689580eb.

I looked into this, and find that the cause of the problem is that
pgindent touched src/interfaces/ecpg/include/sqlca.h, which is copied
verbatim into preprocessed files by the ecpg preprocessor, so the expected
files had to change in tandem. This amounts to a dependency, but the make
rules don't know about it. Should they? That particular file changes so
seldom that it'd hardly be worth worrying about, but I'm not sure which
other files can get copied similarly.

What _is_ odd is that I had to change these files after the pgindent run
in head, but _not_ in the back branches when I removed the tabs from
comments. I assume there is something new in 9.4 about they way they
are built.

I'm confused by this statement. Your tab-adjustment commits in the back
branches also touched both sqlca.h and the ecpg expected files.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11

Bruce Momjian

bruce@momjian.us

over 11 years ago

In reply to: Tom Lane (#10)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

On Tue, May 6, 2014 at 05:05:00PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Tue, May 6, 2014 at 03:54:24PM -0400, Tom Lane wrote:

But having said that, if this didn't work then there's something broken
about the make rules for the ecpg tests. I'm a bit suspicious of commit
69e9768e7b183d4b276d0e067a5a0000689580eb.

I looked into this, and find that the cause of the problem is that
pgindent touched src/interfaces/ecpg/include/sqlca.h, which is copied
verbatim into preprocessed files by the ecpg preprocessor, so the expected
files had to change in tandem. This amounts to a dependency, but the make
rules don't know about it. Should they? That particular file changes so
seldom that it'd hardly be worth worrying about, but I'm not sure which
other files can get copied similarly.

What _is_ odd is that I had to change these files after the pgindent run
in head, but _not_ in the back branches when I removed the tabs from
comments. I assume there is something new in 9.4 about they way they
are built.

I'm confused by this statement. Your tab-adjustment commits in the back
branches also touched both sqlca.h and the ecpg expected files.

They probably did in the back branches as I hit _all_ C files. I wonder
if pgindent somehow skipped some of them.

Ah, found it. There is an excludes pattern file list I had forgotten
about; it has:

/s_lock\.h$
/ecpg/test/expected/
/snowball/libstemmer/
/ecpg/include/(sqlda|sqltypes)\.h$
/ecpg/include/preproc/struct\.h$
/pl/plperl/ppport\.h$

I am thinking I should back out the tab/comment changes in those files
in the back branches, though I would then need to adjust the ecpg
regression tests. In practice, these files are rarely patched, so it
might be fine to just leave them alone.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Tom Lane (#10)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

I wrote:

I looked into this, and find that the cause of the problem is that
pgindent touched src/interfaces/ecpg/include/sqlca.h, which is copied
verbatim into preprocessed files by the ecpg preprocessor, so the expected
files had to change in tandem. This amounts to a dependency, but the make
rules don't know about it. Should they? That particular file changes so
seldom that it'd hardly be worth worrying about, but I'm not sure which
other files can get copied similarly.

While I'm looking at it: there's no dependency forcing the test .c files
to get rebuilt after the ecpg preprocessor changes, either, and that
seems much more likely to be a routine problem.

Arguably, we need some more dependencies in this rule in
ecpg/test/Makefile.regress:

%.c: %.pgc ../regression.h
$(ECPG) -o $@ -I$(srcdir) $<

I also notice that some of the subdirectory makefiles that include
Makefile.regress have custom build rules that seem mostly duplicative
of this one, except for passing different switches to ecpg. Those
would likewise need additions to their dependency lists, which suggests
that the "../regression.h" part ought to be wrapped up in some macro.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Bruce Momjian (#11)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Bruce Momjian <bruce@momjian.us> writes:

Ah, found it. There is an excludes pattern file list I had forgotten
about; it has:

/s_lock\.h$
/ecpg/test/expected/
/snowball/libstemmer/
/ecpg/include/(sqlda|sqltypes)\.h$
/ecpg/include/preproc/struct\.h$
/pl/plperl/ppport\.h$

Ah, so you've been excluding some of the ecpg/include/ header files but
not sqlca.h.

I am thinking I should back out the tab/comment changes in those files
in the back branches, though I would then need to adjust the ecpg
regression tests. In practice, these files are rarely patched, so it
might be fine to just leave them alone.

No, let's not back them out. The real question here is why sqlca.h is
treated differently from those other three. At least in HEAD, I'd be
inclined to pgindent all of ecpg/include/ and just deal with any ensuing
test fallout. As long as updating the expected files is part of your
pgindent procedure, why not?

IOW, I get the reasons for those other exclusions:

s_lock.h: lots of inline ASM which pgindent doesn't deal well with
/snowball/libstemmer/: upstream code not maintained by us
ppport.h: ditto

But I don't see the reason why we shouldn't expect ecpg's headers to
conform to our layout rules.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14

Bruce Momjian

bruce@momjian.us

over 11 years ago

In reply to: Tom Lane (#13)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

On Tue, May 6, 2014 at 05:35:15PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

Ah, found it. There is an excludes pattern file list I had forgotten
about; it has:

/s_lock\.h$
/ecpg/test/expected/
/snowball/libstemmer/
/ecpg/include/(sqlda|sqltypes)\.h$
/ecpg/include/preproc/struct\.h$
/pl/plperl/ppport\.h$

Ah, so you've been excluding some of the ecpg/include/ header files but
not sqlca.h.

I am thinking I should back out the tab/comment changes in those files
in the back branches, though I would then need to adjust the ecpg
regression tests. In practice, these files are rarely patched, so it
might be fine to just leave them alone.

No, let's not back them out. The real question here is why sqlca.h is
treated differently from those other three. At least in HEAD, I'd be
inclined to pgindent all of ecpg/include/ and just deal with any ensuing
test fallout. As long as updating the expected files is part of your
pgindent procedure, why not?

IOW, I get the reasons for those other exclusions:

s_lock.h: lots of inline ASM which pgindent doesn't deal well with
/snowball/libstemmer/: upstream code not maintained by us
ppport.h: ditto

But I don't see the reason why we shouldn't expect ecpg's headers to
conform to our layout rules.

I don't know who ecpg got in there. Let me know what you would like
done.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Bruce Momjian (#14)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

Bruce Momjian <bruce@momjian.us> writes:

On Tue, May 6, 2014 at 05:35:15PM -0400, Tom Lane wrote:

But I don't see the reason why we shouldn't expect ecpg's headers to
conform to our layout rules.

I don't know who ecpg got in there. Let me know what you would like
done.

What I'm suggesting is that, in HEAD only, you remove these exclusion
entries:

/ecpg/include/(sqlda|sqltypes)\.h$
/ecpg/include/preproc/struct\.h$

then redo the pgindent run (presumably only those three files will change)
and make any necessary updates in the ecpg expected files.

Note that it's just chance that the back branch updates didn't hit those
three files already, since you said you weren't using the filter on them.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16

Bruce Momjian

bruce@momjian.us

over 11 years ago

In reply to: Tom Lane (#15)

Re: [COMMITTERS] pgsql: pgindent run for 9.4

On Tue, May 6, 2014 at 06:24:47PM -0400, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

On Tue, May 6, 2014 at 05:35:15PM -0400, Tom Lane wrote:

But I don't see the reason why we shouldn't expect ecpg's headers to
conform to our layout rules.

I don't know who ecpg got in there. Let me know what you would like
done.

What I'm suggesting is that, in HEAD only, you remove these exclusion
entries:

/ecpg/include/(sqlda|sqltypes)\.h$
/ecpg/include/preproc/struct\.h$

then redo the pgindent run (presumably only those three files will change)
and make any necessary updates in the ecpg expected files.

OK, done. ecpg exclusion removed, pgindent rerun (it only changed
sqlda.h), and ecpg regression tests updated, all only in head.

Note that it's just chance that the back branch updates didn't hit those
three files already, since you said you weren't using the filter on them.

Right.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers