Обсуждение: MacOS: xsltproc fails with "warning: failed to load external entity"

Hi hackers,

I'm having some difficulties building the documentation on MacOS.

I'm using ./full-build.sh script from [1] repository. It worked just
fine for many years but since recently it started to fail like this:

```
/usr/bin/xsltproc --path . --stringparam pg.version '16devel'
/Users/eax/projects/c/pgscripts/../postgresql/doc/src/sgml/stylesheet.xsl
postgres-full.xml
error : Unknown IO error
warning: failed to load external entity
"http://docbook.sourceforge.net/release/xsl/current/xhtml/chunk.xsl"
compilation error: file
/Users/eax/projects/c/pgscripts/../postgresql/doc/src/sgml/stylesheet.xsl
line 6 element import
xsl:import : unable to load
http://docbook.sourceforge.net/release/xsl/current/xhtml/chunk.xsl
error : Unknown IO error
/Users/eax/projects/c/postgresql/doc/src/sgml/stylesheet-html-common.xsl:4:
warning: failed to load external entity
"http://docbook.sourceforge.net/release/xsl/current/common/entities.ent"
%common.entities;
                 ^
Entity: line 1:
 %common.entities;
                  ^
[...]

```

This is not a network problem. I can download chunk.xsl with wget and
also build the documentation on my Linux laptop.

I've tried `brew reinstall` and also:

```
./configure ... XMLLINT="xmllint --nonet" XSLTPROC="xsltproc --nonet"
```

... as suggested by the documentation [2] but it didn't change anything.

I checked the archive of pgsql-hackers@ but was unable to find
anything relevant.

I'm using MacOS Monterey 12.6.2.

```
$ brew info docbook
==> docbook: stable 5.1 (bottled)
...
$ brew info docbook-xsl
==> docbook-xsl: stable 1.79.2 (bottled)
...
```

At this point I could use a friendly piece of advice from the community.

[1]: https://github.com/afiskon/pgscripts/
[2]: https://www.postgresql.org/docs/15/docguide-toolsets.html

-- 
Best regards,
Aleksander Alekseev

Hi hackers,

> At this point I could use a friendly piece of advice from the community.

I've found a solution:

```
export SGML_CATALOG_FILES=/usr/local/etc/xml/catalog
export XMLLINT="xmllint --catalogs"
export XSLTPROC="xsltproc --catalogs"
```

I will submit a patch for the documentation in a bit, after I'll check
it properly.

-- 
Best regards,
Aleksander Alekseev

Hi hackers,

> I've found a solution:
>
> ```
> export SGML_CATALOG_FILES=/usr/local/etc/xml/catalog
> export XMLLINT="xmllint --catalogs"
> export XSLTPROC="xsltproc --catalogs"
> ```
>
> I will submit a patch for the documentation in a bit, after I'll check
> it properly.

PFA the patch.

I don't have a strong opinion regarding any particular wording and
would like to ask the committer to change it as he sees fit.

-- 
Best regards,
Aleksander Alekseev

Aleksander Alekseev <aleksander@timescale.com> writes:
>> I've found a solution:
>>
>> ```
>> export SGML_CATALOG_FILES=/usr/local/etc/xml/catalog
>> export XMLLINT="xmllint --catalogs"
>> export XSLTPROC="xsltproc --catalogs"
>> ```

Hmm, there is no such directory on my Mac, and indeed this recipe
does not work here.  I tried to transpose it to MacPorts by
substituting /opt/local/etc/xml/catalog, which does exist --- but
the recipe still doesn't work.

I believe what is actually failing is that http://docbook.sourceforge.net
now redirects to https:, and the ancient xsltproc version provided by
Apple doesn't do https.  What you need to do if you want to use their
xsltproc is install a local copy of the SGML catalog files and
stylesheets, preferably in the place that xsltproc would look by default
(/etc/xml/catalog seems to be the standard one).  It would be good to
document how to do that, but this patch doesn't do so.

What we do actually have already is a recommendation to install
appropriate MacPorts or Homebrew packages:

https://www.postgresql.org/docs/devel/docguide-toolsets.html#DOCGUIDE-TOOLSETS-INST-MACOS

and it works okay for me as long as I use MacPorts' version of xsltproc.

            regards, tom lane

Hi Tom,

Thanks for the feedback.

> Hmm, there is no such directory on my Mac, and indeed this recipe
> does not work here.  I tried to transpose it to MacPorts by
> substituting /opt/local/etc/xml/catalog, which does exist --- but
> the recipe still doesn't work.

Well, that's a bummer.

> What we do actually have already is a recommendation to install
> appropriate MacPorts or Homebrew packages:
>
> https://www.postgresql.org/docs/devel/docguide-toolsets.html#DOCGUIDE-TOOLSETS-INST-MACOS
>
> and it works okay for me as long as I use MacPorts' version of xsltproc.

Unfortunately it doesn't work for Homebrew anymore and there seems to
be only one xsltproc in the system.

> I believe what is actually failing is that http://docbook.sourceforge.net
> now redirects to https:, and the ancient xsltproc version provided by
> Apple doesn't do https.  What you need to do if you want to use their
> xsltproc is install a local copy of the SGML catalog files and
> stylesheets, preferably in the place that xsltproc would look by default
> (/etc/xml/catalog seems to be the standard one).  It would be good to
> document how to do that, but this patch doesn't do so.

Fair enough.

I would appreciate it if you could help figuring out how to do this
for MacPorts, since I'm not a MacPorts user. I'll figure out how to do
this for Homebrew.

Does something like:

```
ln -s /opt/local/etc/xml/catalog /etc/xml/catalog
```

... work for you? Does your:

```
xsltproc --help
```

... also say that it uses /etc/xml/catalog path by default?

-- 
Best regards,
Aleksander Alekseev

Aleksander Alekseev <aleksander@timescale.com> writes:
>> What we do actually have already is a recommendation to install
>> appropriate MacPorts or Homebrew packages:
>> https://www.postgresql.org/docs/devel/docguide-toolsets.html#DOCGUIDE-TOOLSETS-INST-MACOS
>> and it works okay for me as long as I use MacPorts' version of xsltproc.

> Unfortunately it doesn't work for Homebrew anymore and there seems to
> be only one xsltproc in the system.

Hmm.  Seems unlikely that Homebrew would have dropped the package(s)
altogether.  But ... poking at this, I discovered that there are
inaccuracies in our docs for MacPorts:

* /opt/local/bin/xsltproc is provided by libxslt, and
/opt/local/bin/xmllint is provided by libxml2, neither of which
will be installed by our recipe as given.  You might have pulled
those ports in already to build Postgres with, but if you didn't, the
recipe will fail.  I wonder if the Homebrew recipe has the same bug.

* At some point MacPorts renamed docbook-xsl to docbook-xsl-nons.
This is harmless at the moment, because if you ask for docbook-xsl
it will automatically install docbook-xsl-nons instead.  I wonder
if that'll be true indefinitely, though.

