cpluspluscheck/headerscheck require build in REL_16_STABLE

Started by Marina Polyakovaalmost 2 years ago6 messages
#1Marina Polyakova
m.polyakova@postgrespro.ru
1 attachment(s)

Hello, hackers!

When running cpluspluscheck/headerscheck on REL_16_STABLE [1]https://github.com/postgres/postgres/commit/e177da5c87a10abac97c028bfb427bafb7353aa2 I found
that everything was ok only if it was preceded by a build and make
maintainer-clean was not used:

$ ./configure --without-icu --with-perl --with-python > configure.txt &&
make -j16 > make.txt &&
make -j16 clean > clean.txt &&
make -j16 cpluspluscheck > cpluspluscheck.txt

$ ./configure --without-icu --with-perl --with-python > configure_1.txt
&&
make -j16 > make.txt &&
make -j16 distclean > clean.txt &&
./configure --without-icu --with-perl --with-python > configure_2.txt &&
make -j16 cpluspluscheck > cpluspluscheck.txt

Otherwise cpluspluscheck/headerscheck will fail:

$ ./configure --without-icu --with-perl --with-python > configure_1.txt
&&
make -j16 > make.txt &&
make -j16 maintainer-clean > clean.txt &&
./configure --without-icu --with-perl --with-python > configure_2.txt &&
make -j16 cpluspluscheck > cpluspluscheck.txt

$ ./configure --without-icu --with-perl --with-python > configure.txt &&
make -j16 cpluspluscheck > cpluspluscheck.txt

In file included from /tmp/cpluspluscheck.Zy4645/test.cpp:3:
/home/marina/postgrespro/postgrespro/src/backend/parser/gramparse.h:29:10:
fatal error: gram.h: No such file or directory
29 | #include "gram.h"
| ^~~~~~~~
compilation terminated.
In file included from /tmp/cpluspluscheck.Zy4645/test.cpp:3:
/home/marina/postgrespro/postgrespro/src/backend/utils/adt/jsonpath_internal.h:26:10:
fatal error: jsonpath_gram.h: No such file or directory
26 | #include "jsonpath_gram.h"
| ^~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [GNUmakefile:141: cpluspluscheck] Error 1

In the other branches everything is fine: these problems begin with
commits [2]https://github.com/postgres/postgres/commit/dac048f71ebbcf2f980d280711f8ff8001331c5d (jsonpath_gram.h) and [3]https://github.com/postgres/postgres/commit/ecaf7c5df54f7fa9df2fdc7225d2bb4e283f0081 (gram.h) and in the master branch
there're no such problems after commit [4]https://github.com/postgres/postgres/commit/721856ff24b3722ce8e894e5a32c9c063cd48455. The attached diff.patch
fixes this issue for me. (IIUC internal headers generated by Bison are
usually excluded from such checks so I also excluded gramparse.h and
jsonpath_internal.h...)

[1]: https://github.com/postgres/postgres/commit/e177da5c87a10abac97c028bfb427bafb7353aa2
https://github.com/postgres/postgres/commit/e177da5c87a10abac97c028bfb427bafb7353aa2
[2]: https://github.com/postgres/postgres/commit/dac048f71ebbcf2f980d280711f8ff8001331c5d
https://github.com/postgres/postgres/commit/dac048f71ebbcf2f980d280711f8ff8001331c5d
[3]: https://github.com/postgres/postgres/commit/ecaf7c5df54f7fa9df2fdc7225d2bb4e283f0081
https://github.com/postgres/postgres/commit/ecaf7c5df54f7fa9df2fdc7225d2bb4e283f0081
[4]: https://github.com/postgres/postgres/commit/721856ff24b3722ce8e894e5a32c9c063cd48455
https://github.com/postgres/postgres/commit/721856ff24b3722ce8e894e5a32c9c063cd48455

--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachments:

diff.patchtext/x-diff; name=diff.patchDownload
diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck
index 4e09c4686b30b973fd4ca2443d04b5228e904f4b..e77979c97ebd09dff18cb98090254bd3724de0ff 100755
--- a/src/tools/pginclude/cpluspluscheck
+++ b/src/tools/pginclude/cpluspluscheck
@@ -121,14 +121,15 @@ do
 
 	# We can't make these Bison output files compilable standalone
 	# without using "%code require", which old Bison versions lack.
