pgsql: Move snowball_create.sql creation into perl file

Re: Andres Freund

Move snowball_create.sql creation into perl file

This is in preparation for building postgres with meson / ninja.

We already have duplicated code for this between the make and msvc
builds. Adding a third copy seems like a bad plan, thus move the generation
into a perl script.

As we don't want to rely on perl being available for builds from tarballs,
generate the file during distprep.

Author: Peter Eisentraut <peter@eisentraut.org>
Author: Andres Freund <andres@anarazel.de>
Discussion: /messages/by-id/5e216522-ba3c-f0e6-7f97-5276d0270029@enterprisedb.com

Hi,

this seems to have broken out-of-tree builds from tarballs:

make -C backend/snowball install
make[3]: Entering directory '/srv/projects/postgresql/debian/16/build/src/backend/snowball'
/bin/mkdir -p '/srv/projects/postgresql/debian/16/build/tmp_install/usr/lib/postgresql/16/lib'
/bin/mkdir -p '/srv/projects/postgresql/debian/16/build/tmp_install/usr/share/postgresql/16' '/srv/projects/postgresql/debian/16/build/tmp_install/usr/share/postgresql/16/tsearch_data'
/usr/bin/install -c -m 755 dict_snowball.so '/srv/projects/postgresql/debian/16/build/tmp_install/usr/lib/postgresql/16/lib/dict_snowball.so'
/usr/bin/install -c -m 644 snowball_create.sql '/srv/projects/postgresql/debian/16/build/tmp_install/usr/share/postgresql/16'
/usr/bin/install: cannot stat 'snowball_create.sql': No such file or directory
make[3]: *** [Makefile:110: install] Error 1

The file is present in src/backend/snowball/ but not in build/src/backend/snowball/:

-rw-r--r-- 1 myon myon 44176 22. Mai 21:20 src/backend/snowball/snowball_create.sql

Christoph

Re: To Andres Freund

this seems to have broken out-of-tree builds from tarballs:

/usr/bin/install -c -m 644 snowball_create.sql '/srv/projects/postgresql/debian/16/build/tmp_install/usr/share/postgresql/16'
/usr/bin/install: cannot stat 'snowball_create.sql': No such file or directory

Fortunately, there is an easy workaround, just delete
src/backend/snowball/snowball_create.sql before building, it will then
be recreated in the proper build directory.

Christoph

Christoph Berg <myon@debian.org> writes:

this seems to have broken out-of-tree builds from tarballs:

/usr/bin/install -c -m 644 snowball_create.sql '/srv/projects/postgresql/debian/16/build/tmp_install/usr/share/postgresql/16'
/usr/bin/install: cannot stat 'snowball_create.sql': No such file or directory

I think the attached will do for a proper fix. I'm not inclined
to re-wrap just for this.

regards, tom lane

Re: Tom Lane

I think the attached will do for a proper fix. I'm not inclined
to re-wrap just for this.

Sure, I just posted it here in case others run into the same problem.

Thanks!

Christoph

Hi,

On 2023-05-23 10:46:30 -0400, Tom Lane wrote:

Christoph Berg <myon@debian.org> writes:

this seems to have broken out-of-tree builds from tarballs:

/usr/bin/install -c -m 644 snowball_create.sql '/srv/projects/postgresql/debian/16/build/tmp_install/usr/share/postgresql/16'
/usr/bin/install: cannot stat 'snowball_create.sql': No such file or directory

I think the attached will do for a proper fix.

Thanks.

I'm not inclined to re-wrap just for this.

Agreed.

I wonder if we should add a CI task to test creating a tarball and building
from it, both inside the source directory and as a vpath build? We rebuild for
both gcc and clang, each with assertions and without, to check if there are
warnings. We could probably just switch to building from the tarball for some
of those.

I guess I need to go and check how long the "release" tarball generation
takes...

Greetings,

Andres Freund

[ dropping -packagers ]

Andres Freund <andres@anarazel.de> writes:

I guess I need to go and check how long the "release" tarball generation
takes...

It's quick except for the documentation-generating steps. Maybe
we could test that part only once?

regards, tom lane

Hi,

On 2023-05-23 14:51:03 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

I guess I need to go and check how long the "release" tarball generation
takes...

It's quick except for the documentation-generating steps. Maybe
we could test that part only once?