I also wonder whether we shouldn't point at the meta-package docbook-xml
instead of naming a particular version here (and having to update
that from time to time).  The extra disk space to install all the DTD
versions is entirely insignificant (< 2MB).

> Does your:
> xsltproc --help
> ... also say that it uses /etc/xml/catalog path by default?

Both /usr/bin/xsltproc and /opt/local/bin/xsltproc say

        --catalogs : use SGML catalogs from $SGML_CATALOG_FILES
                     otherwise XML Catalogs starting from
                 file:///etc/xml/catalog are activated by default

However, this appears to be a lie for /opt/local/bin/xsltproc;
what it's apparently *actually* using is /opt/local/etc/xml/catalog,
which is what MacPorts provides.

I repeated the test I did this morning, and this time using --catalogs
with SGML_CATALOG_FILES set to /opt/local/etc/xml/catalog worked for me,
using either copy of xsltproc.  I must've fat-fingered it somehow before.
Nonetheless, I doubt that that recipe is worth recommending to MacPorts
users: if they pull in the DTD packages they might as well pull in libxml2
and libxslt, and then they don't need to adjust anything.

In short, I think we need to update J.2.4 to say this for MacPorts:

sudo port install libxml2 libxslt docbook-xml docbook-xsl-nons fop

and I strongly suspect that the Homebrew recipe has a similar oversight.

            regards, tom lane

On 30.01.23 20:04, Aleksander Alekseev wrote:
> I would appreciate it if you could help figuring out how to do this
> for MacPorts, since I'm not a MacPorts user. I'll figure out how to do
> this for Homebrew.

I'm on macOS Monterey and Homebrew.  I'm sure I have gone through many 
variations of this setup, but checking what I happen to be using right 
now, Makefile.global says

XMLLINT         = /usr/bin/xmllint
XSLTPROC        = /usr/bin/xsltproc

and in the environment there is

XML_CATALOG_FILES=/usr/local/etc/xml/catalog

Just testing this right now, you can avoid having to set this 
environment variable by making the default catalog file /etc/xml/catalog 
include /usr/local/etc/xml/catalog.

It also works for me to use the Homebrew-provided versions of these tools:

XMLLINT         = /usr/local/opt/libxml2/bin/xmllint
XSLTPROC        = /usr/local/opt/libxslt/bin/xsltproc

But I can't determine right now what catalog file they look at by 
default.  It appears that it's neither /etc/xml/catalog nor 
/usr/local/etc/xml/catalog.  So in this case, setting XML_CATALOG_FILES 
is necessary.

For either sets of tools, the automatic download option doesn't appear 
to work anymore.  This probably has to do with either the https or the 
redirects that have been mentioned.

Hi hackers,

> /opt/local/bin/xsltproc is provided by libxslt, and
> /opt/local/bin/xmllint is provided by libxml2, neither of which
> will be installed by our recipe as given.  You might have pulled
> those ports in already to build Postgres with, but if you didn't, the
> recipe will fail.  I wonder if the Homebrew recipe has the same bug.

Right, I had libxml2 installed (which provides xmllint) but not
libxslt (which provides xsltproc). For this reason I could find only
the version of xsltproc shipped with macOS.

> Both /usr/bin/xsltproc and /opt/local/bin/xsltproc say
>
>        --catalogs : use SGML catalogs from $SGML_CATALOG_FILES
>                     otherwise XML Catalogs starting from
>                 file:///etc/xml/catalog are activated by default
>
> However, this appears to be a lie for /opt/local/bin/xsltproc;
> what it's apparently *actually* using is /opt/local/etc/xml/catalog,
> which is what MacPorts provides.

> I repeated the test I did this morning, and this time using --catalogs
> with SGML_CATALOG_FILES set to /opt/local/etc/xml/catalog worked for me,
> using either copy of xsltproc.  I must've fat-fingered it somehow before.
> Nonetheless, I doubt that that recipe is worth recommending to MacPorts
> users: if they pull in the DTD packages they might as well pull in libxml2
> and libxslt, and then they don't need to adjust anything.

Got it, thanks.

> In short, I think we need to update J.2.4 to say this for MacPorts:
>
> sudo port install libxml2 libxslt docbook-xml docbook-xsl-nons fop

Agree. I decided to include libxml2 and libxslt for Homebrew as well.
The documentation above explains what these packages are needed for
and also says that some of the packages may be optional. E.g. fop is
actually not strictly required but we recommend installing it anyway.

> But I can't determine right now what catalog file they look at by
> default.  It appears that it's neither /etc/xml/catalog nor
> /usr/local/etc/xml/catalog.  So in this case, setting XML_CATALOG_FILES
> is necessary.
>
> For either sets of tools, the automatic download option doesn't appear
> to work anymore.  This probably has to do with either the https or the
> redirects that have been mentioned.

Peter, thanks for reporting this. I got the same results: neither
tools work without setting XML_CATALOG_FILES and setting this
environment variable work for both Homebrew and macOS versions.

Here is the summary of our findings. PFA the updated patch v2.


While on it, I noticed that the documentation says "On macOS, you can
build the HTML and man documentation without installing anything
extra." I strongly suspect this may not be true anymore. This is
somewhat difficult to check however. Some of the recommended packages
were installed as dependencies of other packages and I don't feel like
taking a risk of running:

```
brew uninstall --ignore-dependencies libxml2 libxslt docbook docbook-xsl
```

... right now. However maybe we should rephrase this to make sure
there are fewer supported/recommended ways of building the
documentation? The alternative ways may also work but if they don't
there will be no actions required from us.

I included the corresponding path as well.

-- 
Best regards,
Aleksander Alekseev

Aleksander Alekseev <aleksander@timescale.com> writes:
>> For either sets of tools, the automatic download option doesn't appear
>> to work anymore.  This probably has to do with either the https or the
>> redirects that have been mentioned.

> Peter, thanks for reporting this. I got the same results: neither
> tools work without setting XML_CATALOG_FILES and setting this
> environment variable work for both Homebrew and macOS versions.

> Here is the summary of our findings. PFA the updated patch v2.

It's worse than that: I find that

    export XML_CATALOG_FILES=/dev/null

breaks the docs build on RHEL8 and Fedora 37 (latest) too, with the
same "failed to load external entity" symptom.  I conclude from this
that there is no version of xsltproc anywhere that can still download
the required files automatically.  So we need to take out the advice
that says you can rely on auto-download for everybody, not just macOS.

If this is indeed the case, perhaps we ought to start inserting --nonet
into the invocations.  There's not much use in allowing these tools to
perform internet access when the best-case scenario is that they fail.
(Worst-case, you could end up getting hacked, perhaps?)

            regards, tom lane

