Thread
Commits
-
Update typedefs.list and re-run pgindent
- eaedf0df7197 11.0 landed
-
pgindent run?
Robert Haas <robertmhaas@gmail.com> — 2017-11-28T19:51:06Z
If nobody minds too much, I'd like to update typedefs.list and pgindent the whole tree again. We've generally done a pretty good job with pgindenting patches as they are committed this cycle, but we're starting to accumulate things here and there that are not indented according to what pgindent likes -- and that makes it harder to keep later patches indented properly. You can reindent the files they touch, but then you have to strip back out the changes that are unrelated to the patch you're working on. There are only four source files where more than a dozen lines will be touched, so I don't think this will inconvenience anyone too much: src/backend/access/transam/xlog.c | 14 +++--- src/backend/tcop/postgres.c | 18 +++---- src/backend/optimizer/util/relnode.c | 34 +++++++------- src/backend/catalog/partition.c | 39 ++++++++-------- src/backend/utils/mmgr/generation.c | 62 ++++++++++++------------- And the changes to typedefs.list would be, as of now, like this: src/tools/pgindent/typedefs.list | 53 ++++++++++++++------- Obviously, one of my motivations here is that I keep ending up commiting patches to partition.c to fix bugs and add features, and the fact I've missed that a few times is now coming back to haunt me. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: pgindent run?
Andres Freund <andres@anarazel.de> — 2017-11-28T19:54:46Z
Hi, On 2017-11-28 14:51:06 -0500, Robert Haas wrote: > If nobody minds too much, I'd like to update typedefs.list and > pgindent the whole tree again. +1. Greetings, Andres Freund
-
Re: pgindent run?
Robert Haas <robertmhaas@gmail.com> — 2017-11-28T19:55:50Z
On Tue, Nov 28, 2017 at 2:51 PM, Robert Haas <robertmhaas@gmail.com> wrote: > There are only four source files where more than a dozen lines will be touched... ...and of course by "four" I mean "five". -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: pgindent run?
Mark Dilger <hornschnorter@gmail.com> — 2017-11-28T20:06:34Z
> On Nov 28, 2017, at 11:51 AM, Robert Haas <robertmhaas@gmail.com> wrote: > > If nobody minds too much, I'd like to update typedefs.list and > pgindent the whole tree again. We've generally done a pretty good job > with pgindenting patches as they are committed this cycle, but we're > starting to accumulate things here and there that are not indented > according to what pgindent likes -- and that makes it harder to keep > later patches indented properly. You can reindent the files they > touch, but then you have to strip back out the changes that are > unrelated to the patch you're working on. There are only four source > files where more than a dozen lines will be touched, so I don't think > this will inconvenience anyone too much: > > src/backend/access/transam/xlog.c | 14 +++--- > src/backend/tcop/postgres.c | 18 +++---- > src/backend/optimizer/util/relnode.c | 34 +++++++------- > src/backend/catalog/partition.c | 39 ++++++++-------- > src/backend/utils/mmgr/generation.c | 62 ++++++++++++------------- > > And the changes to typedefs.list would be, as of now, like this: > > src/tools/pgindent/typedefs.list | 53 ++++++++++++++------- > > Obviously, one of my motivations here is that I keep ending up > commiting patches to partition.c to fix bugs and add features, and the > fact I've missed that a few times is now coming back to haunt me. I have no objection, but if the community intends to keep everything indented per project standards, why is there no git hook to reject improperly indented code at commit time? I've suffered some pain trying to merge code pre-global-indent-run into a branch post-global-indent-run and would rather this not keep happening. Sorry if this has been debated before. A quick search did not turn up any previous conversation, but that doesn't mean it has never been discussed. mark
-
Re: pgindent run?
Tom Lane <tgl@sss.pgh.pa.us> — 2017-11-28T20:37:29Z
Andres Freund <andres@anarazel.de> writes: > On 2017-11-28 14:51:06 -0500, Robert Haas wrote: >> If nobody minds too much, I'd like to update typedefs.list and >> pgindent the whole tree again. > +1. OK by me --- I've several times restrained myself from just doing an ad-hoc reindent on some of these files. regards, tom lane
-
Re: pgindent run?
Tom Lane <tgl@sss.pgh.pa.us> — 2017-11-28T20:47:17Z
Mark Dilger <hornschnorter@gmail.com> writes: > I have no objection, but if the community intends to keep everything > indented per project standards, why is there no git hook to reject > improperly indented code at commit time? I've suffered some pain > trying to merge code pre-global-indent-run into a branch > post-global-indent-run and would rather this not keep happening. I think that'd be taking it too far, especially given that the dependency on a typedefs list means that the git hook might have a different idea of what's correctly indented than the committer does. It'd be very hard to debug such discrepancies and figure out what would satisfy the hook. In the end we're trying to minimize the net amount of pain involved. I doubt that mechanized enforcement would fall at the global minimum. regards, tom lane
-
Re: pgindent run?
Thomas Munro <thomas.munro@enterprisedb.com> — 2017-11-28T21:23:11Z
On Wed, Nov 29, 2017 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Mark Dilger <hornschnorter@gmail.com> writes: >> I have no objection, but if the community intends to keep everything >> indented per project standards, why is there no git hook to reject >> improperly indented code at commit time? I've suffered some pain >> trying to merge code pre-global-indent-run into a branch >> post-global-indent-run and would rather this not keep happening. > > I think that'd be taking it too far, especially given that the dependency > on a typedefs list means that the git hook might have a different idea > of what's correctly indented than the committer does. It'd be very hard > to debug such discrepancies and figure out what would satisfy the hook. > > In the end we're trying to minimize the net amount of pain involved. > I doubt that mechanized enforcement would fall at the global minimum. Is there no way to get reasonable indentation that doesn't depend on that typedefs list? -- Thomas Munro http://www.enterprisedb.com
-
Re: pgindent run?
Tom Lane <tgl@sss.pgh.pa.us> — 2017-11-28T21:38:12Z
Thomas Munro <thomas.munro@enterprisedb.com> writes: > On Wed, Nov 29, 2017 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I think that'd be taking it too far, especially given that the dependency >> on a typedefs list means that the git hook might have a different idea >> of what's correctly indented than the committer does. It'd be very hard >> to debug such discrepancies and figure out what would satisfy the hook. > Is there no way to get reasonable indentation that doesn't depend on > that typedefs list? Perhaps, but not with the tool we've got. It's well known that C is unparseable without knowing which identifiers are typedefs, so it doesn't exactly surprise me that it might not be sanely indentable without knowing that. But I've not thought hard about it, nor looked for alternate tools. regards, tom lane
-
Re: pgindent run?
Mark Dilger <hornschnorter@gmail.com> — 2017-11-28T22:34:08Z
> On Nov 28, 2017, at 12:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Mark Dilger <hornschnorter@gmail.com> writes: >> I have no objection, but if the community intends to keep everything >> indented per project standards, why is there no git hook to reject >> improperly indented code at commit time? I've suffered some pain >> trying to merge code pre-global-indent-run into a branch >> post-global-indent-run and would rather this not keep happening. > > I think that'd be taking it too far, especially given that the dependency > on a typedefs list means that the git hook might have a different idea > of what's correctly indented than the committer does. It'd be very hard > to debug such discrepancies and figure out what would satisfy the hook. It sounds like it just requires that the committer also commit any changes to the typedefs list, such that the indenter run by the git hook can use the same list the committer is using. For many commits, the typedefs list won't change, and the hook would just use the most recent one from the repository. Barring any objections, I'll see if I can make that work on my local git repo and post a patch if so. mark
-
Re: pgindent run?
Tom Lane <tgl@sss.pgh.pa.us> — 2017-11-28T22:57:48Z
Mark Dilger <hornschnorter@gmail.com> writes: >> On Nov 28, 2017, at 12:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I think that'd be taking it too far, especially given that the dependency >> on a typedefs list means that the git hook might have a different idea >> of what's correctly indented than the committer does. It'd be very hard >> to debug such discrepancies and figure out what would satisfy the hook. > It sounds like it just requires that the committer also commit any changes > to the typedefs list, such that the indenter run by the git hook can use the > same list the committer is using. For many commits, the typedefs list won't > change, and the hook would just use the most recent one from the repository. > Barring any objections, I'll see if I can make that work on my local git repo > and post a patch if so. The other problem that would have to be considered is cross-branch variation in the indent rules. We've generally been in the habit of back-patching HEAD diffs without worrying about whether they meet back-branch rules; certainly nobody maintains typedefs.list in the back branches. Maybe the most expedient answer for that is to only enforce indentation in HEAD. I'm still not really on board with this though. I can definitely see the day coming when it would block a security patch and somebody would be scrambling desperately to fix their indentation under time pressure, even though perhaps the patch had been fine when created. regards, tom lane
-
Re: pgindent run?
Mark Dilger <hornschnorter@gmail.com> — 2017-11-28T23:16:18Z
> On Nov 28, 2017, at 2:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Mark Dilger <hornschnorter@gmail.com> writes: >>> On Nov 28, 2017, at 12:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> I think that'd be taking it too far, especially given that the dependency >>> on a typedefs list means that the git hook might have a different idea >>> of what's correctly indented than the committer does. It'd be very hard >>> to debug such discrepancies and figure out what would satisfy the hook. > >> It sounds like it just requires that the committer also commit any changes >> to the typedefs list, such that the indenter run by the git hook can use the >> same list the committer is using. For many commits, the typedefs list won't >> change, and the hook would just use the most recent one from the repository. > >> Barring any objections, I'll see if I can make that work on my local git repo >> and post a patch if so. > > The other problem that would have to be considered is cross-branch > variation in the indent rules. We've generally been in the habit of > back-patching HEAD diffs without worrying about whether they meet > back-branch rules; certainly nobody maintains typedefs.list in the > back branches. Maybe the most expedient answer for that is to only > enforce indentation in HEAD. > > I'm still not really on board with this though. I can definitely > see the day coming when it would block a security patch and somebody > would be scrambling desperately to fix their indentation under time > pressure, even though perhaps the patch had been fine when created. Ok, I'll consider the idea dead. I don't see any solution to that. mark
-
Re: pgindent run?
Bruce Momjian <bruce@momjian.us> — 2018-01-24T03:22:47Z
On Tue, Nov 28, 2017 at 04:38:12PM -0500, Tom Lane wrote: > Thomas Munro <thomas.munro@enterprisedb.com> writes: > > On Wed, Nov 29, 2017 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> I think that'd be taking it too far, especially given that the dependency > >> on a typedefs list means that the git hook might have a different idea > >> of what's correctly indented than the committer does. It'd be very hard > >> to debug such discrepancies and figure out what would satisfy the hook. > > > Is there no way to get reasonable indentation that doesn't depend on > > that typedefs list? > > Perhaps, but not with the tool we've got. > > It's well known that C is unparseable without knowing which identifiers > are typedefs, so it doesn't exactly surprise me that it might not be > sanely indentable without knowing that. But I've not thought hard about > it, nor looked for alternate tools. For people curious about the C typedef parsing details: http://calculist.blogspot.com/2009/02/c-typedef-parsing-problem.html https://stackoverflow.com/questions/243383/why-cant-c-be-parsed-with-a-lr1-parser -- 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 +
-
Re: pgindent run?
Andres Freund <andres@anarazel.de> — 2018-01-24T18:32:29Z
On 2018-01-23 22:22:47 -0500, Bruce Momjian wrote: > On Tue, Nov 28, 2017 at 04:38:12PM -0500, Tom Lane wrote: > > Thomas Munro <thomas.munro@enterprisedb.com> writes: > > > On Wed, Nov 29, 2017 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > >> I think that'd be taking it too far, especially given that the dependency > > >> on a typedefs list means that the git hook might have a different idea > > >> of what's correctly indented than the committer does. It'd be very hard > > >> to debug such discrepancies and figure out what would satisfy the hook. > > > > > Is there no way to get reasonable indentation that doesn't depend on > > > that typedefs list? > > > > Perhaps, but not with the tool we've got. > > > > It's well known that C is unparseable without knowing which identifiers > > are typedefs, so it doesn't exactly surprise me that it might not be > > sanely indentable without knowing that. But I've not thought hard about > > it, nor looked for alternate tools. > > For people curious about the C typedef parsing details: > > http://calculist.blogspot.com/2009/02/c-typedef-parsing-problem.html > https://stackoverflow.com/questions/243383/why-cant-c-be-parsed-with-a-lr1-parser FWIW, I think this problem could just as well be addressed with a few printing heuristics instead of actually needing an actual list of typedefs. There'd be one or two edge cases of bad formatting, but the end result would be far less painful than what we have today, were basically nobody can format their patches without a lot of manual cherry-picking or large scale unrelated whitespace changes. Greetings, Andres Freund
-
Re: pgindent run?
Tom Lane <tgl@sss.pgh.pa.us> — 2018-01-24T19:34:07Z
Andres Freund <andres@anarazel.de> writes: > FWIW, I think this problem could just as well be addressed with a few > printing heuristics instead of actually needing an actual list of > typedefs. Step right up and implement that, and we'd all be happier. Certainly the typedefs list is a pain in the rear. > There'd be one or two edge cases of bad formatting, but the > end result would be far less painful than what we have today, were > basically nobody can format their patches without a lot of manual > cherry-picking or large scale unrelated whitespace changes. IMO, the big problem here is that people commit large changes without having pgindent'd them first. regards, tom lane
-
Re: pgindent run?
Andres Freund <andres@anarazel.de> — 2018-01-24T19:42:28Z
On January 24, 2018 11:34:07 AM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote: >Andres Freund <andres@anarazel.de> writes: >> There'd be one or two edge cases of bad formatting, but the >> end result would be far less painful than what we have today, were >> basically nobody can format their patches without a lot of manual >> cherry-picking or large scale unrelated whitespace changes. > >IMO, the big problem here is that people commit large changes without >having pgindent'd them first. Well, I think it'd really have to be every patch that's indented properly. And that's hard given the way typedefs.list is maintained. Without most new typedefs included one continually reindents with a lot of damage. It'd be less bad is we automated the maintenance of the lists so a) patch authors can automatically add to the list and include that in commits b) the committed list gets updated automatically every few days based on bf results c) there's automated whole tree pgindent runs every few days. Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.