First thing I noticed that 'make dist' doesn't work in a vpath, failing in a
somewhat obscure way (likely because in a vpath build the the copy from the
source dir doesn't include GNUMakefile). Do we expect it to work?

Besides docs, the slowest part appears to be gzip --best and then bzip2, as
those runs serially and takes 11 and 13 seconds respectively here...

The first thing I tried was:
make -j8 dist GZIP=pigz BZIP2=pbzip2

unfortunately that results in

pigz: abort: cannot provide files in GZIP environment variable

echo GZIP=pigz >> src/Makefile.custom
echo BZIP2=pbzip2 >> src/Makefile.custom

reduces that to

real 1m6.472s
user 1m28.316s
sys 0m5.340s

real 0m54.811s
user 1m42.078s
sys 0m6.183s

still not great...

OTOH, we currently already build the docs as part of the CompilerWarnings
test. I don't think there's a reason to test that twice?

For me make distcheck currently fails:

In file included from ../../src/include/postgres.h:46,
from hashfn.c:24:
../../src/include/utils/elog.h:79:10: fatal error: utils/errcodes.h: No such file or directory
79 | #include "utils/errcodes.h"
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [<builtin>: hashfn.o] Error 1

at first I thought it was due to my use of -j8 - but it doesn't even work
without that.

That's due to MAKELEVEL:

submake-generated-headers:
ifndef NO_GENERATED_HEADERS
ifeq ($(MAKELEVEL),0)
$(MAKE) -C $(top_builddir)/src/backend generated-headers
endif
endif

So the distcheck target needs to reset MAKELEVEL=0 - unless somebody has a
better idea?

Separately, it's somewhat confusing that we include errcodes.h etc in
src/backend/utils, rather than its final location, in src/include/utils. It
works, even without perl, because copying the file doesn't require perl, it's
just generating it...

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

First thing I noticed that 'make dist' doesn't work in a vpath, failing in a
somewhat obscure way (likely because in a vpath build the the copy from the
source dir doesn't include GNUMakefile). Do we expect it to work?

Don't see how it could possibly be useful in a vpath, because you'd have
the real source files and the generated files in different trees.

regards, tom lane

Re: Andres Freund

That's due to MAKELEVEL:

submake-generated-headers:
ifndef NO_GENERATED_HEADERS
ifeq ($(MAKELEVEL),0)
$(MAKE) -C $(top_builddir)/src/backend generated-headers
endif
endif

So the distcheck target needs to reset MAKELEVEL=0 - unless somebody has a
better idea?

Fwiw, I've had that problem as well in the Debian packages where
debian/rules is already a Makefile and calling $(MAKE) from there
trips up that logic. The workaround I used is:

override_dh_auto_build-arch:
# set MAKELEVEL to 0 to force building submake-generated-headers in src/Makefile.global(.in)
MAKELEVEL=0 $(MAKE) -C build/src all

...
override_dh_auto_test-arch:
ifeq (, $(findstring nocheck, $(DEB_BUILD_OPTIONS)))
# when tests fail, print newest log files
# initdb doesn't like LANG and LC_ALL to contradict, unset LANG and LC_CTYPE here
# temp-install wants to be invoked from a top-level make, unset MAKELEVEL here
# tell pg_upgrade to create its sockets in /tmp to avoid too long paths
unset LANG LC_CTYPE MAKELEVEL; ulimit -c unlimited; \
if ! make -C build check-world \
$(TEMP_CONFIG) \
PGSOCKETDIR="/tmp" \
PG_TEST_EXTRA='ssl' \
PROVE_FLAGS="--verbose"; \
...

(Just mentioning this, not asking it to be changed.)

Re: Tom Lane

Andres Freund <andres@anarazel.de> writes:

First thing I noticed that 'make dist' doesn't work in a vpath, failing in a
somewhat obscure way (likely because in a vpath build the the copy from the
source dir doesn't include GNUMakefile). Do we expect it to work?

Don't see how it could possibly be useful in a vpath, because you'd have
the real source files and the generated files in different trees.

I don't think "make dist" is generally expected to work in vpath
builds, that's probably one indirection layer too much. (The "make
distcheck" rule generated by automake tests vpath builds, though.)

Christoph

On 24.05.23 23:24, Andres Freund wrote:

First thing I noticed that 'make dist' doesn't work in a vpath, failing in a
somewhat obscure way (likely because in a vpath build the the copy from the
source dir doesn't include GNUMakefile). Do we expect it to work?

I don't think so.

Separately, it's somewhat confusing that we include errcodes.h etc in
src/backend/utils, rather than its final location, in src/include/utils. It
works, even without perl, because copying the file doesn't require perl, it's
just generating it...

The "copying" is actually a symlink, right? I don't think we want to
ship symlinks in the tarball?

Hi,

On 2023-05-26 09:02:33 +0200, Peter Eisentraut wrote:

On 24.05.23 23:24, Andres Freund wrote:

First thing I noticed that 'make dist' doesn't work in a vpath, failing in a
somewhat obscure way (likely because in a vpath build the the copy from the
source dir doesn't include GNUMakefile). Do we expect it to work?

I don't think so.

Maybe we should just error out in that case, instead of failing in an obscure
way down the line?

Separately, it's somewhat confusing that we include errcodes.h etc in
src/backend/utils, rather than its final location, in src/include/utils. It
works, even without perl, because copying the file doesn't require perl, it's
just generating it...

The "copying" is actually a symlink, right? I don't think we want to ship
symlinks in the tarball?

Fair point - still seems we should just create the files in the right
directory instead of doing it in the wrong place and then creating symlinks to
make them accessible...

Greetings,

Andres Freund

On 27.05.23 14:47, Andres Freund wrote:

Separately, it's somewhat confusing that we include errcodes.h etc in
src/backend/utils, rather than its final location, in src/include/utils. It
works, even without perl, because copying the file doesn't require perl, it's
just generating it...

The "copying" is actually a symlink, right? I don't think we want to ship
symlinks in the tarball?

Fair point - still seems we should just create the files in the right
directory instead of doing it in the wrong place and then creating symlinks to
make them accessible...

Right. I think the reason this was set up this way is that with make it
is generally dubious to create target files outside of the current
directory.