I wrote:
> It's worse than that: I find that
>     export XML_CATALOG_FILES=/dev/null
> breaks the docs build on RHEL8 and Fedora 37 (latest) too, with the
> same "failed to load external entity" symptom.  I conclude from this
> that there is no version of xsltproc anywhere that can still download
> the required files automatically.  So we need to take out the advice
> that says you can rely on auto-download for everybody, not just macOS.

> If this is indeed the case, perhaps we ought to start inserting --nonet
> into the invocations.  There's not much use in allowing these tools to
> perform internet access when the best-case scenario is that they fail.

Concretely, I'm thinking something like the attached.  Notes:

1. I have not tested the meson changes.

2. As this is written, you can't override the --nonet options very
easily in the Makefile build (you could do so at runtime by setting
XSLTPROC, but not at configure time); and you can't override them at
all in the meson build.  Given the lack of evidence that it's still
useful to allow net access, I'm untroubled by that.  I did intentionally
skip using "override" in the Makefile, though, to allow that case.

3. For consistency with the directions for other platforms, I made
the package lists for macOS just mention libxslt.  That should
be enough to pull in libxml2 as well.

4. Use of --nonet changes the error message you get if xsltproc
can't find the DTDs.  I copied the error I get from MacPorts'
version of xsltproc, but can you confirm it's the same on Homebrew?

            regards, tom lane

diff --git a/doc/src/sgml/Makefile b/doc/src/sgml/Makefile
index 4f0e39223c..b96c7cbf22 100644
--- a/doc/src/sgml/Makefile
+++ b/doc/src/sgml/Makefile
@@ -41,11 +41,15 @@ endif

 XMLINCLUDE = --path .

-ifndef XMLLINT
+ifdef XMLLINT
+XMLLINT := $(XMLLINT) --nonet
+else
 XMLLINT = $(missing) xmllint
 endif

-ifndef XSLTPROC
+ifdef XSLTPROC
+XSLTPROC := $(XSLTPROC) --nonet
+else
 XSLTPROC = $(missing) xsltproc
 endif

diff --git a/doc/src/sgml/docguide.sgml b/doc/src/sgml/docguide.sgml
index 787caef70d..cf9d4d4389 100644
--- a/doc/src/sgml/docguide.sgml
+++ b/doc/src/sgml/docguide.sgml
@@ -151,18 +151,6 @@
    here.
   </para>

-  <para>
-   You can get away with not installing DocBook XML and the DocBook XSLT
-   stylesheets locally, because the required files will be downloaded from the
-   Internet and cached locally.  This may in fact be the preferred solution if
-   your operating system packages provide only an old version of these files,
-   or if no packages are available at all.
-   If you want to prevent any attempt to access the Internet while building
-   the documentation, you need to pass the <option>--nonet</option> option
-   to <command>xmllint</command> and <command>xsltproc</command>; see below
-   for an example.
-  </para>
-
   <sect2 id="docguide-toolsets-inst-fedora-et-al">
    <title>Installation on Fedora, RHEL, and Derivatives</title>

@@ -207,22 +195,38 @@ apt-get install docbook-xml docbook-xsl fop libxml2-utils xsltproc
   <sect2 id="docguide-toolsets-inst-macos">
    <title>macOS</title>

-   <para>
-    On macOS, you can build the HTML and man documentation without installing
-    anything extra.  If you want to build PDFs or want to install a local copy
-    of DocBook, you can get those from your preferred package manager.
-   </para>
-
    <para>
     If you use MacPorts, the following will get you set up:
 <programlisting>
-sudo port install docbook-xml-4.5 docbook-xsl fop
+sudo port install docbook-xml docbook-xsl-nons fop libxslt
 </programlisting>
     If you use Homebrew, use this:
 <programlisting>
-brew install docbook docbook-xsl fop
+brew install docbook docbook-xsl fop libxslt
 </programlisting>
    </para>
+
+   <para>
+    The Homebrew-supplied programs require the following environment variable
+    to be set:
+<programlisting>
+export XML_CATALOG_FILES=/usr/local/etc/xml/catalog
+</programlisting>
+    Without it, <command>xsltproc</command> will throw errors like this:
+<programlisting>
+I/O error : Attempt to load network entity http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd
+postgres.sgml:21: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd"
+...
+</programlisting>
+   </para>
+
+   <para>
+    While it is possible to use the Apple-provided versions
+    of <command>xmllint</command> and <command>xsltproc</command>
+    instead of those from MacPorts or Homebrew, you'll still need
+    to install the DocBook DTD and stylesheets, and set up a catalog
+    file that points to them.
+   </para>
   </sect2>

   <sect2 id="docguide-toolsets-configure">
@@ -253,12 +257,6 @@ checking for dbtoepub... dbtoepub
    these programs, for example
 <screen>
 ./configure ... XMLLINT=/opt/local/bin/xmllint ...
-</screen>
-   Also, if you want to ensure that <filename>xmllint</filename>
-   and <filename>xsltproc</filename> will not perform any network access,
-   you can do something like
-<screen>
-./configure ... XMLLINT="xmllint --nonet" XSLTPROC="xsltproc --nonet" ...
 </screen>
   </para>
   </sect2>
diff --git a/doc/src/sgml/images/Makefile b/doc/src/sgml/images/Makefile
index f9e356348b..645519095d 100644
--- a/doc/src/sgml/images/Makefile
+++ b/doc/src/sgml/images/Makefile
@@ -9,7 +9,7 @@ ALL_IMAGES = \

 DITAA = ditaa
 DOT = dot
-XSLTPROC = xsltproc
+XSLTPROC = xsltproc --nonet

 all: $(ALL_IMAGES)

diff --git a/doc/src/sgml/meson.build b/doc/src/sgml/meson.build
index b9f4c6a05b..524a4e00e4 100644
--- a/doc/src/sgml/meson.build
+++ b/doc/src/sgml/meson.build
@@ -87,7 +87,8 @@ postgres_full_xml = custom_target('postgres-full.xml',
   input: 'postgres.sgml',
   output: 'postgres-full.xml',
   depfile: 'postgres-full.xml.d',
-  command: [xmllint, '--noent', '--valid', '--path', '@OUTDIR@', '-o', '@OUTPUT@', '@INPUT@'],
+  command: [xmllint, '--nonet', '--noent', '--valid',
+            '--path', '@OUTDIR@', '-o', '@OUTPUT@', '@INPUT@'],
   depends: doc_generated,
   build_by_default: false,
 )
@@ -100,6 +101,7 @@ alldocs += postgres_full_xml
 #
 if xsltproc_bin.found()
   xsltproc_flags = [
+    '--nonet',
     '--stringparam', 'pg.version', pg_version,
     '--param', 'website.stylesheet', '1'
   ]

Hi,

On 2023-01-31 18:54:31 -0500, Tom Lane wrote:
> 1. I have not tested the meson changes.

Works here.


