Re: Remove distprep
On 2023-11-21 Tu 13:23, Tom Lane wrote: Alvaro Herrera writes: Hmm, do we still need to have README.git as a separate file from README? Also, looking at README, I see it refers to the INSTALL file in the root, but that doesn't exist. "make -C doc/src/sgml INSTALL" creates it, but it's not copied to the root directory. Do we need some fixup for that? Yeah, we clearly need to rethink this area if the plan is that tarballs will be pristine git pulls. I think we want just README at the top level, and I propose we give up on the text INSTALL file altogether (thereby removing a documentation build gotcha that catches people every so often). I propose that in 2023 it ought to be sufficient for the README file to point at build instructions on the web. +1 cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Remove distprep
Alvaro Herrera writes: > Hmm, do we still need to have README.git as a separate file from README? > Also, looking at README, I see it refers to the INSTALL file in the > root, but that doesn't exist. "make -C doc/src/sgml INSTALL" creates > it, but it's not copied to the root directory. Do we need some fixup > for that? Yeah, we clearly need to rethink this area if the plan is that tarballs will be pristine git pulls. I think we want just README at the top level, and I propose we give up on the text INSTALL file altogether (thereby removing a documentation build gotcha that catches people every so often). I propose that in 2023 it ought to be sufficient for the README file to point at build instructions on the web. regards, tom lane
Re: Remove distprep
On 2023-Nov-07, Michael Paquier wrote: > On Mon, Nov 06, 2023 at 04:21:40PM +0100, Peter Eisentraut wrote: > > done > > Nice to see 721856ff24b3 in, thanks! Hmm, do we still need to have README.git as a separate file from README? Also, looking at README, I see it refers to the INSTALL file in the root, but that doesn't exist. "make -C doc/src/sgml INSTALL" creates it, but it's not copied to the root directory. Do we need some fixup for that? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Learn about compilers. Then everything looks like either a compiler or a database, and now you have two problems but one of them is fun." https://twitter.com/thingskatedid/status/1456027786158776329
Re: Remove distprep
On Mon, Nov 06, 2023 at 04:21:40PM +0100, Peter Eisentraut wrote: > done Nice to see 721856ff24b3 in, thanks! -- Michael signature.asc Description: PGP signature
Re: Remove distprep
On 02.11.23 23:34, Andres Freund wrote: On 2023-11-01 16:39:24 -0400, Peter Eisentraut wrote: OTOH, it seems somewhat unlikely that maintainer-clean is utilized much in extensions. I see it in things like postgis, but that has it's own configure etc, even though it also invokes pgxs. I thought about this. I don't think this is something that any extension would use. If they care about the distinction between distclean and maintainer-clean, are they also doing their own distprep and dist? Seems unlikely. I mean, if some extension is actually affected, I'm happy to accommodate, but we can deal with that when we learn about it. Moreover, if we are moving forward in this direction, we would presumably also like the extensions to get rid of their distprep step. So I think we are ready to move ahead with this patch. There have been some light complaints earlier in this thread that people wanted to keep some way to clean only some of the files. But there hasn't been any concrete follow-up on that, as far as I can see, so I don't know what to do about that. +1, let's do this. We can add dedicated target for more specific cases later if we decide we want that. done
Re: Remove distprep
On 2023-11-01 16:39:24 -0400, Peter Eisentraut wrote: > > OTOH, it seems somewhat unlikely that maintainer-clean is utilized much in > > extensions. I see it in things like postgis, but that has it's own configure > > etc, even though it also invokes pgxs. > > I thought about this. I don't think this is something that any extension > would use. If they care about the distinction between distclean and > maintainer-clean, are they also doing their own distprep and dist? Seems > unlikely. I mean, if some extension is actually affected, I'm happy to > accommodate, but we can deal with that when we learn about it. Moreover, if > we are moving forward in this direction, we would presumably also like the > extensions to get rid of their distprep step. > > So I think we are ready to move ahead with this patch. There have been some > light complaints earlier in this thread that people wanted to keep some way > to clean only some of the files. But there hasn't been any concrete > follow-up on that, as far as I can see, so I don't know what to do about > that. +1, let's do this. We can add dedicated target for more specific cases later if we decide we want that.
Re: Remove distprep
On 09.10.23 17:14, Andres Freund wrote: It kinda works, but I'm not sure how well. Because the aliasing happens in Makefile.global, we won't know about the "original" maintainer-clean target once recursing into a subdir. That's perhaps OK, because extensions likely won't utilize subdirectories? But I'm not sure. I know that some people build postgres extensions by adding them to contrib/, in those cases it won't work. OTOH, it seems somewhat unlikely that maintainer-clean is utilized much in extensions. I see it in things like postgis, but that has it's own configure etc, even though it also invokes pgxs. I thought about this. I don't think this is something that any extension would use. If they care about the distinction between distclean and maintainer-clean, are they also doing their own distprep and dist? Seems unlikely. I mean, if some extension is actually affected, I'm happy to accommodate, but we can deal with that when we learn about it. Moreover, if we are moving forward in this direction, we would presumably also like the extensions to get rid of their distprep step. So I think we are ready to move ahead with this patch. There have been some light complaints earlier in this thread that people wanted to keep some way to clean only some of the files. But there hasn't been any concrete follow-up on that, as far as I can see, so I don't know what to do about that.
Re: Remove distprep
Hi, On 2023-10-09 12:16:23 +0200, Peter Eisentraut wrote: > On 06.10.23 20:50, Andres Freund wrote: > > The only thing I wonder is whether we ought to keep a maintainer-clean > > target (as an alias to distclean), so that extensions that added things > > to maintainer-clean continue to work. > > The patch does do that. It kinda works, but I'm not sure how well. Because the aliasing happens in Makefile.global, we won't know about the "original" maintainer-clean target once recursing into a subdir. That's perhaps OK, because extensions likely won't utilize subdirectories? But I'm not sure. I know that some people build postgres extensions by adding them to contrib/, in those cases it won't work. OTOH, it seems somewhat unlikely that maintainer-clean is utilized much in extensions. I see it in things like postgis, but that has it's own configure etc, even though it also invokes pgxs. I wish we had an easy way of a) downloading most working open-source extensions b) building many of those Greetings, Andres Freund
Re: Remove distprep
On 06.10.23 20:50, Andres Freund wrote: The only thing I wonder is whether we ought to keep a maintainer-clean target (as an alias to distclean), so that extensions that added things to maintainer-clean continue to work. The patch does do that.
Re: Remove distprep
Hi, On 2023-10-05 17:46:46 +0200, Peter Eisentraut wrote: > The attached updated patch is also split up like Andres suggested nearby. Thanks. > (Not sure if it would be good to commit it that way, but it's easier to look > at for now for sure.) I'd push together, but I think the split is useful when looking later as well. I played around with these for a bit without finding an issue. The only thing I wonder is whether we ought to keep a maintainer-clean target (as an alias to distclean), so that extensions that added things to maintainer-clean continue to work. Greetings, Andres Freund
Re: Remove distprep
On Fri, Oct 06, 2023 at 08:38:31AM +0200, Peter Eisentraut wrote: > Yes, something like that. Some people wanted a tarball of just the HTML > docs for download. Similar to the PDFs currently, I suppose. I suspected so. I've marked the patch as RfC for now. -- Michael signature.asc Description: PGP signature
Re: Remove distprep
On 06.10.23 04:00, Michael Paquier wrote: That's not really something for this patch, but I got to ask. What's the final plan for the documentation when it comes to releases? A second tarball separated from the git-only tarball that includes all that and the HTML docs, generated with a new "doc-only" set of meson commands? Yes, something like that. Some people wanted a tarball of just the HTML docs for download. Similar to the PDFs currently, I suppose.
Re: Remove distprep
On Thu, Oct 05, 2023 at 05:46:46PM +0200, Peter Eisentraut wrote: > Ok, I think I found a better way to address this. It requires keeping a > subset of the old distprep target in src/backend/Makefile for use by nls.mk. > I have checked that the above sequence now works, and that the generated > .pot files are identical to before this patch. generated-parser-sources looks like an elegant split. > (Not sure if it would be good to commit it that way, but it's easier to look > at for now for sure.) Not sure. I'd be OK if the patch set is committed into two pieces as well. I guess that's up to how you feel about that at the end ;) While looking at the last references of the distribution tarball, this came out: # This allows removing some files from the distribution tarballs while # keeping the dependencies satisfied. .SECONDARY: $(GENERATED_SGML) .SECONDARY: postgres-full.xml .SECONDARY: INSTALL.html INSTALL.xml .SECONDARY: postgres-A4.fo postgres-US.fo That's not really something for this patch, but I got to ask. What's the final plan for the documentation when it comes to releases? A second tarball separated from the git-only tarball that includes all that and the HTML docs, generated with a new "doc-only" set of meson commands? -- Michael signature.asc Description: PGP signature
Re: Remove distprep
ut is pre-generated.)" >&2;}
-
- FLEX=
+ as_fn_error $? "flex not found" "$LINENO" 5
else
FLEX=$pgac_cv_path_flex
pgac_flex_version=`$FLEX --version 2>/dev/null`
@@ -10329,27 +10298,14 @@ $as_echo "$as_me: using perl $pgac_perl_version" >&6;}
if echo "$pgac_perl_version" | sed 's/[.a-z_]/ /g' | \
$AWK '{ if ($1 == 5 && ($2 >= 14)) exit 1; else exit 0;}'
then
-{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING:
-*** The installed version of Perl, $PERL, is too old to use with PostgreSQL.
-*** Perl version 5.14 or later is required, but this is $pgac_perl_version."
>&5
-$as_echo "$as_me: WARNING:
+as_fn_error $? "
*** The installed version of Perl, $PERL, is too old to use with PostgreSQL.
-*** Perl version 5.14 or later is required, but this is $pgac_perl_version."
>&2;}
-PERL=""
+*** Perl version 5.14 or later is required, but this is $pgac_perl_version."
"$LINENO" 5
fi
fi
if test -z "$PERL"; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING:
-*** Without Perl you will not be able to build PostgreSQL from Git.
-*** You can obtain Perl from any CPAN mirror site.
-*** (If you are using the official distribution of PostgreSQL then you do not
-*** need to worry about this, because the Perl output is pre-generated.)" >&5
-$as_echo "$as_me: WARNING:
-*** Without Perl you will not be able to build PostgreSQL from Git.
-*** You can obtain Perl from any CPAN mirror site.
-*** (If you are using the official distribution of PostgreSQL then you do not
-*** need to worry about this, because the Perl output is pre-generated.)" >&2;}
+ as_fn_error $? "Perl not found" "$LINENO" 5
fi
if test "$with_perl" = yes; then
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index f4b1f81189..468759583c 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -102,6 +102,41 @@ Requirements
+
+
+
+ flex
+
+
+ lex
+
+
+ bison
+
+
+ yacc
+
+
+ Flex 2.5.35 or later and
+ Bison 2.3 or later are required. Other
+ lex and yacc
+ programs cannot be used.
+
+
+
+
+
+
+ perl
+
+
+ Perl 5.14 or later is needed during the build
+ process and to run some test suites. (This requirement is separate from
+ the requirements for building PL/Perl; see
+ below.)
+
+
+
@@ -315,51 +350,6 @@ Requirements
-
- If you are building from a Git tree instead of
- using a released source package, or if you want to do server development,
- you also need the following packages:
-
-
-
-
-
- flex
-
-
- lex
-
-
- bison
-
-
- yacc
-
-
- Flex and Bison
- are needed to build from a Git checkout, or if you changed the actual
- scanner and parser definition files. If you need them, be sure
- to get Flex 2.5.35 or later and
- Bison 2.3 or later. Other
lex
- and yacc programs cannot be used.
-
-
-
-
-
- perl
-
-
- Perl 5.14 or later is needed to build from a
Git checkout,
- or if you changed the input files for any of the build steps that
- use Perl scripts. If building on Windows you will need
- Perl in any case.
Perl is
- also required to run some test suites.
-
-
-
-
-
If you need to get a GNU package, you can find
it at your local GNU mirror site (see The Source Code Repository
has some discussion on working with Git.
-
- Note that building PostgreSQL from the source
- repository requires reasonably up-to-date versions of
bison,
- flex, and Perl.
- These tools are not needed to build from a distribution tarball, because
- the files generated with these tools are included in the tarball.
- Other tool requirements
- are the same as shown in .
-
-
Getting the Source via Git
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 7b66590801..985b3ea62b 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -319,12 +319,8 @@ X = @EXEEXT@
# Perl
-ifneq (@PERL@,)
-# quoted to protect pathname with spaces
-PERL = '@PERL@'
-else
-PERL = $(missing) perl
-endif
+# quoted to protect pathname with spaces
+PERL = '@PERL@'
perl_archlibexp= @perl_archlibexp@
perl_privlibexp= @perl_privlibexp@
perl_includespec = @perl_includespec@
@@ -777,21 +773,13 @@ TAS = @TAS@
# Global targets and rules
%.c: %.l
-ifdef FLEX
$(FLEX) $(if $(FLEX_NO_BACKUP),-b) $(FLEXFLAGS) -o'$@' $<
@$(
Re: Remove distprep
Hi, On 2023-08-23 12:46:45 +0200, Peter Eisentraut wrote: > Subject: [PATCH v4] Remove distprep > > A PostgreSQL release tarball contains a number of prebuilt files, in > particular files produced by bison, flex, perl, and well as html and > man documentation. We have done this consistent with established > practice at the time to not require these tools for building from a > tarball. Some of these tools were hard to get, or get the right > version of, from time to time, and shipping the prebuilt output was a > convenience to users. > > Now this has at least two problems: > > One, we have to make the build system(s) work in two modes: Building > from a git checkout and building from a tarball. This is pretty > complicated, but it works so far for autoconf/make. It does not > currently work for meson; you can currently only build with meson from > a git checkout. Making meson builds work from a tarball seems very > difficult or impossible. One particular problem is that since meson > requires a separate build directory, we cannot make the build update > files like gram.h in the source tree. So if you were to build from a > tarball and update gram.y, you will have a gram.h in the source tree > and one in the build tree, but the way things work is that the > compiler will always use the one in the source tree. So you cannot, > for example, make any gram.y changes when building from a tarball. > This seems impossible to fix in a non-horrible way. I think it might be possible to fix in a non-horrible way, just that the effort doing so could be much better spent on other things. It's maybe also worth mentioning that this does *not* work reliably with vpath make builds either... > The make maintainer-clean target, whose job it is to remove the > prebuilt files in addition to what make distclean does, is now just an > alias to make distprep. (In practice, it is probably obsolete given > that git clean is available.) FWIW, I find a "full clean" target useful to be sure that we don't produce "untracked" build products. Do a full build, then run "full clean", then see what remains. > 88 files changed, 169 insertions(+), 409 deletions(-) It might be worthwhile to split this into a bit smaller chunks, e.g. depending on perl, bison, flex, and then separately the various makefile bits that are all over the tree. Greetings, Andres Freund
Re: Remove distprep
On Wed, Aug 23, 2023 at 12:46:45PM +0200, Peter Eisentraut wrote: > Apparently, the headerscheck and cpluspluscheck targets still didn't work > right in the Cirrus jobs. Here is an updated patch to address that. This > is also rebased over some recent changes that affected this patch (generated > wait events stuff). -gettext-files: distprep +gettext-files: postgres This bit in src/backend/nls.mk does not seem right to me. The following sequence works on HEAD, but breaks with the patch because the files that should be automatically generated from the perl scripts aren't anymore: $ ./configure ... $ make -C src/backend/ gettext-files [...] In file included from ../../../../src/include/postgres.h:46, from brin.c:16: ../../../../src/include/utils/elog.h:79:10: fatal error: utils/errcodes.h: No such file or directory 79 | #include "utils/errcodes.h" # Technically, this should depend on Makefile.global, but then # version.sgml would need to be rebuilt after every configure run, # even in distribution tarballs. So this is cheating a bit, but it There is this comment in doc/src/sgml/Makefile. Does it still apply? Note that building PostgreSQL from the source repository requires reasonably up-to-date versions of bison, flex, and Perl. These tools are not needed to build from a distribution tarball, because the files generated with these tools are included in the tarball. Other tool requirements This paragraph exists in sourcerepo.sgml, but it should be updated, I guess, because now these three binaries would be required when building from a tarball. # specparse.c and specscanner.c are in the distribution tarball, # so do not clean them here This comment in src/test/isolation/Makefile should be removed. -- Michael signature.asc Description: PGP signature
Re: Remove distprep
On 14.07.23 10:56, Peter Eisentraut wrote: On 14.07.23 09:54, Peter Eisentraut wrote: diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck index 4e09c4686b..287395887c 100755 --- a/src/tools/pginclude/cpluspluscheck +++ b/src/tools/pginclude/cpluspluscheck @@ -134,6 +134,9 @@ do test "$f" = src/interfaces/ecpg/preproc/preproc.h && continue test "$f" = src/test/isolation/specparse.h && continue + # FIXME + test "$f" = src/backend/utils/adt/jsonpath_internal.h && continue + # ppport.h is not under our control, so we can't make it standalone. test "$f" = src/pl/plperl/ppport.h && continue Hm, what's that about? Don't remember ... ;-) I removed this. Ah, there was a reason. The headerscheck and cpluspluscheck targets need jsonpath_gram.h to be built first. Which previously happened indirectly somehow? I have fixed this in the new patch version. I also fixed the issue that Álvaro reported nearby. Apparently, the headerscheck and cpluspluscheck targets still didn't work right in the Cirrus jobs. Here is an updated patch to address that. This is also rebased over some recent changes that affected this patch (generated wait events stuff). From c386175ab4194aa2b17a8374f73a9de4aa0edb56 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 23 Aug 2023 12:15:42 +0200 Subject: [PATCH v4] Remove distprep A PostgreSQL release tarball contains a number of prebuilt files, in particular files produced by bison, flex, perl, and well as html and man documentation. We have done this consistent with established practice at the time to not require these tools for building from a tarball. Some of these tools were hard to get, or get the right version of, from time to time, and shipping the prebuilt output was a convenience to users. Now this has at least two problems: One, we have to make the build system(s) work in two modes: Building from a git checkout and building from a tarball. This is pretty complicated, but it works so far for autoconf/make. It does not currently work for meson; you can currently only build with meson from a git checkout. Making meson builds work from a tarball seems very difficult or impossible. One particular problem is that since meson requires a separate build directory, we cannot make the build update files like gram.h in the source tree. So if you were to build from a tarball and update gram.y, you will have a gram.h in the source tree and one in the build tree, but the way things work is that the compiler will always use the one in the source tree. So you cannot, for example, make any gram.y changes when building from a tarball. This seems impossible to fix in a non-horrible way. Second, there is increased interest nowadays in precisely tracking the origin of software. We can reasonably track contributions into the git tree, and users can reasonably track the path from a tarball to packages and downloads and installs. But what happens between the git tree and the tarball is obscure and in some cases non-reproducible. The solution for both of these issues is to get rid of the step that adds prebuilt files to the tarball. The tarball now only contains what is in the git tree (*). Getting the additional build dependencies is no longer a problem nowadays, and the complications to keep these dual build modes working are significant. And of course we want to get the meson build system working universally. This commit removes the make distprep target altogether. The make dist target continues to do its job, it just doesn't call distprep anymore. (*) - The tarball also contains the INSTALL file that is built at make dist time, but not by distprep. This is unchanged for now. The make maintainer-clean target, whose job it is to remove the prebuilt files in addition to what make distclean does, is now just an alias to make distprep. (In practice, it is probably obsolete given that git clean is available.) The following programs are now hard build requirements in configure (they were already required by meson.build): - bison - flex - perl --- GNUmakefile.in| 6 +- config/perl.m4| 9 +- config/programs.m4| 21 + configure | 62 ++ contrib/cube/Makefile | 7 +- contrib/fuzzystrmatch/Makefile| 9 +- contrib/seg/Makefile | 7 +- doc/Makefile | 2 +- doc/src/Makefile | 2 +- doc/src/sgml/Makefile | 9 +- doc/src/sgml/installation.sgml| 83 --- meson.build | 2 +- src/Makefile | 5 +- src/Makefile.global.in
Reproducibility (Re: Remove distprep)
Re: Michael Paquier > Is reproducibility something you've brought to a separate thread? > FWIW, I'd be interested in improving this area for the in-core code, > if need be. (Not material for this thread, of course). All the "normal" things like C compilation are actually already reproducible. The bit addressed by the mentioned patch is that the compiler flags are recorded for later output by pg_config, and that includes -ffile-prefix-map=/path/to/source=. which does improve C reproducibility, but ironically is itself not reproducible when the source is then compiled in a different directory. The patch simply removes that flag from the information stored. https://salsa.debian.org/postgresql/postgresql/-/blob/15/debian/patches/filter-debug-prefix-map Not sure not much of that would be material for inclusion in PG. This fix made PG 10 reproducible in Debian for about a week. Then LLVM happened :D. The .bc files still record the build path, and so far no one has found a way to prevent that: https://tests.reproducible-builds.org/debian/rb-pkg/unstable/arm64/diffoscope-results/postgresql-15.html Afaict that's the last part to be resolved (but it's been a while since I checked). clang seems to have learned about -ffile-prefix-map= in the meantime, that needs to be tested. Christoph
Re: Remove distprep
On Fri, Aug 18, 2023 at 10:22:47AM +0200, Christoph Berg wrote: > Yes, mostly. Since autoconf had not seen a new release for so long, > everyone started to patch it, and one of the things that Debian and > others added was --runstatedir=DIR. The toolchain is also using it, > it's part of the default set of options used by dh_auto_configure. > > In parallel, the standard debhelper toolchain also started to run > autoreconf by default, so instead of telling dh_auto_configure to omit > --runstatedir, it was really easier to patch configure.ac to remove > the autoconf 2.69 check. Ah, I didn't know this part of the story. Thanks for the insights. > Two of the other patches are touching configure(.ac) anyway to tweak > compiler flags (reproducibility, aarch64 tweaks). Is reproducibility something you've brought to a separate thread? FWIW, I'd be interested in improving this area for the in-core code, if need be. (Not material for this thread, of course). -- Michael signature.asc Description: PGP signature
Re: Remove distprep
Re: Michael Paquier > This one comes down to Debian that patches autoconf with its own set > of options, requiring a new ./configure in the tree, right? Yes, mostly. Since autoconf had not seen a new release for so long, everyone started to patch it, and one of the things that Debian and others added was --runstatedir=DIR. The toolchain is also using it, it's part of the default set of options used by dh_auto_configure. In parallel, the standard debhelper toolchain also started to run autoreconf by default, so instead of telling dh_auto_configure to omit --runstatedir, it was really easier to patch configure.ac to remove the autoconf 2.69 check. Two of the other patches are touching configure(.ac) anyway to tweak compiler flags (reproducibility, aarch64 tweaks). Christoph
Re: Remove distprep
On 2023-08-18 10:11:12 +0900, Michael Paquier wrote: > On Wed, Aug 09, 2023 at 07:38:40AM -0700, Andres Freund wrote: > > On 2023-08-09 16:25:28 +0200, Christoph Berg wrote: > >> Understood, I was just pointing out there are more types of generated > >> files in there. > > > > The situation for configure is somewhat different, due to being maintained > > in > > the repository, rather than just being included in the tarball... > > This one comes down to Debian that patches autoconf with its own set > of options, requiring a new ./configure in the tree, right? I'm not sure what you're really asking here?
Re: Remove distprep
On Wed, Aug 09, 2023 at 07:38:40AM -0700, Andres Freund wrote: > On 2023-08-09 16:25:28 +0200, Christoph Berg wrote: >> Understood, I was just pointing out there are more types of generated >> files in there. > > The situation for configure is somewhat different, due to being maintained in > the repository, rather than just being included in the tarball... This one comes down to Debian that patches autoconf with its own set of options, requiring a new ./configure in the tree, right? -- Michael signature.asc Description: PGP signature
Re: Remove distprep
On 14.07.23 11:48, Tom Lane wrote: Peter Eisentraut writes: Ah, there was a reason. The headerscheck and cpluspluscheck targets need jsonpath_gram.h to be built first. Which previously happened indirectly somehow? I have fixed this in the new patch version. I also fixed the issue that Álvaro reported nearby. Have we yet run this concept past the packagers list? I'm still wondering if they will raise any objection to getting rid of all the prebuilt files. So far there hasn't been any feedback from packagers that would appear to affect the outcome here. Also, I personally don't like the fact that you have removed the distinction between distclean and maintainer-clean. I use distclean-and-reconfigure quite a lot to avoid having to rebuild bison/flex outputs. This patch seems to have destroyed that workflow optimization in return for not much. The distclean target has a standard meaning that is baked into downstream build systems, so it would be pretty disruptive if distclean didn't actually clean everything back down to what was in the distribution tarball. We could add a different clean target that cleans not quite everything, if you can suggest a definition of what that should do.
Re: Remove distprep
Hi, Thanks for sending the -packagers email Peter! On 2023-08-09 16:25:28 +0200, Christoph Berg wrote: > Re: Tom Lane > > Meh ... the fact that it works fine for you doesn't mean it will work > > fine elsewhere. Since we're trying to get out from under maintaining > > the autoconf build system, I don't think it makes sense to open > > ourselves up to having to do more work on it. A policy of benign > > neglect seems appropriate to me. > > Understood, I was just pointing out there are more types of generated > files in there. The situation for configure is somewhat different, due to being maintained in the repository, rather than just being included in the tarball... Greetings, Andres Freund
Re: Remove distprep
Re: Tom Lane > Meh ... the fact that it works fine for you doesn't mean it will work > fine elsewhere. Since we're trying to get out from under maintaining > the autoconf build system, I don't think it makes sense to open > ourselves up to having to do more work on it. A policy of benign > neglect seems appropriate to me. Understood, I was just pointing out there are more types of generated files in there. Christoph
Re: Remove distprep
Christoph Berg writes: > Most notably, we are also rebuilding "configure" using autoconf 2.71 > without issues. Perhaps we can get rid of the 2.69 hardcoding there? Meh ... the fact that it works fine for you doesn't mean it will work fine elsewhere. Since we're trying to get out from under maintaining the autoconf build system, I don't think it makes sense to open ourselves up to having to do more work on it. A policy of benign neglect seems appropriate to me. regards, tom lane
Re: Remove distprep
Re: Tom Lane > Have we yet run this concept past the packagers list? I'm still > wondering if they will raise any objection to getting rid of all > the prebuilt files. No problem for Debian, we are building snapshot releases from plain git already without issues. In fact, there are already some explicit rm/clean rules in the packages to force rebuilding some (most?) of the pre-built files. Most notably, we are also rebuilding "configure" using autoconf 2.71 without issues. Perhaps we can get rid of the 2.69 hardcoding there? Thanks for the heads-up, Christoph
Re: Remove distprep
Peter Eisentraut writes: > Ah, there was a reason. The headerscheck and cpluspluscheck targets > need jsonpath_gram.h to be built first. Which previously happened > indirectly somehow? I have fixed this in the new patch version. I also > fixed the issue that Álvaro reported nearby. Have we yet run this concept past the packagers list? I'm still wondering if they will raise any objection to getting rid of all the prebuilt files. Also, I personally don't like the fact that you have removed the distinction between distclean and maintainer-clean. I use distclean-and-reconfigure quite a lot to avoid having to rebuild bison/flex outputs. This patch seems to have destroyed that workflow optimization in return for not much. regards, tom lane
Re: Remove distprep
On 14.07.23 09:54, Peter Eisentraut wrote: diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck index 4e09c4686b..287395887c 100755 --- a/src/tools/pginclude/cpluspluscheck +++ b/src/tools/pginclude/cpluspluscheck @@ -134,6 +134,9 @@ do test "$f" = src/interfaces/ecpg/preproc/preproc.h && continue test "$f" = src/test/isolation/specparse.h && continue + # FIXME + test "$f" = src/backend/utils/adt/jsonpath_internal.h && continue + # ppport.h is not under our control, so we can't make it standalone. test "$f" = src/pl/plperl/ppport.h && continue Hm, what's that about? Don't remember ... ;-) I removed this. Ah, there was a reason. The headerscheck and cpluspluscheck targets need jsonpath_gram.h to be built first. Which previously happened indirectly somehow? I have fixed this in the new patch version. I also fixed the issue that Álvaro reported nearby. From 5b5e46ea28f4911408eb40936e814b4a05281baa Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 14 Jul 2023 10:53:40 +0200 Subject: [PATCH v3] Remove distprep A PostgreSQL release tarball contains a number of prebuilt files, in particular files produced by bison, flex, perl, and well as html and man documentation. We have done this consistent with established practice at the time to not require these tools for building from a tarball. Some of these tools were hard to get, or get the right version of, from time to time, and shipping the prebuilt output was a convenience to users. Now this has at least two problems: One, we have to make the build system(s) work in two modes: Building from a git checkout and building from a tarball. This is pretty complicated, but it works so far for autoconf/make. It does not currently work for meson; you can currently only build with meson from a git checkout. Making meson builds work from a tarball seems very difficult or impossible. One particular problem is that since meson requires a separate build directory, we cannot make the build update files like gram.h in the source tree. So if you were to build from a tarball and update gram.y, you will have a gram.h in the source tree and one in the build tree, but the way things work is that the compiler will always use the one in the source tree. So you cannot, for example, make any gram.y changes when building from a tarball. This seems impossible to fix in a non-horrible way. Second, there is increased interest nowadays in precisely tracking the origin of software. We can reasonably track contributions into the git tree, and users can reasonably track the path from a tarball to packages and downloads and installs. But what happens between the git tree and the tarball is obscure and in some cases non-reproducible. The solution for both of these issues is to get rid of the step that adds prebuilt files to the tarball. The tarball now only contains what is in the git tree (*). Getting the additional build dependencies is no longer a problem nowadays, and the complications to keep these dual build modes working are significant. And of course we want to get the meson build system working universally. This commit removes the make distprep target altogether. The make dist target continues to do its job, it just doesn't call distprep anymore. (*) - The tarball also contains the INSTALL file that is built at make dist time, but not by distprep. This is unchanged for now. The make maintainer-clean target, whose job it is to remove the prebuilt files in addition to what make distclean does, is now just an alias to make distprep. (In practice, it is probably obsolete given that git clean is available.) The following programs are now hard build requirements in configure (they were already required by meson.build): - bison - flex - perl --- GNUmakefile.in| 6 +- config/perl.m4| 9 +- config/programs.m4| 21 + configure | 62 ++ contrib/cube/Makefile | 7 +- contrib/fuzzystrmatch/Makefile| 9 +- contrib/seg/Makefile | 7 +- doc/Makefile | 2 +- doc/src/Makefile | 2 +- doc/src/sgml/Makefile | 9 +- doc/src/sgml/installation.sgml| 83 --- meson.build | 2 +- src/Makefile | 5 +- src/Makefile.global.in| 30 +++ src/backend/Makefile | 65 ++- src/backend/bootstrap/Makefile| 6 +- src/backend/catalog/Makefile | 14 +--- src/backend/jit/llvm/Makefile | 2 +- src/backend/nls.mk
Re: Remove distprep
On 2023-Jul-14, Peter Eisentraut wrote: > diff --git a/src/backend/parser/Makefile b/src/backend/parser/Makefile > index 9f1c4022bb..3d33b082f2 100644 > --- a/src/backend/parser/Makefile > +++ b/src/backend/parser/Makefile > @@ -64,8 +64,8 @@ scan.c: FLEX_FIX_WARNING=yes > # Force these dependencies to be known even without dependency info built: > gram.o scan.o parser.o: gram.h > > - > -# gram.c, gram.h, and scan.c are in the distribution tarball, so they > -# are not cleaned here. > -clean distclean maintainer-clean: > +clean: > + rm -f parser/gram.c \ > + parser/gram.h \ > + parser/scan.c > rm -f lex.backup Hmm, this hunk and the equivalents in src/backend/bootstrap and src/backend/replication are wrong: you moved the rule from the parent directory's makefile to the directory where the files reside, but didn't remove the directory name from the command arguments, so the files aren't actually deleted. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "El destino baraja y nosotros jugamos" (A. Schopenhauer)
Re: Remove distprep
On 13.07.23 01:19, Andres Freund wrote:
One thing in particular that isn't clear right now is how "make world"
should behave if the documentation tools are not found. Maybe we should
make a build option, like "--with-docs", to mirror the meson behavior.
Isn't that somewhat unrelated to distprep? I see that you removed missing,
but I don't really understand why as part of this commit?
Ok, I put the docs stuff back the way it was and put "missing" back in.
-# If there are any files in the source directory that we also generate in the
-# build directory, they might get preferred over the newly generated files,
-# e.g. because of a #include "file", which always will search in the current
-# directory first.
-message('checking for file conflicts between source and build directory')
You're thinking this can be removed because distclean is now reliable? There
were some pretty annoying to debug issues early on, where people switched from
an in-tree autoconf build to meson, with some files left over from the source
build, causing problems at a *later* time (when the files should have changed,
but the wrong ones were picked up). That's not really related distprep etc,
so I'd change this separately, if we want to change it.
Ok, I kept it in.
diff --git a/src/tools/pginclude/cpluspluscheck
b/src/tools/pginclude/cpluspluscheck
index 4e09c4686b..287395887c 100755
--- a/src/tools/pginclude/cpluspluscheck
+++ b/src/tools/pginclude/cpluspluscheck
@@ -134,6 +134,9 @@ do
test "$f" = src/interfaces/ecpg/preproc/preproc.h && continue
test "$f" = src/test/isolation/specparse.h && continue
+ # FIXME
+ test "$f" = src/backend/utils/adt/jsonpath_internal.h && continue
+
# ppport.h is not under our control, so we can't make it standalone.
test "$f" = src/pl/plperl/ppport.h && continue
Hm, what's that about?
Don't remember ... ;-) I removed this.
Attached is a new version with the above changes, also updated for the
recently added generate-wait_event_types.pl, and I have adjusted all the
header file linking to use relative paths consistently. This addresses
all issues known to me.
From 260d055b0428130d9db96bed2298495ce7e93505 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Fri, 14 Jul 2023 09:33:09 +0200
Subject: [PATCH v2] Remove distprep
A PostgreSQL release tarball contains a number of prebuilt files, in
particular files produced by bison, flex, perl, and well as html and
man documentation. We have done this consistent with established
practice at the time to not require these tools for building from a
tarball. Some of these tools were hard to get, or get the right
version of, from time to time, and shipping the prebuilt output was a
convenience to users.
Now this has at least two problems:
One, we have to make the build system(s) work in two modes: Building
from a git checkout and building from a tarball. This is pretty
complicated, but it works so far for autoconf/make. It does not
currently work for meson; you can currently only build with meson from
a git checkout. Making meson builds work from a tarball seems very
difficult or impossible. One particular problem is that since meson
requires a separate build directory, we cannot make the build update
files like gram.h in the source tree. So if you were to build from a
tarball and update gram.y, you will have a gram.h in the source tree
and one in the build tree, but the way things work is that the
compiler will always use the one in the source tree. So you cannot,
for example, make any gram.y changes when building from a tarball.
This seems impossible to fix in a non-horrible way.
Second, there is increased interest nowadays in precisely tracking the
origin of software. We can reasonably track contributions into the
git tree, and users can reasonably track the path from a tarball to
packages and downloads and installs. But what happens between the git
tree and the tarball is obscure and in some cases non-reproducible.
The solution for both of these issues is to get rid of the step that
adds prebuilt files to the tarball. The tarball now only contains
what is in the git tree (*). Getting the additional build
dependencies is no longer a problem nowadays, and the complications to
keep these dual build modes working are significant. And of course we
want to get the meson build system working universally.
This commit removes the make distprep target altogether. The make
dist target continues to do its job, it just doesn't call distprep
anymore.
(*) - The tarball also contains the INSTALL file that is built at make
dist time, but not by distprep. This is unchanged for now.
The make maintainer-clean target, whose job it is to remove the
prebuilt files in addition to what make distclean does, is now just an
alias to make distprep. (In practice, it is probably obsolete giv
Re: Remove distprep
Hi,
On 2023-06-09 11:10:14 +0200, Peter Eisentraut wrote:
> Per discussion at the unconference[0], I started to write a patch that
> removes the make distprep target. A more detailed explanation of the
> rationale is also in the patch.
Thanks for tackling this!
> It needs some polishing around the edges, but I wanted to put it out here to
> get things moving and avoid duplicate work.
It'd be nice if we could get this in soon, so we could move ahead with the
src/tools/msvc removal.
> One thing in particular that isn't clear right now is how "make world"
> should behave if the documentation tools are not found. Maybe we should
> make a build option, like "--with-docs", to mirror the meson behavior.
Isn't that somewhat unrelated to distprep? I see that you removed missing,
but I don't really understand why as part of this commit?
> -# If there are any files in the source directory that we also generate in the
> -# build directory, they might get preferred over the newly generated files,
> -# e.g. because of a #include "file", which always will search in the current
> -# directory first.
> -message('checking for file conflicts between source and build directory')
You're thinking this can be removed because distclean is now reliable? There
were some pretty annoying to debug issues early on, where people switched from
an in-tree autoconf build to meson, with some files left over from the source
build, causing problems at a *later* time (when the files should have changed,
but the wrong ones were picked up). That's not really related distprep etc,
so I'd change this separately, if we want to change it.
> diff --git a/src/tools/pginclude/cpluspluscheck
> b/src/tools/pginclude/cpluspluscheck
> index 4e09c4686b..287395887c 100755
> --- a/src/tools/pginclude/cpluspluscheck
> +++ b/src/tools/pginclude/cpluspluscheck
> @@ -134,6 +134,9 @@ do
> test "$f" = src/interfaces/ecpg/preproc/preproc.h && continue
> test "$f" = src/test/isolation/specparse.h && continue
>
> + # FIXME
> + test "$f" = src/backend/utils/adt/jsonpath_internal.h && continue
> +
> # ppport.h is not under our control, so we can't make it standalone.
> test "$f" = src/pl/plperl/ppport.h && continue
Hm, what's that about?
We really ought to replace these scripts with something better, which
understands concurrency...
Greetings,
Andres Freund
Remove distprep
Per discussion at the unconference[0], I started to write a patch that removes the make distprep target. A more detailed explanation of the rationale is also in the patch. It needs some polishing around the edges, but I wanted to put it out here to get things moving and avoid duplicate work. One thing in particular that isn't clear right now is how "make world" should behave if the documentation tools are not found. Maybe we should make a build option, like "--with-docs", to mirror the meson behavior. [0]: https://wiki.postgresql.org/wiki/PgCon_2023_Developer_Unconference#Build_SystemFrom b187ce08fe140225f9ff24bf3ae4d2e97f57221d Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 9 Jun 2023 10:53:51 +0200 Subject: [PATCH v1] Remove distprep A PostgreSQL release tarball contains a number of prebuilt files, in particular files produced by bison, flex, perl, and well as html and man documentation. We have done this consistent with established practice at the time to not require these tools for building from a tarball. Some of these tools were hard to get, or get the right version of, from time to time, and shipping the prebuilt output was a convenience to users. Now this has at least two problems: One, we have to make the build system(s) work in two modes: Building from a git checkout and building from a tarball. This is pretty complicated, but it works so far for autoconf/make. It does not currently work for meson; you can currently only build with meson from a git checkout. Making meson builds work from a tarball seems very difficult or impossible. One particular problem is that since meson requires a separate build directory, we cannot make the build update files like gram.h in the source tree. So if you were to build from a tarball and update gram.y, you will have a gram.h in the source tree and one in the build tree, but the way things work is that the compiler will always use the one in the source tree. So you cannot, for example, make any gram.y changes when building from a tarball. This seems impossible to fix in a non-horrible way. Second, there is increased interest nowadays in precisely tracking the origin of software. We can reasonably track contributions into the git tree, and users can reasonably track the path from a tarball to packages and downloads and installs. But what happens between the git tree and the tarball is obscure and in some cases non-reproducible. The solution for both of these issues is to get rid of the step that adds prebuilt files to the tarball. The tarball now only contains what is in the git tree (*). Getting the additional build dependencies is no longer a problem nowadays, and the complications to keep these dual build modes working are significant. And of course we want to get the meson build system working universally. This commit removes the make distprep target altogether. The make dist target continues to do its job, it just doesn't call distprep anymore. (*) - The tarball also contains the INSTALL file that is built at make dist time, but not by distprep. This is unchanged for now. The make maintainer-clean target, whose job it is to remove the prebuilt files in addition to what make distclean does, is now just an alias to make distprep. (In practice, it is probably obsolete given that git clean is available.) The following programs are now hard build requirements in configure (they were already required by meson.build): - bison - flex - perl FIXME: What should happen in configure if xmllint and xsltproc are not found? --- GNUmakefile.in| 6 +- config/Makefile | 2 - config/meson.build| 2 +- config/missing| 54 config/perl.m4| 9 +- config/programs.m4| 21 + configure | 62 ++ contrib/cube/Makefile | 7 +- contrib/fuzzystrmatch/Makefile| 9 +- contrib/seg/Makefile | 7 +- doc/Makefile | 2 +- doc/src/Makefile | 2 +- doc/src/sgml/Makefile | 31 +-- doc/src/sgml/installation.sgml| 83 --- meson.build | 56 + src/Makefile | 5 +- src/Makefile.global.in| 32 +++ src/backend/Makefile | 52 ++-- src/backend/bootstrap/Makefile| 6 +- src/backend/catalog/Makefile | 8 +- src/backend/jit/llvm/Makefile | 2 +- src/backend/nls.mk| 2 +- src/backend/nodes/Makefile| 6 +- src/backend/parser/Makefile | 8 +- src/b
