bugCompact Disc Input and Control Library - Bugs: bug #54191, Error linking cdda-player

 
 

bug #54191: Error linking cdda-player

Submitter:  Stef <stefanbidi>
Submitted:  Tue 26 Jun 2018 10:34:58 PM UTC
   
 
Category:  utility program Severity:  5 - Average
Item Group:  cdda-player Status:  Fixed
Privacy:  Public Assigned to:  rocky
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 04 Jul 2018 01:29:11 AM UTC, comment #7: 

This is now merged into master.

Thanks for your work on this.

Rocky Bernstein <rocky>
Group administrator
Thu 28 Jun 2018 09:26:55 PM UTC, comment #6: 

Thanks. That works on ubuntu.

It is in the stef-cdda-lib-patch branch http://git.savannah.gnu.org/cgit/libcdio.git/log/?h=stef-cdda-lib-patch

I'll give it a couple of days for review before merging it into master.

Rocky Bernstein <rocky>
Group administrator
Thu 28 Jun 2018 09:18:08 PM UTC, comment #5: 

OK, I noticed you were using PKG_CHECK_MODULES in other places, so I decided to take that approach. It works for me, so hopefully it will work for everyone.

It sets CDDA_PLAYER_LIBS to the output of pkg-config --libs ncurses. If that fails, it searches for only for the keypad() function in curses.

I made a few assumptions in order to simplify the test:

  • If the development package for ncurses is installed, ncurses.pc will exist;
  • The keypad function must exist in the libraries listed in ncurses.pc;
  • If ncurses.pc does not exist, then the only other option is -lcurses.


Hopefully, those are reasonable enough assumptions.

(file #44454)

Stef <stefanbidi>
Thu 28 Jun 2018 08:49:22 PM UTC, comment #4: 

Ok. Would you work up a new patch, or work off the git branch. I tried something like what you suggested but I must have made a mistake somewhere.

Rocky Bernstein <rocky>
Group administrator
Thu 28 Jun 2018 08:39:55 PM UTC, comment #3: 

Thanks for looking into it!

I see how using ac_cv_search_keypad was a bad idea. The issue with using LIBS in the way you committed it will be that now CDDA_PLAYER_LIBS will contain -lncurses twice + whatever LIBS have been checked for before (LIBS contains every library tested by AC_CHECK_LIB or AC_SEARCH_LIBS). It's possible that CDDA_PLAYER_LIBS will be set to "-lncurses -lsocket -lnsl -lncurses -ltinfo" (those are the other cases in configure.ac where AC_CHECK_LIB is used before this line).

I was essentially trying to find a way to pull only the "-ltinfo" or "". I do not think autoconf supports doing that. Maybe something like this would work?:
if test "x$ac_cv_search_keypad" != "xnone required"; then
  CDDA_PLAYER_LIBS="$CDDA_PLAYER_LIBS $ac_cv_search_keypad"
fi

Stef <stefanbidi>
Thu 28 Jun 2018 08:22:59 PM UTC, comment #2: 

I've committed this in branch step-cdda-lib-patch.
 See http://git.savannah.gnu.org/cgit/libcdio.git/log/?h=step-cdda-lib-patch

Please make sure this still works for you. I'll also request others to look at it before we merge it into the master branch.

There was one small change in using $ac_cv_search_keypad. First, I think $ac_ variables are not supposed to be used. More to the point in my case the value was coming out "none required".

It looks like $LIBS is the variable you are supposed to use to add additonal libraries. See https://autotools.io/autoconf/finding.html :

> action-if-found, action-if-not-found
> As usual, the macro supports expanding code for success and
> failure. In this instance, each will be called at most once,
> and the default action-if-found code, adding the library to
> the LIBS variables, is always executed, even if a parameter is passed.

Rocky Bernstein <rocky>
Group administrator
Thu 28 Jun 2018 07:10:34 PM UTC, comment #1: 

I came up with a patch that I believe will work for everyone.

It uses AC_SEARCH_LIBS, instead of AC_CHECK_LIB. So, if keypad() is defined in -lncurses or -lcurses, it does exactly what it does now. However, if -ltinfo is needed, it is added to CDDA_PLAYER_LIBS along with the curses library.

(file #44453)

Stef <stefanbidi>
Tue 26 Jun 2018 10:34:58 PM UTC, original submission:  

I get the following error when building libcdio 2.0.0:
/lib/libtinfo.so.6: error adding symbols: DSO missing from command line

The error stems from the fact that the configure script does not pass -ltinfo to the linker (I can fix the error by manually adding -ltinfo to CDDA_PLAYER_LIBS). I reviewed configure.ac, and noticed that the tests for the keypad() function are actually incorrect. First, configure sets HAVE_KEYPAD to an empty value. Then, it checks for mvprintw in ncurses, which in my system returns true and sets CDDA_PLAYER_LIBS="$CDDA_PLAYER_LIBS -lncurses". After that it does AC_CHECK_LIB($LIBCURSES, keypad, [HAVE_KEYPAD=yes]), which does not have the desired side effect. In config.h I have the following:
#define HAVE_KEYPAD /**/

I believe that either the configure script needs to check for libtinfo, or something like PKG_CONFIG_MODULES should be used to find out what to link. On my system, for example, `pkg-config --libs ncurses` returns "-lncurses -ltinfo".

Stef <stefanbidi>

 

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

Attach Files:
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rocky (Updated the item)
  • -email is unavailable- added by stefanbidi (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-07-04 rocky StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2018-06-28 stefanbidi Attached File- Added libcdio-2.0.0-add_tinfo_v2.patch, #44454
    2018-06-28 rocky StatusNone In Progress
        Assigned toNone rocky
    2018-06-28 stefanbidi Attached File- Added libcdio-2.0.0-add_tinfo.patch, #44453
    2018-06-26 rocky Item GroupNone cdda-player

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code