> 2. As this is written, you can't override the --nonet options very
> easily in the Makefile build (you could do so at runtime by setting
> XSLTPROC, but not at configure time); and you can't override them at
> all in the meson build.  Given the lack of evidence that it's still
> useful to allow net access, I'm untroubled by that.  I did intentionally
> skip using "override" in the Makefile, though, to allow that case.

I'm not troubled by this either.


I wonder if we should provide a build target to download the stylesheets
ourselves. The amount of packages our instructions download is quite
substantial. We could perhaps trim them a bit, but we intentionally are
including things to build pdfs etc as well, which does make sense...


Greetings,

Andres Freund

Hi hackers,

> Concretely, I'm thinking something like the attached.  Notes:

> > 1. I have not tested the meson changes.
> Works here.

Took me a while to figure out how to build the documentation with Meson:

```
XML_CATALOG_FILES=/usr/local/etc/xml/catalog ninja -C build alldocs
```

It works. Perhaps we should add:

```
ninja -C build alldocs
```

... command to installation.sgml file while on it, to the 17.4.1
Building and Installation with Meson / Short Version section.

> > 2. As this is written, you can't override the --nonet options very
> > easily in the Makefile build (you could do so at runtime by setting
> > XSLTPROC, but not at configure time); and you can't override them at
> > all in the meson build.  Given the lack of evidence that it's still
> > useful to allow net access, I'm untroubled by that.  I did intentionally
> > skip using "override" in the Makefile, though, to allow that case.
>
> I'm not troubled by this either.

Neither am I.

> 3. For consistency with the directions for other platforms, I made
> the package lists for macOS just mention libxslt.  That should
> be enough to pull in libxml2 as well.

Fair enough.

> 4. Use of --nonet changes the error message you get if xsltproc
> can't find the DTDs.  I copied the error I get from MacPorts'
> version of xsltproc, but can you confirm it's the same on Homebrew?

Yes, the message is the same.

-- 
Best regards,
Aleksander Alekseev

Hi,

On 2023-02-01 13:05:32 +0300, Aleksander Alekseev wrote:
> Took me a while to figure out how to build the documentation with Meson:
> 
> ```
> XML_CATALOG_FILES=/usr/local/etc/xml/catalog ninja -C build alldocs
> ```
> 
> It works. Perhaps we should add:
> 
> ```
> ninja -C build alldocs
> ```

FWIW, just 'docs' would build just the multi-page html/man pages,
alldocs takes a lot longer...

And yes, adding that to the docs is a good idea.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2023-02-01 13:05:32 +0300, Aleksander Alekseev wrote:
>> It works. Perhaps we should add:
>> ninja -C build alldocs

> FWIW, just 'docs' would build just the multi-page html/man pages,
> alldocs takes a lot longer...

Hmm ... why does 'docs' include the man pages, and not just the html?
It's unlike what "make -C doc/src/sgml all" does in the Makefile
system, and I don't find it to be an improvement.  I want the man
pages approximately never, so I don't care to wait around for them
to be built.

While I'm bitching ... section 17.1 doesn't mention that you need
ninja to use meson, much less mention the minimum version.  And
the minimum version appears to be newer than RHEL8's 1.8.2,
which I find pretty unfortunate.  On RHEL8, it fails with

$ ninja
ninja: error: build.ninja:6771: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing list
ifit affects you 

I did manage to test this stuff on bleeding-edge Fedora,
but ...

            regards, tom lane

Hi,

On 2023-02-01 12:23:27 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2023-02-01 13:05:32 +0300, Aleksander Alekseev wrote:
> >> It works. Perhaps we should add:
> >> ninja -C build alldocs
> 
> > FWIW, just 'docs' would build just the multi-page html/man pages,
> > alldocs takes a lot longer...
> 
> Hmm ... why does 'docs' include the man pages, and not just the html?

I think it's because the makefile is doing things a bit oddly, and I
didn't quite grok that in the right moment.

I probably just saw:
all: html man

but before that there's

# Make "html" the default target, since that is what most people tend
# to want to use.
html:


> It's unlike what "make -C doc/src/sgml all" does in the Makefile
> system, and I don't find it to be an improvement.

Well, that'd actually build the manpages too, afaics :). But I get the
point.

I really have no opinion on what we should should build under what
name. Happy to change what's included in 'docs', add additional targets,
etc.


> I want the man pages approximately never, so I don't care to wait
> around for them to be built.
> 
> While I'm bitching ... section 17.1 doesn't mention that you need
> ninja to use meson, much less mention the minimum version.

Peter rewrote the requirements (almost?) entirely while committing the
docs from Samay and hasn't responded to my concerns about the new
form...


Normally the ninja version that's pulled in by meson should suffice. I
suspect that the problem you found can be worked around.

> And the minimum version appears to be newer than RHEL8's 1.8.2, which
> I find pretty unfortunate.  On RHEL8, it fails with
> $ ninja
> ninja: error: build.ninja:6771: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing
listif it affects you
 

What's in that line +- 2 lines?  And/or what are the steps that got you
to that point?

I'll try building 1.8.2 and reproing.


> I did manage to test this stuff on bleeding-edge Fedora,
> but ...

Yea, I worked a fair bit to avoid requiring a too new version, I'll try
to figure out what went wrong.  I did built on rhel8 not long ago, so I
suspect it's a corner case somewhere.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2023-02-01 12:23:27 -0500, Tom Lane wrote:
>> It's unlike what "make -C doc/src/sgml all" does in the Makefile
>> system, and I don't find it to be an improvement.

> Well, that'd actually build the manpages too, afaics :). But I get the
> point.

Ah, sorry, I too had forgotten that "all" isn't the default target
there.  I actually just go into that directory and type "make".

> I really have no opinion on what we should should build under what
> name. Happy to change what's included in 'docs', add additional targets,
> etc.

I think "docs" for just the html and "alldocs" for all supported
outputs is probably reasonable.  If we ever get to the point of
building distribution tarballs with meson, we might need another
target for html+man, but I suppose that's a long way off.

>> And the minimum version appears to be newer than RHEL8's 1.8.2, which
>> I find pretty unfortunate.  On RHEL8, it fails with
>> $ ninja
>> ninja: error: build.ninja:6771: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing
listif it affects you 

> What's in that line +- 2 lines?  And/or what are the steps that got you
> to that point?

"meson setup build" is sufficient to see it --- apparently ninja
gets invoked at the end of that, and it's already unhappy.  But
it repeats after "cd build; ninja".

It seems to be unhappy about the stanza for building sql_help.c?
Line 6771 is the blank line after "description" in this bit:

build src/bin/psql/sql_help.c src/bin/psql/sql_help.h: CUSTOM_COMMAND_DEP  | ../src/bin/psql/create_help.pl
/usr/bin/perl
 DEPFILE = src/bin/psql/sql_help.dep
 DEPFILE_UNQUOTED = src/bin/psql/sql_help.dep
 COMMAND = /usr/bin/perl ../src/bin/psql/create_help.pl --docdir ../doc/src/sgml/ref --depfile
