Re: [groff] 1.22.4.rc4 - Final RC before official 1.22.4

[Ingo Schwarze]

Hi Bertrand,

Bertrand Garrigues wrote on Sun, Dec 09, 2018 at 12:52:04AM +0100:
> On Sat, Dec 08 2018 at 08:06:11 PM, Ingo Schwarze <address@hidden> wrote:

>> On Solaris 10, Perl 5.8.4 is in use.  On that ancient version of Perl,
>> Deri's latest gropdf(1) endianness fix is ineffective because support
>> for "L<" appears to be broken in unpack(3p).
> [...]
>> I don't think we should attempt to work around that ancient Perl bug.
>> I mean, srsly, Perl 5.8.4 in 2018?
 
> I can't judge if it would be easy or not to work around this problem (I
> let Deri comment on that), but I agree that it's probably not worth
> spending too much time on it.

Answered in my other mail directly to Deri; it appears that thanks
to Ralph's insight, the fix is quite easy - and actually shortens
the source code by one whole byte!  :-)

>> But the following seems suspicious (on OpenBSD):
>>
>>    $ grep hypot ../configure
>>   gl_hypot_required=plain
>>     # Code from module hypot:
>>     gl_LIBOBJS="$gl_LIBOBJS hypot.$ac_objext"
>>
>>    $ grep hypot config.log config.status
>>   [ no output ]
>>
>>    $ find . -name '*hypot*'
>>   ./src/libs/libgroff/.deps/libgroff_a-hypot.Po
>>   ./src/libs/libgroff/libgroff_a-hypot.o
>>   ./lib/.deps/hypot.Po
>>
>> Nothing whatsoever is tested, and looking at src/libs/libgroff/hypot.cpp,
>> it is merely a completely trivial wrapper.
>>
>> So what is the gnulib "hypot" module supposed to do?
>> If it does nothing, why do you even use it?

> For most of its modules, gnulib provides a m4 macro to check if a given
> function is broken on the current system (here 'gnulib_m4/hypot.m4') and
> a replacement function (here 'gnulib/lib/hypot.c').  If gnulib sees that
> the hypot on your system is broken

My point is that with the current groff build system in git master HEAD,
no such test is done.  The code in gnulib_m4/hypot.m4 never gets used
for anything, it seems.  To confirm this, i just commented out the line

  double hypot(double, double);

in /usr/include/math.h, deleted the build/ directory, re-ran

  mkdir build
  cd build
  ../configure
  make

and that dies as follows:

    CXX      src/libs/libgroff/libgroff_a-hypot.o
  ../src/libs/libgroff/hypot.cpp:
    In function 'double groff_hypot(double, double)':
  ../src/libs/libgroff/hypot.cpp:25:
    error: 'hypot' was not declared in this scope
  *** Error 1 in . (Makefile:7241 'src/libs/libgroff/libgroff_a-hypot.o':
    @echo "  CXX     " src/libs/libgroff/libgroff_a-hypot.o;g++ -DHAVE_C...)
  *** Error 1 in /co/groff/build (Makefile:5453 'all')

The lack of hypot() in libm is *not* detected, the gnulib replacement
is *not* used, and the build just crashes.

> it will make you use its own function
> instead (the only thing is that you must include <config.h> first in the
> file that uses the function).  Otherwise (like on your system, or mine)
> nothing happens and you use directly the 'hypot' function of your
> system.  In 'configure' the replacement of the system's 'hypot' by
> gnulib's 'hypot' is done here:
> 
>    if test $REPLACE_HYPOT = 1; then

I sure hate autoconf because it is overengineered to the point of
absurdity and hence quite hard to understand and even harder to use
correctly.  But be that as it may - it seems that you didn't chant
all the magical incantations needed to make it work.

> As the 'GROFF_NEED_DECLARATION([hypot])' call was doing some (home-made)
> compatibility checks, probably that means some compatibility issues were
> found before on 'hypot'.  That's why I thought I would be safer to add
> gnulib's 'hypot' module.

I fully agree with getting rid of GROFF_NEED_DECLARATION(): it doesn't
work.  What i don't particularly like is using half of the gnulib hypot
module in a way that doesn't work either, merely failing in a different
way in different circumstances.

Let's either use it in some way that actually does something useful,
or else let's get rid of it altogether.

Yours,
  Ingo