Обсуждение: 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_System
Вложения
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
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.
Вложения
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)
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.
Вложения
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
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
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
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
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
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.
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
Вложения
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: 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
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
Вложения
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
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).
Вложения
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
Вложения
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
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.)
Вложения
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
Вложения
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.
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
Вложения
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
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.
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
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.
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.
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
On Mon, Nov 06, 2023 at 04:21:40PM +0100, Peter Eisentraut wrote: > done Nice to see 721856ff24b3 in, thanks! -- Michael
Вложения
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
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
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