Re: Importing pg_bsd_indent into our source tree

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Importing pg_bsd_indent into our source tree
Date: 2023-02-09 21:55:32
Message-ID: 20230209215532.oomwhj5jyggmciq7@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2023-02-09 13:02:02 -0800, Andres Freund wrote:
> On 2023-02-09 13:30:30 -0500, Tom Lane wrote:
> > 0003 lacks meson support (anyone want to help with that?)
>
> I'll give it a go, unless somebody else wants to.

Did that in the attached.

I didn't convert the test though, due to the duplicating it'd create. Perhaps
we should just move it to a shell script? Or maybe it just doesn't matter
enough to bother with?

> Do we expect pg_bsd_indent to build / work on windows, right now? If it
> doesn't, do we want to make that a hard requirement?

> I'll have CI test that, once I added meson support.

It doesn't build as-is with msvc, but does build with mingw. Failure:
https://cirrus-ci.com/task/6290206869946368?logs=build#L1573

"cl" "-Isrc\tools/pg_bsd_indent\pg_bsd_indent.exe.p" "-Isrc\tools/pg_bsd_indent" "-I..\src\tools\pg_bsd_indent" "-Isrc\include" "-I..\src\include" "-Ic:\openssl\1.1\include" "-I..\src\include\port\win32" "-I..\src\include\port\win32_msvc" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/Od" "/Zi" "/DWIN32" "/DWINDOWS" "/D__WINDOWS__" "/D__WIN32__" "/D_CRT_SECURE_NO_DEPRECATE" "/D_CRT_NONSTDC_NO_DEPRECATE" "/wd4018" "/wd4244" "/wd4273" "/wd4101" "/wd4102" "/wd4090" "/wd4267" "/Fdsrc\tools/pg_bsd_indent\pg_bsd_indent.exe.p\args.c.pdb" /Fosrc/tools/pg_bsd_indent/pg_bsd_indent.exe.p/args.c.obj "/c" ../src/tools/pg_bsd_indent/args.c
../src/tools/pg_bsd_indent/args.c(179): error C2065: 'PATH_MAX': undeclared identifier
../src/tools/pg_bsd_indent/args.c(179): error C2057: expected constant expression
../src/tools/pg_bsd_indent/args.c(179): error C2466: cannot allocate an array of constant size 0
../src/tools/pg_bsd_indent/args.c(179): error C2133: 'fname': unknown size
../src/tools/pg_bsd_indent/args.c(183): warning C4034: sizeof returns 0
../src/tools/pg_bsd_indent/args.c(185): warning C4034: sizeof returns 0
[1557/2161] Compiling C object src/tools/pg_bsd_indent/pg_bsd_indent.exe.p/err.c.obj
[1558/2161] Precompiling header src/interfaces/ecpg/ecpglib/libecpg.dll.p/meson_pch-c.c
[1559/2161] Compiling C object src/tools/pg_bsd_indent/pg_bsd_indent.exe.p/indent.c.obj
FAILED: src/tools/pg_bsd_indent/pg_bsd_indent.exe.p/indent.c.obj
"cl" "-Isrc\tools/pg_bsd_indent\pg_bsd_indent.exe.p" "-Isrc\tools/pg_bsd_indent" "-I..\src\tools\pg_bsd_indent" "-Isrc\include" "-I..\src\include" "-Ic:\openssl\1.1\include" "-I..\src\include\port\win32" "-I..\src\include\port\win32_msvc" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/Od" "/Zi" "/DWIN32" "/DWINDOWS" "/D__WINDOWS__" "/D__WIN32__" "/D_CRT_SECURE_NO_DEPRECATE" "/D_CRT_NONSTDC_NO_DEPRECATE" "/wd4018" "/wd4244" "/wd4273" "/wd4101" "/wd4102" "/wd4090" "/wd4267" "/Fdsrc\tools/pg_bsd_indent\pg_bsd_indent.exe.p\indent.c.pdb" /Fosrc/tools/pg_bsd_indent/pg_bsd_indent.exe.p/indent.c.obj "/c" ../src/tools/pg_bsd_indent/indent.c
../src/tools/pg_bsd_indent/indent.c(63): error C2065: 'MAXPATHLEN': undeclared identifier
../src/tools/pg_bsd_indent/indent.c(63): error C2057: expected constant expression
../src/tools/pg_bsd_indent/indent.c(63): error C2466: cannot allocate an array of constant size 0

This specific issue at least should be easily fixable.

Freebsd emits a compiler warning:

[21:37:50.909] In file included from ../src/tools/pg_bsd_indent/indent.c:54:
[21:37:50.909] ../src/tools/pg_bsd_indent/indent.h:31:9: warning: 'nitems' macro redefined [-Wmacro-redefined]
[21:37:50.909] #define nitems(array) (sizeof (array) / sizeof (array[0]))
[21:37:50.909] ^
[21:37:50.909] /usr/include/sys/param.h:306:9: note: previous definition is here
[21:37:50.909] #define nitems(x) (sizeof((x)) / sizeof((x)[0]))
[21:37:50.909] ^
[21:37:50.911] 1 warning generated.

To we really want to require users to install pg_bsd_indent into PATH? Seems
like we ought to have a build target to invoke pgindent with a path to
pg_bsd_indent or such? But I guess we can address that later.

Independent of this specific patch: You seem to be generating your patch
series by invoking git show and redirecting that to a file? How do you apply a
series of such patches, while maintaining the commit messages? When git
format-patch is used, I can just use git am, but that doesn't work with your
patch series.

Greetings,

Andres Freund

Attachment Content-Type Size
v1-0001-import-pg_bsd_indent-verbatim.patch text/x-diff 179.0 KB
v1-0002-adjust-pg_bsd_indent-copyright.patch text/x-diff 9.7 KB
v1-0003-merge-into-our-build-system.patch text/x-diff 7.3 KB
v1-0004-fix-initialization-indenting.patch text/x-diff 1.5 KB
v2-0005-pg_bsd_indent-build-under-meson.patch text/x-diff 1.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-02-09 22:09:39 Re: ICU locale validation / canonicalization
Previous Message Jacob Champion 2023-02-09 21:46:04 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security