bugGNU troff - Bugs: bug #51554, [patch] add missing libgnu.a in...

 
 

bug #51554: [patch] add missing libgnu.a in *_LDADD in *.am

Submitted by:  Ingo Schwarze <schwarze>
Submitted on:  Mon 24 Jul 2017 08:09:23 AM UTC  
 
Category: NoneSeverity: 5 - Blocker
Item Group: Incorrect behaviourStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: ClosedPlanned Release: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Tue 15 Aug 2017 09:30:48 PM UTC, comment #4:

I've tested Ingo's patch and it looks correct to me, commited on master.

Bertrand Garrigues <bgarrigues>
Project Administrator
Tue 25 Jul 2017 06:15:58 PM UTC, comment #3:

Sorry, I was mistaken – I thought that the `bootstrap' script takes care of adding `libgnu.a' automatically to the link chain.

So yes, your patch is OK.

Note that the gnulib implementation of fprintf is only used if the native implementation doesn't pass the related tests for POSIX conformity. Interestingly, the corresponding M4 file (`printf.m4') is by far the largest test of all gnulib modules.

Obviously, OpenBSD's implementation doesn't conform to POSIX – is this intentional?

Werner LEMBERG <wl>
Project Administrator
Tue 25 Jul 2017 01:37:42 PM UTC, comment #2:

Hi Werner, thanks for having a look. Your comment helped me to better understand what is going on, but i think what you suggest in comment #1 misses the point.

In https://savannah.gnu.org/bugs/?51330 , Bertrand added the gnulib fprintf-posix module to the groff gnulib configuration. If i understand correctly, the intention of that change is to replace the native fprintf(3) implementation of the operating system we are running on with the gnulib implementation, if the configuration system comes to the conclusion that - for whatever reason - it dislikes the native fprintf(3). In that case, all programs using fprintf(3) must link against gnulib, and that is what my patch "libgnu.patch" does.

I think that Bertrand's patch committed in https://savannah.gnu.org/bugs/?51330 cannot have been tested at all. It simply cannot build at all when the fprintf-posix check returns a negative result.

This doesn't have anything to do with any of the following questions:

- Whether gnulib improved anything in their detection or replacement code since February. (Anecdotically, adding --remote to all the "git submodule update" lines in bootstrap makes no difference, but that doesn't matter for the question at hand.)

- Whether gnulib tests for fprintf(3) are sensible or bogus, and whether the gnulib implementation of fprintf(3) may be correct or buggy.

- Whether the OpenBSD implementation of fprintf(3) is correct or buggy.

These are also potentially interesting questions (especially the last one), but they are unrelated to my patch.

All that said, i doubt the wisdom of replacing the fprintf(3) implementation (no matter what the circumstances). While there might be a theoretical reason to do that, i cannot remember ever having seen a system in practice where that is needed. But again, whether or not we want to use gnulib fprintf-posix is a separate question. IF we want to use it, then each and every program using fprintf(3) has to link against libgnu.a.

Ingo Schwarze <schwarze>
Mon 24 Jul 2017 06:07:48 PM UTC, comment #1:

Thanks for the patch, but I'm rather sure this is the wrong approach. Have you tried to update the gnulib submodule version in groff? Currently, it checks out commit 4084b3 from February; maybe it's necessary to update it so that it builds correctly on OpenBSD.

If that fails I ask you to contact the gnulib mailing list, reporting all the gory details of your failure; as soon as this is fixed upstream, it gets automatically corrected within groff by requesting a newer git submodule commit.

Werner LEMBERG <wl>
Project Administrator
Mon 24 Jul 2017 08:09:23 AM UTC, original submission:

I just tried to build groff from git -master on OpenBSD-current (which used to work in the past) and it failed miserably because it couldn't find rpl_fprintf() at link time. I need the following patch to get it to build, and i think the patch is correct because if you are using gnulib and the related autodetection glue globally, then all programs potentially picking that up must be linked against gnulib.

All the same, i only added lib/libgnu.a in those Makefile.ams where i saw actual build failures, such that you can be sure that i'm not touching anything that isn't broken.

That said, i have no idea what suddenly froze over in autohell such that it no longer likes the native fprintf(3) found in the OpenBSD libc, but i consider that unrelated to this patch.

While here, amend INSTALL.REPO to recommend the autohell versions Bertrand actually used for testing, rather than the minimum ones. After all, the whole point of these variables is to make the build as reliable and as well-tested as possible.

Ingo Schwarze <schwarze>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #41312:  libgnu.patch added by schwarze (8KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bgarrigues (Posted a comment)
  • -unavailable- added by wl (Posted a comment)
  • -unavailable- added by schwarze (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 15 Aug 2017 09:30:48 PM UTCbgarriguesSeverity3 - Normal=>5 - Blocker
      StatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Tue 25 Jul 2017 06:15:58 PM UTCwlStatusNeed Info=>Confirmed
    Mon 24 Jul 2017 06:07:48 PM UTCwlStatusNone=>Need Info
    Mon 24 Jul 2017 08:09:23 AM UTCschwarzeAttached File-=>Added libgnu.patch, #41312

    Back to the top


    Powered by Savane 3.1-cleanup1