src/bin/psql/sql_help.dep--outdir src/bin/psql --basename sql_help 
 description = Generating$ psql_help$ with$ a$ custom$ command

build src/bin/psql/psql.p/meson-generated_.._psqlscanslash.c.o: c_COMPILER src/bin/psql/psqlscanslash.c ||
src/bin/psql/sql_help.hsrc/include/catalog/pg_aggregate_d.h src/include/catalog/pg_am_d.h
src/include/catalog/pg_amop_d.hsrc/include/catalog/pg_amproc_d.h src/include/catalog/pg_attrdef_d.h
src/include/catalog/pg_attribute_d.hsrc/include/catalog/pg_auth_members_d.h src/include/catalog/pg_authid_d.h
src/include/catalog/pg_cast_d.hsrc/include/catalog/pg_class_d.h src/include/catalog/pg_collation_d.h
src/include/catalog/pg_constraint_d.hsrc/include/catalog/pg_conversion_d.h src/include/catalog/pg_database_d.h
src/include/catalog/pg_db_role_setting_d.hsrc/include/catalog/pg_default_acl_d.h src/include/catalog/pg_depend_d.h
src/include/catalog/pg_description_d.hsrc/include/catalog/pg_enum_d.h src/include/catalog/pg_event_trigger_d.h
src/include/catalog/pg_extension_d.hsrc/include/catalog/pg_foreign_data_wrapper_d.h
src/include/catalog/pg_foreign_server_d.hsrc/include/catalog/pg_foreign_table_d.h src/include/catalog/pg_index_d.h
src/include/catalog/pg_inherits_d.hsrc/include/catalog/pg_init_privs_d.h src/include/catalog/pg_language_d.h
src/include/catalog/pg_largeobject_d.hsrc/include/catalog/pg_largeobject_metadata_d.h
src/include/catalog/pg_namespace_d.hsrc/include/catalog/pg_opclass_d.h src/include/catalog/pg_operator_d.h
src/include/catalog/pg_opfamily_d.hsrc/include/catalog/pg_parameter_acl_d.h
src/include/catalog/pg_partitioned_table_d.hsrc/include/catalog/pg_policy_d.h src/include/catalog/pg_proc_d.h
src/include/catalog/pg_publication_d.hsrc/include/catalog/pg_publication_namespace_d.h
src/include/catalog/pg_publication_rel_d.hsrc/include/catalog/pg_range_d.h
src/include/catalog/pg_replication_origin_d.hsrc/include/catalog/pg_rewrite_d.h src/include/catalog/pg_seclabel_d.h
src/include/catalog/pg_sequence_d.hsrc/include/catalog/pg_shdepend_d.h src/include/catalog/pg_shdescription_d.h
src/include/catalog/pg_shseclabel_d.hsrc/include/catalog/pg_statistic_d.h src/include/catalog/pg_statistic_ext_d.h
src/include/catalog/pg_statistic_ext_data_d.hsrc/include/catalog/pg_subscription_d.h
src/include/catalog/pg_subscription_rel_d.hsrc/include/catalog/pg_tablespace_d.h src/include/catalog/pg_transform_d.h
src/include/catalog/pg_trigger_d.hsrc/include/catalog/pg_ts_config_d.h src/include/catalog/pg_ts_config_map_d.h
src/include/catalog/pg_ts_dict_d.hsrc/include/catalog/pg_ts_parser_d.h src/include/catalog/pg_ts_template_d.h
src/include/catalog/pg_type_d.hsrc/include/catalog/pg_user_mapping_d.h src/include/catalog/postgres.bki
src/include/catalog/schemapg.hsrc/include/catalog/system_constraints.sql src/include/catalog/system_fk_info.h
src/include/nodes/nodetags.hsrc/include/utils/errcodes.h 
 DEPFILE = src/bin/psql/psql.p/meson-generated_.._psqlscanslash.c.o.d
 DEPFILE_UNQUOTED = src/bin/psql/psql.p/meson-generated_.._psqlscanslash.c.o.d
 ARGS = -Isrc/bin/psql/psql.p -Isrc/bin/psql -I../src/bin/psql -Isrc/include -I../src/include -Isrc/interfaces/libpq
-I../src/interfaces/libpq-fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O3 -fno-strict-aliasing
-fwrapv-fexcess-precision=standard -D_GNU_SOURCE -Wmissing-prototypes -Wpointer-arith -Werror=vla -Wendif-labels
-Wmissing-format-attribute-Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security
-Wdeclaration-after-statement-Wno-format-truncation -Wno-stringop-truncation -pthread 


>> I did manage to test this stuff on bleeding-edge Fedora,
>> but ...

> Yea, I worked a fair bit to avoid requiring a too new version, I'll try
> to figure out what went wrong.  I did built on rhel8 not long ago, so I
> suspect it's a corner case somewhere.

Oh, interesting.  Let me know if you want me to test anything in
particular.

            regards, tom lane

Hi,

On 2023-02-01 09:49:00 -0800, Andres Freund wrote:
> On 2023-02-01 12:23:27 -0500, Tom Lane wrote:
> > And the minimum version appears to be newer than RHEL8's 1.8.2, which
> > I find pretty unfortunate.  On RHEL8, it fails with
> > $ ninja
> > ninja: error: build.ninja:6771: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing
listif it affects you
 
> 
> What's in that line +- 2 lines?  And/or what are the steps that got you
> to that point?
> 
> I'll try building 1.8.2 and reproing.
> 
> 
> > I did manage to test this stuff on bleeding-edge Fedora,
> > but ...
> 
> Yea, I worked a fair bit to avoid requiring a too new version, I'll try
> to figure out what went wrong.  I did built on rhel8 not long ago, so I
> suspect it's a corner case somewhere.

Unfortunately the test script accidentally pulled in ninja from epel,
hence not noticing the issue.


There's three issues:

One is easy enough, albeit slightly annoying: 1.8.2 wants the
"depending" file only be named once in a dependency file. Slightly
uglier code in snowball_create.pl, but whatever.

The second is one case of multiple outputs with a depfile:
create_help.pl creates both sql_help.c and sql_help.h. Not immediately
sure what a good solution here is. The brute force solution would be to
invoke it twice, but I don't like that at all.

The last case is the various man directories. That'd be easy enough to
avoid if we generated them inside a man/ directory.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2023-02-01 09:49:00 -0800, Andres Freund wrote:
>> On 2023-02-01 12:23:27 -0500, Tom Lane wrote:
>>> And the minimum version appears to be newer than RHEL8's 1.8.2, which
>>> I find pretty unfortunate.

> Unfortunately the test script accidentally pulled in ninja from epel,
> hence not noticing the issue.

