Обсуждение: Add pgindent test to check if codebase is correctly formatted
Had some time to watch code run through an extensive test suite, so thought I would propose this patch that is probably about 75% of the way to the stated $subject. I had to add in a hack for Meson, and I couldn't figure out a good hack for autotools. I think a good solution would be to distribute pgindent and pg_bsd_indent. At Neon, we are trying to format our extension code using pgindent. I am sure there are other extension authors out there too that format using pgindent. Distributing pg_bsd_indent and pgindent in the postgresql-devel package would be a great help to those of us that pgindent out of tree code. It would also have the added benefit of adding the tools to $PREFIX/bin, which would make the test that I added not need a hack to get the pg_bsd_indent executable. -- Tristan Partin Neon (https://neon.tech)
Вложения
On Tue, Jan 16, 2024 at 07:22:23PM -0600, Tristan Partin wrote: > I think a good solution would be to distribute pgindent and pg_bsd_indent. > At Neon, we are trying to format our extension code using pgindent. I am > sure there are other extension authors out there too that format using > pgindent. Distributing pg_bsd_indent and pgindent in the postgresql-devel > package would be a great help to those of us that pgindent out of tree code. > It would also have the added benefit of adding the tools to $PREFIX/bin, > which would make the test that I added not need a hack to get the > pg_bsd_indent executable. So your point is that pg_bsd_indent and pgindent are in the source tree, but not in any package distribution? Isn't that a packager decision? -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
On Tue Jan 16, 2024 at 7:27 PM CST, Bruce Momjian wrote: > On Tue, Jan 16, 2024 at 07:22:23PM -0600, Tristan Partin wrote: > > I think a good solution would be to distribute pgindent and pg_bsd_indent. > > At Neon, we are trying to format our extension code using pgindent. I am > > sure there are other extension authors out there too that format using > > pgindent. Distributing pg_bsd_indent and pgindent in the postgresql-devel > > package would be a great help to those of us that pgindent out of tree code. > > It would also have the added benefit of adding the tools to $PREFIX/bin, > > which would make the test that I added not need a hack to get the > > pg_bsd_indent executable. > > So your point is that pg_bsd_indent and pgindent are in the source tree, > but not in any package distribution? Isn't that a packager decision? It requires changes to at least the Meson build files. pg_bsd_indent is not marked for installation currently. There is a TODO there. pgindent has no install_data() for instance. pg_bsd_indent seemingly gets installed somewhere in autotools given the contents of its Makefile, but I didn't see anything in my install tree afterward. Sure RPM/DEB packagers can solve this issue downstream, but that doesn't help those of that run "meson install" or "make install" upstream. Packagers are probably more likely to package the tools if they are marked for installation by upstream too. Hope this helps to better explain what changes would be required within the Postgres source tree. -- Tristan Partin Neon (https://neon.tech)
On Tue, Jan 16, 2024 at 07:32:47PM -0600, Tristan Partin wrote: > It requires changes to at least the Meson build files. pg_bsd_indent is not > marked for installation currently. There is a TODO there. pgindent has no > install_data() for instance. pg_bsd_indent seemingly gets installed > somewhere in autotools given the contents of its Makefile, but I didn't see > anything in my install tree afterward. > > Sure RPM/DEB packagers can solve this issue downstream, but that doesn't > help those of that run "meson install" or "make install" upstream. Packagers > are probably more likely to package the tools if they are marked for > installation by upstream too. > > Hope this helps to better explain what changes would be required within the > Postgres source tree. Yes, it does, thanks. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you.
Hmm, should this also install typedefs.list and pgindent.man? What about the tooling to reformat Perl code? -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "Linux transformó mi computadora, de una `máquina para hacer cosas', en un aparato realmente entretenido, sobre el cual cada día aprendo algo nuevo" (Jaime Salinas)
On Wed Jan 17, 2024 at 3:50 AM CST, Alvaro Herrera wrote: > Hmm, should this also install typedefs.list and pgindent.man? > What about the tooling to reformat Perl code? Good point about pgindent.man. It would definitely be good to install alongside pgindent and pg_bsd_indent. I don't know if we need to install the typedefs.list file. I think it would just be good enough to also install the find_typedefs script. But it needs some fixing up first[0]. Extension authors can then just generate their own typedefs.list that will include the typedefs of the extension and the typedefs of the postgres types they use. At least, that is what we have found works at Neon. I cannot vouch for extension authors writing Perl but I think it could make sense to install the src/test/perl tree, so extension authors could more easily write tests for their extensions in Perl. But we could install the perltidy file and whatever else too. I keep my Perl writing to a minimum, so I am not the best person to vouch for these usecases. [0]: https://www.postgresql.org/message-id/aaa59ef5-dce8-7369-5cae-487727664127%40dunslane.net -- Tristan Partin Neon (https://neon.tech)