Обсуждение: Re: 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: https://postgr.es/m/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
diff --git a/src/backend/snowball/Makefile b/src/backend/snowball/Makefile
index 29076371db..4bebfa0250 100644
--- a/src/backend/snowball/Makefile
+++ b/src/backend/snowball/Makefile
@@ -106,10 +106,13 @@ $(SQLSCRIPT): snowball_create.pl snowball_func.sql.in snowball.sql.in
distprep: $(SQLSCRIPT)
-install: all installdirs install-lib
- $(INSTALL_DATA) $(SQLSCRIPT) '$(DESTDIR)$(datadir)'
+install: all installdirs install-lib install-script
$(INSTALL_DATA) $(addprefix $(srcdir)/stopwords/,$(stop_files)) '$(DESTDIR)$(datadir)/$(DICTDIR)'
+# $(SQLSCRIPT) might be in the srcdir or the build dir
+install-script: $(SQLSCRIPT)
+ $(INSTALL_DATA) $< '$(DESTDIR)$(datadir)'
+
installdirs: installdirs-lib
$(MKDIR_P) '$(DESTDIR)$(datadir)' '$(DESTDIR)$(datadir)/$(DICTDIR)'
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.