Ah.  For myself, pulling the newer version from epel would not be a big
problem.  I think what we need to do is figure out what is the minimum
ninja version we want to support, and then see if we need to make any
of these changes.  I don't have hard data on which distros have which
versions of ninja, but surely somebody checked that at some point?

            regards, tom lane

Hi,

Here are my two cents.

> the minimum version appears to be newer than RHEL8's 1.8.2,
> which I find pretty unfortunate.  On RHEL8, it fails with

> $ ninja
> ninja: error: build.ninja:6771: multiple outputs aren't (yet?) supported by depslog; bring this up on the mailing
listif it affects you
 

> [...]  I don't have hard data on which distros have which
> versions of ninja, but surely somebody checked that at some point?

I'm using three different systems at the moment and the minimum
version of Ninja that is known to work is 1.10.1.

> Normally the ninja version that's pulled in by meson should suffice.

There are several ways to install Meson one of which, if you want the
latest version, is just using PIP:

```
pip3 install --user meson
```

Naturally Ninja will not be pulled in this case.

-- 
Best regards,
Aleksander Alekseev

Hi,

On 2023-02-01 14:20:19 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2023-02-01 09:49:00 -0800, Andres Freund wrote:
> >> On 2023-02-01 12:23:27 -0500, Tom Lane wrote:
> >>> And the minimum version appears to be newer than RHEL8's 1.8.2, which
> >>> I find pretty unfortunate.
> 
> > Unfortunately the test script accidentally pulled in ninja from epel,
> > hence not noticing the issue.
> 
> Ah.  For myself, pulling the newer version from epel would not be a big
> problem.  I think what we need to do is figure out what is the minimum
> ninja version we want to support, and then see if we need to make any
> of these changes.  I don't have hard data on which distros have which
> versions of ninja, but surely somebody checked that at some point?

I did survey available meson versions, and chose what features to
use. But not really ninja, since I didn't know about this specific issue
and other than this the ninja version differences were handled by meson.

As all the issues are related to more precise dependencies, I somehwat
wonder if it'd be good enough to use less accurate dependencies with
1.8.2. But I don't like it.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> I did survey available meson versions, and chose what features to
> use. But not really ninja, since I didn't know about this specific issue
> and other than this the ninja version differences were handled by meson.

> As all the issues are related to more precise dependencies, I somehwat
> wonder if it'd be good enough to use less accurate dependencies with
> 1.8.2. But I don't like it.

Nah, I don't like that either.  I did a crude survey of ninja's version
history by seeing which version is in each recent Fedora release:

f20/ninja-build.spec:Version:        1.4.0
f21/ninja-build.spec:Version:        1.5.1
f22/ninja-build.spec:Version:        1.5.3
f23/ninja-build.spec:Version:        1.7.1
f24/ninja-build.spec:Version:        1.7.2
f25/ninja-build.spec:Version:        1.8.2
f26/ninja-build.spec:Version:        1.8.2
f27/ninja-build.spec:Version:        1.8.2
f28/ninja-build.spec:Version:        1.8.2
f29/ninja-build.spec:Version:        1.8.2
f30/ninja-build.spec:Version:        1.9.0
f31/ninja-build.spec:Version:        1.10.1
f32/ninja-build.spec:Version:        1.10.1
f33/ninja-build.spec:Version:        1.10.2
f34/ninja-build.spec:Version:        1.10.2
f35/ninja-build.spec:Version:        1.10.2
f36/ninja-build.spec:Version:        1.10.2
f37/ninja-build.spec:Version:        1.10.2
rawhide/ninja-build.spec:Version:        1.11.1

Remembering that Fedora has a six-month release cycle, this shows that
1.8.2 was around for awhile but 1.9.x was a real flash-in-the-pan.
We can probably get away with saying that you need 1.10 or newer.
That's already three-plus years old.

            regards, tom lane

I pushed the discussed documentation improvements, and changed the
behavior of "ninja docs" to only build the HTML docs.  However,
I've not done anything about documenting what is the minimum
ninja version.

            regards, tom lane

Hi,

On 2023-02-08 17:18:13 -0500, Tom Lane wrote:
> However, I've not done anything about documenting what is the minimum ninja
> version.

Sorry, plan to tackle work around this tomorrow. Got stuck for much longer
than I had hoped to debug flapping tests (parts resolved, several others not).

My next step is to survey ninja versions across OSs / versions.

Greetings,

Andres Freund

On 08.02.23 23:18, Tom Lane wrote:
> I pushed the discussed documentation improvements, and changed the
> behavior of "ninja docs" to only build the HTML docs.

I don't like this change.  Now the default set of docs is different 
between the make builds and the meson builds.  And people will be less 
likely to make sure the man pages still build.

What's wrong with just typing "ninja html"?

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 08.02.23 23:18, Tom Lane wrote:
>> I pushed the discussed documentation improvements, and changed the
>> behavior of "ninja docs" to only build the HTML docs.

> I don't like this change.  Now the default set of docs is different 
> between the make builds and the meson builds.  And people will be less 
> likely to make sure the man pages still build.

What?  The default behavior of "make" has been to build only the
html docs for many years.  And I've never ever seen a case where
the html docs build and the man pages don't.

> What's wrong with just typing "ninja html"?

Don't really care how the command is spelled, but there needs to
be a convenient way to get that behavior.

            regards, tom lane

Hi,

On 2023-02-09 09:57:42 -0500, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> > On 08.02.23 23:18, Tom Lane wrote:
> >> I pushed the discussed documentation improvements, and changed the
> >> behavior of "ninja docs" to only build the HTML docs.
>
> > I don't like this change.  Now the default set of docs is different
> > between the make builds and the meson builds.  And people will be less
> > likely to make sure the man pages still build.
>
> What?  The default behavior of "make" has been to build only the
> html docs for many years.  And I've never ever seen a case where
> the html docs build and the man pages don't.

I think this misunderstanding is again due to the confusion between the 'all'
target in doc/src/sgml and the default target, just like earlier in the thread
/ why I ended up with the prior set of targets under 'docs'.

  # Make "html" the default target, since that is what most people tend
  # to want to use.
  html:
  ...
  all: html man


Given the repeated confusion from that, among fairly senior hackers, perhaps
we ought to at least put those lines next to each other? It's certainly not
obvious as-is.



> > What's wrong with just typing "ninja html"?
>
> Don't really care how the command is spelled, but there needs to
> be a convenient way to get that behavior.

Perhaps we should have doc-html, doc-man, doc-all or such?

The shell autocompletions for ninja work pretty well for me, a prefix like
that would make it easier to discover such "sub"-targets.


I'm was pondering adding a 'help' target that shows important targets.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> I think this misunderstanding is again due to the confusion between the 'all'
> target in doc/src/sgml and the default target, just like earlier in the thread
> / why I ended up with the prior set of targets under 'docs'.

>   # Make "html" the default target, since that is what most people tend
>   # to want to use.
>   html:
>   ...
>   all: html man