-	# parser/gram.h will be included by parser/gramparse.h anyway.
 	test "$f" = contrib/cube/cubeparse.h && continue
 	test "$f" = contrib/seg/segparse.h && continue
 	test "$f" = src/backend/bootstrap/bootparse.h && continue
 	test "$f" = src/backend/parser/gram.h && continue
+	test "$f" = src/backend/parser/gramparse.h && continue
 	test "$f" = src/backend/replication/repl_gram.h && continue
 	test "$f" = src/backend/replication/syncrep_gram.h && continue
 	test "$f" = src/backend/utils/adt/jsonpath_gram.h && continue
+	test "$f" = src/backend/utils/adt/jsonpath_internal.h && continue
 	test "$f" = src/bin/pgbench/exprparse.h && continue
 	test "$f" = src/pl/plpgsql/src/pl_gram.h && continue
 	test "$f" = src/interfaces/ecpg/preproc/preproc.h && continue
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index 8dee1b56709d06a9ef1f5e80b956a079fa8a1e6a..30e0e47684e571e8c86459654a0ad37326d798e0 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -116,14 +116,15 @@ do
 
 	# We can't make these Bison output files compilable standalone
 	# without using "%code require", which old Bison versions lack.
-	# parser/gram.h will be included by parser/gramparse.h anyway.
 	test "$f" = contrib/cube/cubeparse.h && continue
 	test "$f" = contrib/seg/segparse.h && continue
 	test "$f" = src/backend/bootstrap/bootparse.h && continue
 	test "$f" = src/backend/parser/gram.h && continue
+	test "$f" = src/backend/parser/gramparse.h && continue
 	test "$f" = src/backend/replication/repl_gram.h && continue
 	test "$f" = src/backend/replication/syncrep_gram.h && continue
 	test "$f" = src/backend/utils/adt/jsonpath_gram.h && continue
+	test "$f" = src/backend/utils/adt/jsonpath_internal.h && continue
 	test "$f" = src/bin/pgbench/exprparse.h && continue
 	test "$f" = src/pl/plpgsql/src/pl_gram.h && continue
 	test "$f" = src/interfaces/ecpg/preproc/preproc.h && continue
