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

 
 

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

Submitter:  Ingo Schwarze <schwarze>
Submitted:  Mon 24 Jul 2017 08:09:23 AM UTC
   
 
Category:  General Severity:  5 - Blocker
Item Group:  Incorrect behaviour Status:  Fixed
Privacy:  Public Assigned to:  bgarrigues
Open/Closed:  Closed Planned Release:  1.22.4
* Mandatory Fields

Add a New Comment Rich Markup
   

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>
Group 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>
Group 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>
Group Member
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>
Group 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>
Group Member

 

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

Attach Files:
   
   
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
  • -email is unavailable- added by gbranden (Updated the item)
  • -email is unavailable- added by bgarrigues (Posted a comment)
  • -email is unavailable- added by wl (Posted a comment)
  • -email is unavailable- added by schwarze (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-05-02 gbranden CategoryNone General
        Assigned toNone bgarrigues
        Planned ReleaseNone 1.22.4
        Summary[patch] add missing libgnu.a in _LDADD in .am [PATCH] add missing libgnu.a in _LDADD in .am
    2017-08-15 bgarrigues Severity3 - Normal 5 - Blocker
        StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2017-07-25 wl StatusNeed Info Confirmed
    2017-07-24 wl StatusNone Need Info
    2017-07-24 schwarze Attached File- Added libgnu.patch, #41312

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code