> Given the repeated confusion from that, among fairly senior hackers, perhaps
> we ought to at least put those lines next to each other? It's certainly not
> obvious as-is.

I think there are ordering constraints between these and the
Makefile.global inclusion.  But we could add a comment beside the "all:"
line pointing out that that's not the default target.

> Perhaps we should have doc-html, doc-man, doc-all or such?

No objection here.

If we intend to someday build tarballs with meson, there'd need to be
a target that builds html+man, but that could perhaps be named
"distprep".

            regards, tom lane

Hi,

On 2023-02-09 13:48:46 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I think this misunderstanding is again due to the confusion between the 'all'
> > target in doc/src/sgml and the default target, just like earlier in the thread
> > / why I ended up with the prior set of targets under 'docs'.
> 
> >   # Make "html" the default target, since that is what most people tend
> >   # to want to use.
> >   html:
> >   ...
> >   all: html man
> 
> > Given the repeated confusion from that, among fairly senior hackers, perhaps
> > we ought to at least put those lines next to each other? It's certainly not
> > obvious as-is.
> 
> I think there are ordering constraints between these and the
> Makefile.global inclusion.  But we could add a comment beside the "all:"
> line pointing out that that's not the default target.

Yes, html: has to happen before the inclusion of Makefile.global to become the
default target, but afaics we can just move "all: html man" up?


> If we intend to someday build tarballs with meson, there'd need to be
> a target that builds html+man, but that could perhaps be named
> "distprep".

Yea, a distprep target just depending on all the required targets seems to be
the way to go for that.


Not really related: I think we should seriously consider removing most of the
things distprep includes in the tarball. I'd leave docs in though. But IMO all
of the generated code doesn't make sense in this day and age. I guess that's a
discussion for a different thread and a different day.

Greetings,

Andres Freund

Hi,

On 2023-02-05 16:52:07 -0800, Andres Freund wrote:
> I did survey available meson versions, and chose what features to
> use. But not really ninja, since I didn't know about this specific issue
> and other than this the ninja version differences were handled by meson.

RHEL world confuses me a fair bit. And it seems to have gotten painful to even
get a realistic RHEL-like setup.

RHEL7 epel has ninja 1.10
RHEL8 code ready builder has ninja 1.8
RHEL8 epel does not have ninja
RHEL9 code ready builder has ninja 1.10

So actually RHEL8 doesn't suffice without something external, but RHEL7 + epel
does. Huh.


As pointed out by Aleksander downthread, it's easy to build on RHEL8 if you're
ok using pip, it's just "pip3.6 install meson ninja".


I tried to compile an OS matrix for some relevant OSs / OS versions:

                OS                                        Currently
                Supported       OS Ver          Ninja Ver       Python Version  Meson Version   Sufficient

Debian          unoffical       10              1.8             3.7             0.49            n
Debian          y               11              1.10            3.9             0.56            y
Fedora          n               32              1.10            3.8             0.55            y
FreeBSD         y               12              1.11            3.9             1.0             y
NetBSD          y               8.2             1.11            3.9             0.62            y
OpenBSD         y               7.1             1.10            3.9             0.62            y
RHEL            y               7 + epel        1.10            3.6             0.55            y
RHEL            y               8 + crb         1.8             3.6             0.58            n
RHEL            y               9 + crb         1.10            3.9             0.58            y
Ubuntu          y               18.04           1.8             3.6             0.45            n
Ubuntu          y               20.04           1.10            3.8             0.53            n
Ubuntu          y               22.04           1.10            3.10            0.61            y
openSUSE Leap   y               15.3            1.10            3.6             0.54            y

The only not sufficient ones that bother me to some degree are Ubuntu 20.04
and RHEL 8. The issues are different, oddly enough. Ubuntu has a new enough
ninja, but meson is too old, RHEL has it the other way around.

Greetings,

Andres

Andres Freund <andres@anarazel.de> writes:
> The only not sufficient ones that bother me to some degree are Ubuntu 20.04
> and RHEL 8. The issues are different, oddly enough. Ubuntu has a new enough
> ninja, but meson is too old, RHEL has it the other way around.

Yeah.  Well, we were intending to maintain the autoconf build system for
several years more anyway.  Guess we have to plan on keeping it going
until those platforms are EOL or nearly so.

            regards, tom lane

Hi,

On February 9, 2023 8:45:20 PM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>Andres Freund <andres@anarazel.de> writes:
>> The only not sufficient ones that bother me to some degree are Ubuntu 20.04
>> and RHEL 8. The issues are different, oddly enough. Ubuntu has a new enough
>> ninja, but meson is too old, RHEL has it the other way around.
>
>Yeah.  Well, we were intending to maintain the autoconf build system for
>several years more anyway.  Guess we have to plan on keeping it going
>until those platforms are EOL or nearly so.

Both could be supported with a bit of effort, fwiw. I don't know if it's worth doing so though...

Regards,

Andres

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Andres Freund <andres@anarazel.de> writes:
> On February 9, 2023 8:45:20 PM PST, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah.  Well, we were intending to maintain the autoconf build system for
>> several years more anyway.  Guess we have to plan on keeping it going
>> until those platforms are EOL or nearly so.

> Both could be supported with a bit of effort, fwiw. I don't know if it's worth doing so though...

It's probably not the highest-priority thing to be hacking on, on the
whole.  If we get to the point where we're itching to drop autoconf
and old-platform compatibility is the last thing holding us back,
maybe have a go at it then.

            regards, tom lane

Hi,

On Wed, Feb 08, 2023 at 05:18:13PM -0500, Tom Lane wrote:
> I pushed the discussed documentation improvements, and changed the
> behavior of "ninja docs" to only build the HTML docs.  However,
> I've not done anything about documenting what is the minimum
> ninja version.

FTR the documented XML_CATALOG_FILES environment variable is only valid for
Intel based machines, as homebrew installs everything in a different location
for M1...

I'm attaching a patch to make that distinction, hoping that no one else will
have to waste time trying to figure out how to get it working on such hardware.

> On 27 Mar 2023, at 10:24, Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> Hi,
>
> On Wed, Feb 08, 2023 at 05:18:13PM -0500, Tom Lane wrote:
>> I pushed the discussed documentation improvements, and changed the
>> behavior of "ninja docs" to only build the HTML docs.  However,
>> I've not done anything about documenting what is the minimum
>> ninja version.
>
> FTR the documented XML_CATALOG_FILES environment variable is only valid for
> Intel based machines, as homebrew installs everything in a different location
> for M1...
>
> I'm attaching a patch to make that distinction, hoping that no one else will
> have to waste time trying to figure out how to get it working on such hardware.

LGTM apart from the double // in the export which is easy enough to fix before
pushing.

+export XML_CATALOG_FILES=/opt/homebrew//etc/xml/catalog