#2John Naylor
johncnaylorls@gmail.com
In reply to: Marina Polyakova (#1)
1 attachment(s)
Re: cpluspluscheck/headerscheck require build in REL_16_STABLE

On Fri, Apr 12, 2024 at 11:51 PM Marina Polyakova
<m.polyakova@postgrespro.ru> wrote:

Hello, hackers!

When running cpluspluscheck/headerscheck on REL_16_STABLE [1] I found
that everything was ok only if it was preceded by a build and make
maintainer-clean was not used:

I can reproduce this.

In the other branches everything is fine: these problems begin with
commits [2] (jsonpath_gram.h) and [3] (gram.h) and in the master branch
there're no such problems after commit [4]. The attached diff.patch
fixes this issue for me. (IIUC internal headers generated by Bison are
usually excluded from such checks so I also excluded gramparse.h and
jsonpath_internal.h...)

I'm not in favor of this patch because these files build fine on
master, so there is no good reason to exclude them. We should arrange
so that they build fine on PG16 as well. The problem is, not all the
required headers are generated when invoking `make headerscheck`. The
attached patch brings in some Makefile rules from master to make this
work. Does this fix it for you?

Attachments:

v1-build-headers-for-pg16-headerscheck.patchtext/x-patch; charset=US-ASCII; name=v1-build-headers-for-pg16-headerscheck.patchDownload
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 3c42003175..82cae98a44 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -160,7 +160,7 @@ submake-utils-headers:
 
 .PHONY: generated-headers
 
-generated-headers: $(top_builddir)/src/include/storage/lwlocknames.h submake-catalog-headers submake-nodes-headers submake-utils-headers
+generated-headers: $(top_builddir)/src/include/storage/lwlocknames.h submake-catalog-headers submake-nodes-headers submake-utils-headers parser/gram.h
 
 $(top_builddir)/src/include/storage/lwlocknames.h: storage/lmgr/lwlocknames.h
 	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
diff --git a/src/backend/utils/Makefile b/src/backend/utils/Makefile
index deb901609f..4299735cb6 100644
--- a/src/backend/utils/Makefile
+++ b/src/backend/utils/Makefile
@@ -38,9 +38,12 @@ all: distprep probes.h generated-header-symlinks
 
 distprep: fmgr-stamp errcodes.h
 
-.PHONY: generated-header-symlinks
+.PHONY: generated-header-symlinks submake-adt-headers
 
-generated-header-symlinks: $(top_builddir)/src/include/utils/header-stamp $(top_builddir)/src/include/utils/probes.h
+generated-header-symlinks: $(top_builddir)/src/include/utils/header-stamp $(top_builddir)/src/include/utils/probes.h submake-adt-headers
+
+submake-adt-headers:
+	$(MAKE) -C adt jsonpath_gram.h
 
 $(SUBDIRS:%=%-recursive): fmgr-stamp errcodes.h
 
#3Marina Polyakova
m.polyakova@postgrespro.ru
In reply to: John Naylor (#2)
Re: cpluspluscheck/headerscheck require build in REL_16_STABLE

On 2024-04-13 08:40, John Naylor wrote:

On Fri, Apr 12, 2024 at 11:51 PM Marina Polyakova
<m.polyakova@postgrespro.ru> wrote:

...

In the other branches everything is fine: these problems begin with
commits [2] (jsonpath_gram.h) and [3] (gram.h) and in the master
branch
there're no such problems after commit [4]. The attached diff.patch
fixes this issue for me. (IIUC internal headers generated by Bison are
usually excluded from such checks so I also excluded gramparse.h and
jsonpath_internal.h...)

I'm not in favor of this patch because these files build fine on
master, so there is no good reason to exclude them. We should arrange
so that they build fine on PG16 as well. The problem is, not all the
required headers are generated when invoking `make headerscheck`. The
attached patch brings in some Makefile rules from master to make this
work. Does this fix it for you?

Everything seems to work with this patch, thank you!

--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

#4John Naylor
johncnaylorls@gmail.com
In reply to: Marina Polyakova (#3)
Re: cpluspluscheck/headerscheck require build in REL_16_STABLE

On Mon, Apr 15, 2024 at 9:20 PM Marina Polyakova
<m.polyakova@postgrespro.ru> wrote:

On 2024-04-13 08:40, John Naylor wrote:

so that they build fine on PG16 as well. The problem is, not all the
required headers are generated when invoking `make headerscheck`. The
attached patch brings in some Makefile rules from master to make this
work. Does this fix it for you?

Everything seems to work with this patch, thank you!

Glad to hear it -- I'll push next week when I get back from vacation,
unless there are objections.

#5John Naylor
johncnaylorls@gmail.com
In reply to: John Naylor (#4)
Re: cpluspluscheck/headerscheck require build in REL_16_STABLE

On Wed, Apr 17, 2024 at 7:21 AM John Naylor <johncnaylorls@gmail.com> wrote:

On Mon, Apr 15, 2024 at 9:20 PM Marina Polyakova
<m.polyakova@postgrespro.ru> wrote:

Everything seems to work with this patch, thank you!

Glad to hear it -- I'll push next week when I get back from vacation,
unless there are objections.

Pushed, thanks for the report!

#6Marina Polyakova
m.polyakova@postgrespro.ru
In reply to: John Naylor (#5)
Re: cpluspluscheck/headerscheck require build in REL_16_STABLE

On 2024-04-27 09:14, John Naylor wrote:

On Wed, Apr 17, 2024 at 7:21 AM John Naylor <johncnaylorls@gmail.com>
wrote:

On Mon, Apr 15, 2024 at 9:20 PM Marina Polyakova
<m.polyakova@postgrespro.ru> wrote:

Everything seems to work with this patch, thank you!

Glad to hear it -- I'll push next week when I get back from vacation,
unless there are objections.

Pushed, thanks for the report!

Thank you again!

--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company