Horton

Thread

Commits

  1. Initial pgindent run for v12.

  1. pgindent run next week?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-17T14:29:46Z

    We should do a pgindent run fairly soon, so that people with patches
awaiting the next CF will have plenty of time to rebase them as necessary.
I don't want to do it right this minute, to avoid making trouble for the
several urgent patches we're trying to get done before Monday's beta1
wrap.  But after the beta is tagged seems like it'd be a good time.

Also, how do people feel about adopting the function prototype
indenting change discussed in 

https://www.postgresql.org/message-id/flat/CAEepm%3D0P3FeTXRcU5B2W3jv3PgRVZ-kGUXLGfd42FFhUROO3ug%40mail.gmail.com

?  The required change in pg_bsd_indent isn't quite done, but it
could be done by next week.

			regards, tom lane

  2. Re: pgindent run next week?

    Bruce Momjian <bruce@momjian.us> — 2019-05-17T17:23:36Z

    On Fri, May 17, 2019 at 10:29:46AM -0400, Tom Lane wrote:
> We should do a pgindent run fairly soon, so that people with patches
> awaiting the next CF will have plenty of time to rebase them as necessary.
> I don't want to do it right this minute, to avoid making trouble for the
> several urgent patches we're trying to get done before Monday's beta1
> wrap.  But after the beta is tagged seems like it'd be a good time.
> 
> Also, how do people feel about adopting the function prototype
> indenting change discussed in 
> 
> https://www.postgresql.org/message-id/flat/CAEepm%3D0P3FeTXRcU5B2W3jv3PgRVZ-kGUXLGfd42FFhUROO3ug%40mail.gmail.com
> 
> ?  The required change in pg_bsd_indent isn't quite done, but it
> could be done by next week.

Yes, I think we are good with everything above.  I am thinking you
should do the run since you did the pg_indent modifications.

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

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

  3. Re: pgindent run next week?

    Andres Freund <andres@anarazel.de> — 2019-05-17T17:27:00Z

    Hi,

On 2019-05-17 10:29:46 -0400, Tom Lane wrote:
> We should do a pgindent run fairly soon, so that people with patches
> awaiting the next CF will have plenty of time to rebase them as
> necessary.

+1

> I don't want to do it right this minute, to avoid making trouble for the
> several urgent patches we're trying to get done before Monday's beta1
> wrap.  But after the beta is tagged seems like it'd be a good time.

+1

> Also, how do people feel about adopting the function prototype
> indenting change discussed in

> https://www.postgresql.org/message-id/flat/CAEepm%3D0P3FeTXRcU5B2W3jv3PgRVZ-kGUXLGfd42FFhUROO3ug%40mail.gmail.com

I think it'd be a huge improvement. I find it pretty annoying having to
figure out the indentations to avoid unnecessary pgindent changes (after
Thomas' explanation as to why it happens, I usually just add a linebreak
after the return type, indent everything, and remove it).

Would we want to also apply this to the back branches to avoid spurious
conflicts?

Greetings,

Andres Freund

  4. Re: pgindent run next week?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-17T17:47:02Z

    Andres Freund <andres@anarazel.de> writes:
> On 2019-05-17 10:29:46 -0400, Tom Lane wrote:
>> Also, how do people feel about adopting the function prototype
>> indenting change discussed in
>> https://www.postgresql.org/message-id/flat/CAEepm%3D0P3FeTXRcU5B2W3jv3PgRVZ-kGUXLGfd42FFhUROO3ug%40mail.gmail.com

> I think it'd be a huge improvement.

Yeah, that's probably the biggest remaining bug/issue in pgindent.

> Would we want to also apply this to the back branches to avoid spurious
> conflicts?

I dunno, how far back are you thinking?  I've occasionally wished we
could reindent all the back branches to match HEAD, but realistically,
people carrying out-of-tree patches would scream.

			regards, tom lane

  5. Re: pgindent run next week?

    Bruce Momjian <bruce@momjian.us> — 2019-05-17T17:49:47Z

    On Fri, May 17, 2019 at 01:47:02PM -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-05-17 10:29:46 -0400, Tom Lane wrote:
> >> Also, how do people feel about adopting the function prototype
> >> indenting change discussed in
> >> https://www.postgresql.org/message-id/flat/CAEepm%3D0P3FeTXRcU5B2W3jv3PgRVZ-kGUXLGfd42FFhUROO3ug%40mail.gmail.com
> 
> > I think it'd be a huge improvement.
> 
> Yeah, that's probably the biggest remaining bug/issue in pgindent.
> 
> > Would we want to also apply this to the back branches to avoid spurious
> > conflicts?
> 
> I dunno, how far back are you thinking?  I've occasionally wished we
> could reindent all the back branches to match HEAD, but realistically,
> people carrying out-of-tree patches would scream.