For reference on why Homebrew use a different structure on Apple M1 the below
issue has more details:

    https://github.com/Homebrew/brew/issues/9177

--
Daniel Gustafsson

On Mon, Mar 27, 2023 at 10:32:52AM +0200, Daniel Gustafsson wrote:
> > On 27 Mar 2023, at 10:24, Julien Rouhaud <rjuju123@gmail.com> wrote:
> >
> > Hi,
> >
> > On Wed, Feb 08, 2023 at 05:18:13PM -0500, Tom Lane wrote:
> >> I pushed the discussed documentation improvements, and changed the
> >> behavior of "ninja docs" to only build the HTML docs.  However,
> >> I've not done anything about documenting what is the minimum
> >> ninja version.
> >
> > FTR the documented XML_CATALOG_FILES environment variable is only valid for
> > Intel based machines, as homebrew installs everything in a different location
> > for M1...
> >
> > I'm attaching a patch to make that distinction, hoping that no one else will
> > have to waste time trying to figure out how to get it working on such hardware.
>
> LGTM apart from the double // in the export which is easy enough to fix before
> pushing.
>
> +export XML_CATALOG_FILES=/opt/homebrew//etc/xml/catalog

Oh, I didn't notice it.  Apparently apple's find isn't smart enough to trim a /
when fed with a directory with a trailing /

> For reference on why Homebrew use a different structure on Apple M1 the below
> issue has more details:
>
>     https://github.com/Homebrew/brew/issues/9177

Ah I was wondering why, thanks!

> On 27 Mar 2023, at 10:41, Julien Rouhaud <rjuju123@gmail.com> wrote:
> On Mon, Mar 27, 2023 at 10:32:52AM +0200, Daniel Gustafsson wrote:
>>> On 27 Mar 2023, at 10:24, Julien Rouhaud <rjuju123@gmail.com> wrote:

>>> FTR the documented XML_CATALOG_FILES environment variable is only valid for
>>> Intel based machines, as homebrew installs everything in a different location
>>> for M1...
>>>
>>> I'm attaching a patch to make that distinction, hoping that no one else will
>>> have to waste time trying to figure out how to get it working on such hardware.
>>
>> LGTM apart from the double // in the export which is easy enough to fix before
>> pushing.
>>
>> +export XML_CATALOG_FILES=/opt/homebrew//etc/xml/catalog
>
> Oh, I didn't notice it.  Apparently apple's find isn't smart enough to trim a /
> when fed with a directory with a trailing /

Applied with a tiny but of changes to make it look like the rest of the
paragraph more. Thanks!

--
Daniel Gustafsson

Daniel Gustafsson <daniel@yesql.se> writes:

>> On 27 Mar 2023, at 10:41, Julien Rouhaud <rjuju123@gmail.com> wrote:
>> On Mon, Mar 27, 2023 at 10:32:52AM +0200, Daniel Gustafsson wrote:
>>>> On 27 Mar 2023, at 10:24, Julien Rouhaud <rjuju123@gmail.com> wrote:
>
>>>> FTR the documented XML_CATALOG_FILES environment variable is only valid for
>>>> Intel based machines, as homebrew installs everything in a different location
>>>> for M1...
>>>> 
>>>> I'm attaching a patch to make that distinction, hoping that no one else will
>>>> have to waste time trying to figure out how to get it working on such hardware.
>>> 
>>> LGTM apart from the double // in the export which is easy enough to fix before
>>> pushing.
>>> 
>>> +export XML_CATALOG_FILES=/opt/homebrew//etc/xml/catalog
>> 
>> Oh, I didn't notice it.  Apparently apple's find isn't smart enough to trim a /
>> when fed with a directory with a trailing /
>
> Applied with a tiny but of changes to make it look like the rest of the
> paragraph more. Thanks!

Doesn't this apply to Apple Silicon generally, not just M1? M2 already
exists, and M3 etc. will presumably also appear at some point. The
linked Homebrew issue refers to Apple Silicon, not any specific models.

- ilmari

> On 27 Mar 2023, at 14:04, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
> Daniel Gustafsson <daniel@yesql.se> writes:

>> Applied with a tiny but of changes to make it look like the rest of the
>> paragraph more. Thanks!
>
> Doesn't this apply to Apple Silicon generally, not just M1? M2 already
> exists, and M3 etc. will presumably also appear at some point. The
> linked Homebrew issue refers to Apple Silicon, not any specific models.

Thats a good point, it should say Apple Silicon and not M1 specifically.
Thanks, I'll go fix.

--
Daniel Gustafsson

On Mon, Mar 27, 2023 at 02:06:34PM +0200, Daniel Gustafsson wrote:
> > On 27 Mar 2023, at 14:04, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
> > Daniel Gustafsson <daniel@yesql.se> writes:
>
> >> Applied with a tiny but of changes to make it look like the rest of the
> >> paragraph more. Thanks!
> >
> > Doesn't this apply to Apple Silicon generally, not just M1? M2 already
> > exists, and M3 etc. will presumably also appear at some point. The
> > linked Homebrew issue refers to Apple Silicon, not any specific models.
>
> Thats a good point, it should say Apple Silicon and not M1 specifically.
> Thanks, I'll go fix.

Ah indeed that's a good point.  Thanks for pushing and fixing!

Julien Rouhaud <rjuju123@gmail.com> writes:
> On Mon, Mar 27, 2023 at 02:06:34PM +0200, Daniel Gustafsson wrote:
> On 27 Mar 2023, at 14:04, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>>> Doesn't this apply to Apple Silicon generally, not just M1? M2 already
>>> exists, and M3 etc. will presumably also appear at some point. The
>>> linked Homebrew issue refers to Apple Silicon, not any specific models.

>> Thats a good point, it should say Apple Silicon and not M1 specifically.
>> Thanks, I'll go fix.

> Ah indeed that's a good point.  Thanks for pushing and fixing!

Also, this needs to be back-patched, as this same text appears in
the back branches.

            regards, tom lane

> On 27 Mar 2023, at 16:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Julien Rouhaud <rjuju123@gmail.com> writes:
>> On Mon, Mar 27, 2023 at 02:06:34PM +0200, Daniel Gustafsson wrote:
>> On 27 Mar 2023, at 14:04, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>>>> Doesn't this apply to Apple Silicon generally, not just M1? M2 already
>>>> exists, and M3 etc. will presumably also appear at some point. The
>>>> linked Homebrew issue refers to Apple Silicon, not any specific models.
>
>>> Thats a good point, it should say Apple Silicon and not M1 specifically.
>>> Thanks, I'll go fix.
>
>> Ah indeed that's a good point.  Thanks for pushing and fixing!
>
> Also, this needs to be back-patched, as this same text appears in
> the back branches.

Yeah, it’s on my TODO for tonight when I get back. Since I botched the first commit before I had prepped the
backbranchesI figured I’d give the second some time in case that needed an update as well. 

./daniel