Re: Preliminary results for proposed new pgindent implementation
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Piotr Stefaniak <postgres@piotr-stefaniak.me>
Cc: Bruce Momjian <bruce@momjian.us>, Andres Freund <andres@anarazel.de>,
Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
Alvaro Herrera <alvherre@2ndquadrant.com>,
Robert Haas <robertmhaas@gmail.com>,
"Stephen Frost" <sfrost@snowman.net>,
"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-06-17T19:55:11Z
Lists: pgsql-hackers
Attachments
I wrote: > Piotr Stefaniak <postgres@piotr-stefaniak.me> writes: >> There are also the "portability fixes" and they're the main problem. > Fair enough. I spent some time looking into this. I reverted your commits 198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with standard C equivalents) and fb10acb040b90bdcbad09defd303363db29257d1 (Remove inclusion of sys/cdefs.h) locally and tried to build without those. I've successfully worked around the err.h change by adding cut-down versions of FreeBSD 11's err.h and err.c to the fileset (see attached). However, it's proving impossible to work around having "#include <sys/cdefs.h>" as the first live code in the files. I thought maybe we could provide a dummy cdefs.h file, but that breaks things on platforms where cdefs.h is a real thing and is relied on by other system headers --- which includes both Linux and BSD. It seems we would have to have something like #ifdef HAVE_SYS_CDEFS_H, but that is already a departure from FreeBSD practice. So what I'm currently thinking is that we have to diverge from the FreeBSD sources to the extent of removing #include <sys/cdefs.h> and the __FBSDID() calls, and instead inserting #include "c.h" to pick up PG's own portability definitions. The thing that forced me into the latter is that there seems no way to avoid compiler warnings if we don't decorate the declarations of err() and errx() with noreturn and printf-format attributes --- and we need c.h to provide portable ways of writing those. But there are probably other portability things that we'll need c.h for, anyway, especially if we want to make it work on Windows. So I'm thinking this is a small and easily maintainable difference from the upstream FreeBSD files. When I inserted #include "c.h", I got duplicate-macro-definition warnings about "true" and "false", so I would ask you to add this: --- freebsd_indent/indent_globs.h 2017-06-16 11:06:53.329712682 -0400 +++ new/indent_globs.h 2017-06-17 14:45:41.388015754 -0400 @@ -43,8 +43,12 @@ * of code */ +#ifndef false #define false 0 +#endif +#ifndef true #define true 1 +#endif FILE *input; /* the fid for the input file */ Other than that, I think this is a workable compromise on the portability questions. regards, tom lane
Commits
-
Manually un-break a few URLs that pgindent used to insist on splitting.
- 780b3a4c43fd 10.0 landed
-
Remove entab and associated detritus.
- 81f056c7256f 10.0 landed
-
Phase 3 of pgindent updates.
- 382ceffdf7f6 10.0 landed
-
Phase 2 of pgindent updates.
- c7b8998ebbf3 10.0 landed
-
Initial pgindent run with pg_bsd_indent version 2.0.
- e3860ffa4dd0 10.0 landed
-
Adjust pgindent script to use pg_bsd_indent 2.0.
- 8ff6d4ec7840 10.0 landed
-
Final pgindent run with old pg_bsd_indent (version 1.3).
- 9ef2dbefc7fb 10.0 landed