My regular backpatch pain is SGML files.  :-(

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

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

  6. Re: pgindent run next week?

    Andres Freund <andres@anarazel.de> — 2019-05-17T18:11:30Z

    Hi,

On 2019-05-17 13:47:02 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
 > > Would we want to also apply this to the back branches to avoid spurious
> > conflicts?
> 
> I dunno, how far back are you thinking?  I've occasionally wished we
> could reindent all the back branches to match HEAD, but realistically,
> people carrying out-of-tree patches would scream.

I somehow thought we'd backpatched pgindent changes before, around when
moving to the newer version of indent. But I think we might just have
discussed that, and then didn't go for it...

Not sure if a three-way merge wouldn't take care of many, but not all,
the out-of-tree patch concerns.

Greetings,

Andres Freund

  7. Re: pgindent run next week?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-17T19:10:18Z

    Andres Freund <andres@anarazel.de> writes:
> On 2019-05-17 13:47:02 -0400, Tom Lane wrote:
>> I dunno, how far back are you thinking?  I've occasionally wished we
>> could reindent all the back branches to match HEAD, but realistically,
>> people carrying out-of-tree patches would scream.

> I somehow thought we'd backpatched pgindent changes before, around when
> moving to the newer version of indent. But I think we might just have
> discussed that, and then didn't go for it...

Yeah, we talked about it but never actually did it.

> Not sure if a three-way merge wouldn't take care of many, but not all,
> the out-of-tree patch concerns.

I was wondering about "patch --ignore-whitespace" myself.  In theory,
to the extent that our recent rounds of pgindent fixes just change
indentation, that would be able to cope (most of the time anyway).
But I don't think I'd want to just assume that without testing.

Anybody around here got large patches they're carrying against
back branches, that they could try reapplying after running
a newer version of pgindent?

			regards, tom lane

  8. Re: pgindent run next week?

    Mark Dilger <hornschnorter@gmail.com> — 2019-05-17T21:39:50Z

    
> On May 17, 2019, at 12:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Andres Freund <andres@anarazel.de> writes:
>> On 2019-05-17 13:47:02 -0400, Tom Lane wrote:
>>> I dunno, how far back are you thinking?  I've occasionally wished we
>>> could reindent all the back branches to match HEAD, but realistically,
>>> people carrying out-of-tree patches would scream.
> 
>> I somehow thought we'd backpatched pgindent changes before, around when
>> moving to the newer version of indent. But I think we might just have
>> discussed that, and then didn't go for it...
> 
> Yeah, we talked about it but never actually did it.
> 
>> Not sure if a three-way merge wouldn't take care of many, but not all,
>> the out-of-tree patch concerns.
> 
> I was wondering about "patch --ignore-whitespace" myself.  In theory,
> to the extent that our recent rounds of pgindent fixes just change
> indentation, that would be able to cope (most of the time anyway).
> But I don't think I'd want to just assume that without testing.
> 
> Anybody around here got large patches they're carrying against
> back branches, that they could try reapplying after running
> a newer version of pgindent?

I have forks of 9.1 and 9.5 that each amount to large changes
against the public sources, though I consider those forks to be
defunct.  If you want me to run some particular version of pg_indent
against the public sources of 9.1 and 9.5 and then try to merge the
changed sources into my forks, I could give it a try.  I'm not
sure if this is the sort of thing you have in mind....

mark

  9. Re: pgindent run next week?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-17T21:49:43Z

    Mark Dilger <hornschnorter@gmail.com> writes:
> On May 17, 2019, at 12:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Anybody around here got large patches they're carrying against
>> back branches, that they could try reapplying after running
>> a newer version of pgindent?

> I have forks of 9.1 and 9.5 that each amount to large changes
> against the public sources, though I consider those forks to be
> defunct.  If you want me to run some particular version of pg_indent
> against the public sources of 9.1 and 9.5 and then try to merge the
> changed sources into my forks, I could give it a try.  I'm not
> sure if this is the sort of thing you have in mind....

9.1 is probably too far back to be interesting, but it'd be good
to try the experiment with your 9.5 fork.

Assuming you only want to do this once, I'd suggest waiting till
I push the function-prototype changes to the pg_bsd_indent repo,
and then use that along with the latest pgindent.

			regards, tom lane

  10. Re: pgindent run next week?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-21T21:46:54Z

    Andres Freund <andres@anarazel.de> writes:
> On 2019-05-17 10:29:46 -0400, Tom Lane wrote:
>> We should do a pgindent run fairly soon, so that people with patches
>> awaiting the next CF will have plenty of time to rebase them as
>> necessary.
>> I don't want to do it right this minute, to avoid making trouble for the
>> several urgent patches we're trying to get done before Monday's beta1
>> wrap.  But after the beta is tagged seems like it'd be a good time.

> +1

Hearing no objections, I'll plan on running pgindent tomorrow sometime.

The new underlying pg_bsd_indent (2.1) is available now from

https://git.postgresql.org/git/pg_bsd_indent.git

if anyone wants to do further testing on it.  (To use it with current
pgindent, adjust the INDENT_VERSION value in that script.  You don't
really need to do anything else; the code rendered unnecessary by this
change won't do anything.)


> Would we want to also apply this to the back branches to avoid spurious
> conflicts?

I think we should hold off on any talk of that until we get some results
from Mark Dilger (or anyone else) on how much pain it would cause for
people carrying private patches.

			regards, tom lane

  11. Re: pgindent run next week?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-22T17:07:44Z

    I wrote:
> Hearing no objections, I'll plan on running pgindent tomorrow sometime.

And done.

> The new underlying pg_bsd_indent (2.1) is available now from
> https://git.postgresql.org/git/pg_bsd_indent.git

Please update your local copy if you have one.

			regards, tom lane

  12. Re: pgindent run next week?

    Mark Dilger <hornschnorter@gmail.com> — 2019-05-22T18:54:29Z

    On Wed, May 22, 2019 at 10:07 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > Hearing no objections, I'll plan on running pgindent tomorrow sometime.
>
> And done.
>
> > The new underlying pg_bsd_indent (2.1) is available now from
> > https://git.postgresql.org/git/pg_bsd_indent.git
>
> Please update your local copy if you have one.
>
>                         regards, tom lane
>

I cloned, built and used the new pg_bsd_indent to format my fork of
PostgreSQL 11 (not the 9.1 or 9.5 forks I previously mentioned) and
it caused me no problems whatsoever.  I don't have a strong preference,
but I would vote in favor of running pgindent on the back branches
rather than against, since to the extent that I might need to move
patches between forks of different versions, it will be easier to do if
they have the same indentation.  (In practice, this probably won't
come up for me, since the older forks are defunct and unlikely to
be patched by me.)

mark

  13. Re: pgindent run next week?

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-05-22T19:13:04Z

    On 2019-05-21 23:46, Tom Lane wrote:
>> Would we want to also apply this to the back branches to avoid spurious
>> conflicts?
> I think we should hold off on any talk of that until we get some results
> from Mark Dilger (or anyone else) on how much pain it would cause for
> people carrying private patches.

In my experience, changes to function declarations in header files
happen a lot in forks.  So applying the pgindent change to backbranches
would cause some trouble.

On the other hand, it seems to me that patches that we backpatch between
PostgreSQL branches should normally not touch function declarations in
header files, since that would be an ABI break.  So by not applying the
pgindent change in backbranches we don't lose anything.  And so it would
be better to just leave things as they are.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  14. Re: pgindent run next week?

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-05-22T19:27:18Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> In my experience, changes to function declarations in header files
> happen a lot in forks.  So applying the pgindent change to backbranches
> would cause some trouble.

> On the other hand, it seems to me that patches that we backpatch between
> PostgreSQL branches should normally not touch function declarations in
> header files, since that would be an ABI break.  So by not applying the
> pgindent change in backbranches we don't lose anything.  And so it would
> be better to just leave things as they are.

Maybe we could wait awhile and see how much pain we find in back-patching
across this change.  I have to admit that the v10 pgindent changes have
not been as painful as I expected them to be, so maybe this round will
also prove to be just an annoyance not a major PITA for that.

Another thought is that, at least in principle, we could re-indent only
.c files not .h files in the back branches.  But I'm not sure I believe
your argument that forks are more likely to touch exposed extern
declarations than local static declarations.

			regards, tom lane

  15. Re: pgindent run next week?

    Euler Taveira <euler@timbira.com.br> — 2019-05-22T21:16:42Z

    Em qua, 22 de mai de 2019 às 14:08, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
>
> I wrote:
> > Hearing no objections, I'll plan on running pgindent tomorrow sometime.
>
> And done.
>
> > The new underlying pg_bsd_indent (2.1) is available now from
> > https://git.postgresql.org/git/pg_bsd_indent.git
>
> Please update your local copy if you have one.
>
I give it a try in a fork of PostgreSQL 10. The difference between v10
and my fork is not huge. The stats are 56 files changed, 2240
insertions(+), 203 deletions(-) and patch size is 139 Kb. I have
conflicts in 3 of 19 .h files and 1 of 25 .c files. Like Mark, I don't
have a strong preference, however, re-indent files would reduce
developer time while preparing patches to back branches.


-- 
   Euler Taveira                                   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento