Horton

Thread

Commits

  1. Fix make build on MinGW

  2. Stop generating plain-text INSTALL instructions.

  3. Remove distprep

  1. Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-06-09T09:10:14Z

    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_System

  2. Re: Remove distprep

    Andres Freund <andres@anarazel.de> — 2023-07-12T23:19:49Z

    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

  3. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-07-14T07:54:03Z

    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.

  4. Re: Remove distprep

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-07-14T08:24:43Z

    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)

  5. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-07-14T08:56:26Z

    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.

  6. Re: Remove distprep

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-07-14T09:48:26Z

    Peter Eisentraut <peter@eisentraut.org> 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

  7. Re: Remove distprep

    Christoph Berg <myon@debian.org> — 2023-08-09T09:13:10Z

    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

  8. Re: Remove distprep

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-08-09T14:11:32Z

    Christoph Berg <myon@debian.org> 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

  9. Re: Remove distprep

    Christoph Berg <myon@debian.org> — 2023-08-09T14:25:28Z

    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

  10. Re: Remove distprep

    Andres Freund <andres@anarazel.de> — 2023-08-09T14:38:40Z

    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

  11. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-08-16T06:25:21Z

    On 14.07.23 11:48, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> 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.

  12. Re: Remove distprep

    Michael Paquier <michael@paquier.xyz> — 2023-08-18T01:11:12Z

    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

  13. Re: Remove distprep

    Andres Freund <andres@anarazel.de> — 2023-08-18T02:39:40Z

    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?

  14. Re: Remove distprep

    Christoph Berg <myon@debian.org> — 2023-08-18T08:22:47Z

    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

  15. Re: Remove distprep

    Michael Paquier <michael@paquier.xyz> — 2023-08-21T07:24:47Z

    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

  16. Reproducibility (Re: Remove distprep)

    Christoph Berg <myon@debian.org> — 2023-08-21T12:23:27Z

    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

  17. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-08-23T10:46:45Z

    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).

  18. Re: Remove distprep

    Michael Paquier <michael@paquier.xyz> — 2023-09-26T04:49:08Z

    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 <productname>PostgreSQL</productname> from the source
   repository requires reasonably up-to-date versions of <application>bison</application>,
   <application>flex</application>, and <application>Perl</application>.
   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

  19. Re: Remove distprep

    Andres Freund <andres@anarazel.de> — 2023-09-29T16:00:30Z

    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

  20. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-10-05T15:46:46Z

    On 26.09.23 06:49, Michael Paquier wrote:
> 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"

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.

> # 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?

I have removed the "even in distribution tarballs" bit, but the general 
idea is still valid I think.

>     Note that building <productname>PostgreSQL</productname> from the source
>     repository requires reasonably up-to-date versions of <application>bison</application>,
>     <application>flex</application>, and <application>Perl</application>.
>     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.

I have removed that paragraph.

> # 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.

done

The attached updated patch is also split up like Andres suggested 
nearby.  (Not sure if it would be good to commit it that way, but it's 
easier to look at for now for sure.)

  21. Re: Remove distprep

    Michael Paquier <michael@paquier.xyz> — 2023-10-06T02:00:41Z

    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

  22. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-10-06T06:38:31Z

    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.

  23. Re: Remove distprep

    Michael Paquier <michael@paquier.xyz> — 2023-10-06T08:03:40Z

    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

  24. Re: Remove distprep

    Andres Freund <andres@anarazel.de> — 2023-10-06T18:50:34Z

    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

  25. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-10-09T10:16:23Z

    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.

  26. Re: Remove distprep

    Andres Freund <andres@anarazel.de> — 2023-10-09T21:14:11Z

    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

  27. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-11-01T20:39:24Z

    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.

  28. Re: Remove distprep

    Andres Freund <andres@anarazel.de> — 2023-11-02T22:34:18Z

    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.

  29. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2023-11-06T15:21:40Z

    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

  30. Re: Remove distprep

    Michael Paquier <michael@paquier.xyz> — 2023-11-07T07:19:24Z

    On Mon, Nov 06, 2023 at 04:21:40PM +0100, Peter Eisentraut wrote:
> done

Nice to see 721856ff24b3 in, thanks!
--
Michael

  31. Re: Remove distprep

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-11-21T16:13:12Z

    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

  32. Re: Remove distprep

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-11-21T18:23:02Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> 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

  33. Re: Remove distprep

    Andrew Dunstan <andrew@dunslane.net> — 2023-11-21T21:41:26Z

    On 2023-11-21 Tu 13:23, Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> 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

  34. Re: Remove distprep

    Noah Misch <noah@leadboat.com> — 2024-06-16T19:34:48Z

    On Thu, Oct 05, 2023 at 05:46:46PM +0200, Peter Eisentraut wrote:
> --- a/src/backend/Makefile
> +++ b/src/backend/Makefile

>  $(top_builddir)/src/include/storage/lwlocknames.h: storage/lmgr/lwlocknames.h
> -	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
> -	  cd '$(dir $@)' && rm -f $(notdir $@) && \
> -	  $(LN_S) "$$prereqdir/$(notdir $<)" .
> +	rm -f '$@'
> +	$(LN_S) ../../backend/$< '$@'
>  
>  $(top_builddir)/src/include/utils/wait_event_types.h: utils/activity/wait_event_types.h
> -	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
> -	  cd '$(dir $@)' && rm -f $(notdir $@) && \
> -	  $(LN_S) "$$prereqdir/$(notdir $<)" .
> +	rm -f '$@'
> +	$(LN_S) ../../backend/$< '$@'

These broke the
https://www.postgresql.org/docs/17/installation-platform-notes.html#INSTALLATION-NOTES-MINGW
build, where LN_S='cp -pR'.  On other platforms, "make LN_S='cp -pR'"
reproduces this.  Reverting the above lines fixes things.  The buildfarm has
no coverage for that build scenario (fairywren uses Meson).

  35. Re: Remove distprep

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2024-06-16T19:59:06Z

    On 2024-Jun-16, Noah Misch wrote:

> These broke the
> https://www.postgresql.org/docs/17/installation-platform-notes.html#INSTALLATION-NOTES-MINGW
> build, where LN_S='cp -pR'.  On other platforms, "make LN_S='cp -pR'"
> reproduces this.  Reverting the above lines fixes things.  The buildfarm has
> no coverage for that build scenario (fairywren uses Meson).

I agree we should revert this change.  I had commented on this:
https://postgr.es/m/202404051810.et6ufj5snhmt@alvherre.pgsql

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"La rebeldía es la virtud original del hombre" (Arthur Schopenhauer)

  36. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2024-06-20T07:29:45Z

    On 16.06.24 21:34, Noah Misch wrote:
> On Thu, Oct 05, 2023 at 05:46:46PM +0200, Peter Eisentraut wrote:
>> --- a/src/backend/Makefile
>> +++ b/src/backend/Makefile
> 
>>   $(top_builddir)/src/include/storage/lwlocknames.h: storage/lmgr/lwlocknames.h
>> -	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
>> -	  cd '$(dir $@)' && rm -f $(notdir $@) && \
>> -	  $(LN_S) "$$prereqdir/$(notdir $<)" .
>> +	rm -f '$@'
>> +	$(LN_S) ../../backend/$< '$@'
>>   
>>   $(top_builddir)/src/include/utils/wait_event_types.h: utils/activity/wait_event_types.h
>> -	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
>> -	  cd '$(dir $@)' && rm -f $(notdir $@) && \
>> -	  $(LN_S) "$$prereqdir/$(notdir $<)" .
>> +	rm -f '$@'
>> +	$(LN_S) ../../backend/$< '$@'
> 
> These broke the
> https://www.postgresql.org/docs/17/installation-platform-notes.html#INSTALLATION-NOTES-MINGW
> build, where LN_S='cp -pR'.  On other platforms, "make LN_S='cp -pR'"
> reproduces this.  Reverting the above lines fixes things.  The buildfarm has
> no coverage for that build scenario (fairywren uses Meson).

Is it just these two instances?

Commit 721856ff24b contains a few more hunks that change something about 
LN_S.  Are those ok?

  37. Re: Remove distprep

    Noah Misch <noah@leadboat.com> — 2024-06-20T14:34:05Z

    On Thu, Jun 20, 2024 at 09:29:45AM +0200, Peter Eisentraut wrote:
> On 16.06.24 21:34, Noah Misch wrote:
> > On Thu, Oct 05, 2023 at 05:46:46PM +0200, Peter Eisentraut wrote:
> > > --- a/src/backend/Makefile
> > > +++ b/src/backend/Makefile
> > 
> > >   $(top_builddir)/src/include/storage/lwlocknames.h: storage/lmgr/lwlocknames.h
> > > -	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
> > > -	  cd '$(dir $@)' && rm -f $(notdir $@) && \
> > > -	  $(LN_S) "$$prereqdir/$(notdir $<)" .
> > > +	rm -f '$@'
> > > +	$(LN_S) ../../backend/$< '$@'
> > >   $(top_builddir)/src/include/utils/wait_event_types.h: utils/activity/wait_event_types.h
> > > -	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
> > > -	  cd '$(dir $@)' && rm -f $(notdir $@) && \
> > > -	  $(LN_S) "$$prereqdir/$(notdir $<)" .
> > > +	rm -f '$@'
> > > +	$(LN_S) ../../backend/$< '$@'
> > 
> > These broke the
> > https://www.postgresql.org/docs/17/installation-platform-notes.html#INSTALLATION-NOTES-MINGW
> > build, where LN_S='cp -pR'.  On other platforms, "make LN_S='cp -pR'"
> > reproduces this.  Reverting the above lines fixes things.  The buildfarm has
> > no coverage for that build scenario (fairywren uses Meson).
> 
> Is it just these two instances?

Yes.

> Commit 721856ff24b contains a few more hunks that change something about
> LN_S.  Are those ok?

I'm guessing "make LN_S='cp -pR'" didn't have a problem with those because
they have "." as the second argument.  "cp -pR ../foo ." is compatible with
"ln -s ../foo ." in that both are interpreting "../bar" relative to the same
directory.  Not so for "cp -pR ../foo bar/baz" vs. "ln -s ../foo bar/baz".

  38. Re: Remove distprep

    Peter Eisentraut <peter@eisentraut.org> — 2024-06-21T06:19:53Z

    On 20.06.24 16:34, Noah Misch wrote:
> On Thu, Jun 20, 2024 at 09:29:45AM +0200, Peter Eisentraut wrote:
>> On 16.06.24 21:34, Noah Misch wrote:
>>> On Thu, Oct 05, 2023 at 05:46:46PM +0200, Peter Eisentraut wrote:
>>>> --- a/src/backend/Makefile
>>>> +++ b/src/backend/Makefile
>>>
>>>>    $(top_builddir)/src/include/storage/lwlocknames.h: storage/lmgr/lwlocknames.h
>>>> -	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
>>>> -	  cd '$(dir $@)' && rm -f $(notdir $@) && \
>>>> -	  $(LN_S) "$$prereqdir/$(notdir $<)" .
>>>> +	rm -f '$@'
>>>> +	$(LN_S) ../../backend/$< '$@'
>>>>    $(top_builddir)/src/include/utils/wait_event_types.h: utils/activity/wait_event_types.h
>>>> -	prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
>>>> -	  cd '$(dir $@)' && rm -f $(notdir $@) && \
>>>> -	  $(LN_S) "$$prereqdir/$(notdir $<)" .
>>>> +	rm -f '$@'
>>>> +	$(LN_S) ../../backend/$< '$@'
>>>
>>> These broke the
>>> https://www.postgresql.org/docs/17/installation-platform-notes.html#INSTALLATION-NOTES-MINGW
>>> build, where LN_S='cp -pR'.  On other platforms, "make LN_S='cp -pR'"
>>> reproduces this.  Reverting the above lines fixes things.  The buildfarm has
>>> no coverage for that build scenario (fairywren uses Meson).
>>
>> Is it just these two instances?
> 
> Yes.
> 
>> Commit 721856ff24b contains a few more hunks that change something about
>> LN_S.  Are those ok?
> 
> I'm guessing "make LN_S='cp -pR'" didn't have a problem with those because
> they have "." as the second argument.  "cp -pR ../foo ." is compatible with
> "ln -s ../foo ." in that both are interpreting "../bar" relative to the same
> directory.  Not so for "cp -pR ../foo bar/baz" vs. "ln -s ../foo bar/baz".

Ok, fix pushed.