Re: libc++ 10.0: textproc/groff

2021-01-07 Thread Christian Weisgerber
Ingo Schwarze:

> > In the groff port, we could add #include "config.h" to all files
> > that don't do so already and include .
> 
> I plan to prepare, test, and post patches for the groff port when i find
> time (which will not take more than two days), but if others beat me to
> it, i do not object to such paches being committed at once.

I gave this a try but ran into an obstacle with yacc(1), which puts

#include 
#include 

at the very start of the generated files.  There's no way to prepend
an #include "config.h" there, apart from post-processing yacc's
output with sed...

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: libc++ 10.0: textproc/groff

2021-01-07 Thread Ingo Schwarze
Hi Christian,

[textproc/groff maintainer speaking]

Christian Weisgerber wrote on Wed, Jan 06, 2021 at 05:24:22PM +0100:

> textproc/groff fails to build with libc++ 10.0.  groff blocks over
> a thousand package paths, so this needs to be fixed before we can
> switch to the newer libc++.
> 
> Analysis:
> 
> groff includes a copy of gnulib.  gnulib provides *.h wrappers for
> some standard header files.  Those wrappers are not standalone.
> They are intended to be included in a source file after "config.h"
> has been included.

This is correct.  I recently saw this statement explicitly confirmed
by members of the gnulib project, and it has also been supported by
gnulib documention for a long time.

The GNU troff project has been disregarding this documented requirement,
but i don't think disregarding it is intentional.

> In particular, gnulib has a wrapper for math.h, but not for stdlib.h.
> 
> In libc++ 10,  includes .  When building groff,
> this include directive picks up the gnulib math.h wrapper.  Since
>  is included from some source files that do not include
> the local config.h, the math.h wrapper will then error out:
>   error: "Please include config.h first."
> 
> Possible solutions:
> 
> Cherrypick the libc++ upstream patch that untangles the interactions
> between the two header files and stops stdlib.h from including math.h.
> https://github.com/llvm/llvm-project/commit/c490c5e81ac90cbf079c7cee18cd56171f1e27af
> That patch looks very sensible, but as the commit history shows
> there were problems related to "local submodule visibility" and I
> don't know what that is and how it affects us.

I do not object to doing that if people consider it sensible, but
i can't say whether or not it should be done.  What you say seems
to indicate it should be carefully considered rather than rushed.

> In the groff port, we could add #include "config.h" to all files
> that don't do so already and include .  This is relatively
> clean, but will touch a number of files.

I agree that this is clean and poses little risk.  I am certain that
it will quickly be picked up upstream because the GNU troff project
recently already discussed that "config.h" needs to be included in
more files for reasons resembling these but not identical to these,
and there was no opposition to including "config.h" in more files
when there are good reasons.

I plan to prepare, test, and post patches for the groff port when i find
time (which will not take more than two days), but if others beat me to
it, i do not object to such paches being committed at once.

After our port is patched, i'll also send patches upstream.

> In the groff port, we could alternatively copy FreeBSD's pragmatic
> solution and simply add #include "config.h" to the math.h wrapper.
> That's not correct in some larger gnulib sense, but should suffice
> for the purposes of the port.

That sounds dirtier to me for no good reason, and it seems harder to
judge whether that might cause unanticipated side effects.  Why not
use the cleaner solution that will also be committed upstream when
that is already known?

This dosn't mean that i'm trying to veto a temporary addition to
the math.h wrapper if you prefer that for some reason.

> Since the problem originates with gnulib, it could affect ports
> other than groff, too.  However, a cursory search of the FreeBSD
> ports tree shows only one other port with a gnulib math.in.h patch,
> and that one is unrelated.

Well, according to your analysis and according to what i know about
gnulib, it results from a combination of two factors:

 1. The trap that gnulib sets.
 2. GNU troff walking into the trap by violating the documented gnulib
requirement of including "gnulib.h" before standard headers.

The rarity of the problem - if confirmed - might result from not all
programs that use gnulib walking into trap.

> Suggestions?

Patch textproc/groff quickly, watch out for failures in other ports
after upgrading to libc++ 10.0 (if any other ports are overlooked having
the same problem, they are likely to be as easy to fix as groff) and
optionally consider, free from haste, whether additional patching of
gnulib itself is useful, needless, or even risky.

Yours,
  Ingo



libc++ 10.0: textproc/groff

2021-01-06 Thread Christian Weisgerber
textproc/groff fails to build with libc++ 10.0.  groff blocks over
a thousand package paths, so this needs to be fixed before we can
switch to the newer libc++.

Analysis:

groff includes a copy of gnulib.  gnulib provides *.h wrappers for
some standard header files.  Those wrappers are not standalone.
They are intended to be included in a source file after "config.h"
has been included.

In particular, gnulib has a wrapper for math.h, but not for stdlib.h.

In libc++ 10,  includes .  When building groff,
this include directive picks up the gnulib math.h wrapper.  Since
 is included from some source files that do not include
the local config.h, the math.h wrapper will then error out:
  error: "Please include config.h first."

Possible solutions:

Cherrypick the libc++ upstream patch that untangles the interactions
between the two header files and stops stdlib.h from including math.h.
https://github.com/llvm/llvm-project/commit/c490c5e81ac90cbf079c7cee18cd56171f1e27af
That patch looks very sensible, but as the commit history shows
there were problems related to "local submodule visibility" and I
don't know what that is and how it affects us.

In the groff port, we could add #include "config.h" to all files
that don't do so already and include .  This is relatively
clean, but will touch a number of files.

In the groff port, we could alternatively copy FreeBSD's pragmatic
solution and simply add #include "config.h" to the math.h wrapper.
That's not correct in some larger gnulib sense, but should suffice
for the purposes of the port.

Since the problem originates with gnulib, it could affect ports
other than groff, too.  However, a cursory search of the FreeBSD
ports tree shows only one other port with a gnulib math.in.h patch,
and that one is unrelated.

Suggestions?

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de