bugGNU Octave - Bugs: bug #55447, mkoctfile: wrong LFLAGS warning,...

 
 

bug #55447: mkoctfile: wrong LFLAGS warning, resets LDFLAGS

Submitted by:  Olaf Till <i7tiol>
Submitted on:  Fri 11 Jan 2019 02:39:46 PM UTC  
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Olaf Till Open/Closed:  Closed
Release:  5.0.1 Operating System:  GNU/Linux

Add a New Comment (Rich Markup)
   

You are not logged in

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

 

( Jump to the original submission)

Sun 10 Mar 2019 09:35:51 PM UTC, comment #18: 

The output you're seeing looks correct to me.

I don't know much about how swig uses Octave, but I guess that they are using "mkoctfile -p" only, and running the compiler themselves? If that's the case, swig has two options now with Octave 5:

  • use "pkg-config --libs octinterp"
  • use "mkoctfile -p OCTLIBDIR"

The pkg-config options is brand new in Octave 5. The latter OCTLIBDIR option is backwards compatible going back to Octave 3.x.

Mike Miller <mtmiller>
Project Administrator
Sun 10 Mar 2019 06:33:43 PM UTC, comment #17: 

I hope I'm not muddying things by posting here instead of creating a new bug, but while update the Fedora octave package to 5.1 I'm hitting the following issue.  With octave 4.4:

# mkoctfile -p LFLAGS
-L/usr/lib64/octave/4.4.1 -L/usr/lib64
# mkoctfile -p LDFLAGS
-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld

The libraries are installed in /usr/lib64/octave/4.4.1 which seems to be the default.  -L/usr/lib64 is superfluous but meh.

With octave 5.1.0:

# mkoctfile -p LFLAGS
warning: LFLAGS is deprecated and will be removed in a future version of Octave, use LDFLAGS instead
 -L/usr/lib64
# mkoctfile -p LDFLAGS
-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld

We are not setting LFLAGS anywhere in our build.  The only mention of LFLAGS in the build logs is:

checking whether yytext is a pointer... no
defining LFLAGS to be -I
checking for bison... bison -y

The missing -L/usr/lib64/octave/5.1.0 from LFLAGS is breaking the building of swig which uses mkoctfile -p LFLAGS to get the location of the libraries.  And if LDLAGS is to replace LFLAGS, shouldn't LDFLAGS return the -L options?

Orion Poplawski <opoplawski>
Thu 24 Jan 2019 05:53:53 PM UTC, comment #16: 

I also believe compound options such as -framework are working.  I believe the most straigthforward course now is to start producing release candidates so that we can get user feedback on multiple build systems.

Rik <rik5>
Project Administrator
Thu 24 Jan 2019 05:12:58 PM UTC, comment #15: 

We can work out those issues over the next two releases, we have until Octave 7 to get it right and add better support for adding library or framework options if something is lost here.

FTR, compound options like "-framework vecLib" should be recognized by mkoctfile now, since version 4.4.

In version 5, LFLAGS will still work exactly as it did before, users can continue to use it, they will just see a clear deprecation warning, and hopefully come to us to work out a solution.

The main point of this change was that the variable name LFLAGS is a complete misnomer and shouldn't be used anymore. We are intending to migrate users away from LFLAGS, even if it means we have to add a new variable to serve the same purpose.

Mike Miller <mtmiller>
Project Administrator
Thu 24 Jan 2019 04:13:23 PM UTC, comment #14: 

Sorry, forget my last comment regarding lapack libs, it was nonsense... I should not post while travelling in a train...

Olaf Till <i7tiol>
Project Member
Thu 24 Jan 2019 04:08:30 PM UTC, comment #13: 

Furthermore, in optim, lapack is accessed directly, some symbols of it which are not accessed by Octave. But it is crucial that the same lapack library is linked with optim as with Octave. (Because some tolerances are requested from lapack by optim, and these may change with the algorithm used by lapack.)

The information on which lapack library Octave is linked with was formerly contained in LFLAGS. I don't see how to get this information after LFLAGS is dropped. This may make an important feature of optim unavailable...(?)

Olaf Till <i7tiol>
Project Member
Thu 24 Jan 2019 02:46:27 PM UTC, comment #12: 

In some OF packages, the decision had been made to pass flags naming libraries in LFLAGS, since mkoctfile doesn't understand non-standard flags like -framework (corresponding, I believe, to -l and -include together) on its command-line. There were reports that Apple systems couldn't build these packages before this decision.

Olaf Till <i7tiol>
Project Member
Wed 23 Jan 2019 10:46:49 PM UTC, comment #11: 

I'm sure there's a whole bunch of people stuffing '-lLIB' in to their LDFLAGS, but I guess this will flush them out.

Rik <rik5>
Project Administrator
Wed 23 Jan 2019 10:43:04 PM UTC, comment #10: 

GNU make implicit rule looks like this

$(CXX) $(CXXFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $^ $(LOADLIBES) $(LDLIBS) -o $@

where '$^' are the source files, and notice the '-o $@' is at the end, the order of that doesn't really matter. 'LDLIBS' appears to be GNU make's equivalent of automake's 'LIBS' variable. It's documented in the GNU make manual, in particular the difference between 'LDFLAGS' and 'LDLIBS' is clearly described.

GNU automake default link rule looks like this

$(CXXLD) $(AM_CXXFLAGS)        $(CXXFLAGS) $(AM_LDFLAGS) $(LDFLAGS) -o $@ $(foo_OBJECTS) $(foo_LDADD) $(LIBS)

where 'foo_OBJECTS' is the automake variable derived from 'foo_SOURCES', and 'foo_LDADD' is a documented automake variable for that purpose.

If we do add a new variable, we can create a new one called either 'LIBS' or 'LDLIBS', with a default value of empty, as a placeholder for the user to add whatever library operands they want to the link command. We can add that now for Octave 5 if we agree that this would be useful.

Mike Miller <mtmiller>
Project Administrator
Wed 23 Jan 2019 10:34:09 PM UTC, comment #9: 

No, I believe the current order is correct.

First note that vars["LDFLAGS"] is already included in vars["ALL_LDFLAGS"] for the stand-alone link command. So appending it after vars["DL_LDFLAGS"] as I did here just makes sure that the two commands are applying the user LDFLAGS properly.

The stackoverflow question you point to is confusing LDFLAGS with LIBS or LDLIBS. I think a large part of the confusion is users who group -L options and -Wl options together with -l options. The first two are proper linker options, the -l "options" are actual operands. LDFLAGS should not contain -l arguments. But -l operands do need to come after the 'objfiles' argument.

It's possible there will be some transition pain if users are stuffing "-lfoo" operands into the LFLAGS variable. They should not start putting "-lfoo" operands into LDFLAGS. They should either be passed on the mkoctfile command line, or maybe we need to introduce an entirely new variable, such as "LIBS", for this purpose.

If you look at the GNU make built-in rules or the automake link commands you will see that LDFLAGS properly comes before any file arguments, just like CPPFLAGS and CXXFLAGS.

Mike Miller <mtmiller>
Project Administrator
Wed 23 Jan 2019 10:24:22 PM UTC, comment #8: 

@Mike: Could you check the order of the flags?  My understanding is that LDFLAGS needs to be after the output argument -o.

Here is an example taken from https://stackoverflow.com/questions/13249610/how-to-use-ldflags-in-makefile.

 $(CC) $(CFLAGS) $(OBJECTS) -o client $(LDFLAGS)

The ordinary link command is now:

std::string cmd
  = (vars["DL_LD"] + ' ' + vars["ALL_CXXFLAGS"] + ' '
     + vars["DL_LDFLAGS"] + ' ' + vars["LDFLAGS"] + ' ' + pass_on_options
     + " -o " + octfile + ' ' + objfiles + ' ' + libfiles + ' '
     + ldflags + ' ' + vars["LFLAGS"] + ' ' + octave_libs + ' '
     + vars["OCT_LINK_OPTS"] + ' ' + vars["OCT_LINK_DEPS"]);

I believe LDFLAGS should be placed after LFLAGS so that, if present, it overrides LFLAGS and ldflags.

Rik <rik5>
Project Administrator
Wed 23 Jan 2019 09:47:31 PM UTC, comment #7: 

I pushed a change on the stable branch that I think addresses this:

https://hg.savannah.gnu.org/hgweb/octave/rev/ab5cc8da283c

The correct value of LDFLAGS is restored and used in all builds. The LFLAGS value is still used in the build until it is removed. A warning is printed if the user explicitly calls 'mkoctfile -p LFLAGS' or if LFLAGS is overridden from the environment. This should help migrate users away from relying on LFLAGS and let it be deleted painlessly in a future version.

Mike Miller <mtmiller>
Project Administrator
Mon 14 Jan 2019 09:14:37 PM UTC, comment #6: 

@Mike: That shouldn't be hard to implement.  The harder decision was strategic.  If LDFLAGS will always override LFLAGS the code can be simplified.

Rik <rik5>
Project Administrator
Mon 14 Jan 2019 07:14:22 PM UTC, comment #5: 

I am going to try to make mkoctfile use LDFLAGS and apply it in the link stage without printing a warning.

IMHO, backwards compatibility means that if a user sets LFLAGS, we will still use it, but it doesn't mean that we have to also ignore LDFLAGS because we used to ignore it.

If a new user using Octave 5 for the first time uses LDFLAGS, it should do what they would expect without any warning or without any need to also set LFLAGS, that just seems like the obvious right thing to do.

So again IMHO,

  • user sets LDFLAGS only, mkoctfile uses LDFLAGS, no warning is printed
  • user sets LFLAGS only, mkoctfile uses LFLAGS, prints a warning about the variable being deprecated
  • user sets both LDFLAGS and LFLAGS, mkoctfile prints a warning, uses LDFLAGS, and either ignore or concatenate LFLAGS onto LDFLAGS

This is what I will try to get working.

Mike Miller <mtmiller>
Project Administrator
Mon 14 Jan 2019 07:53:53 AM UTC, comment #4: 

I understand... but then, the warning message in

  // FIXME: Remove LFLAGS in Octave 7.0
  if (vars["LFLAGS"] != vars["LDFLAGS"])
    {
      std::cerr << "warning: LFLAGS is deprecated and will be removed in a future version of Octave, use LDFLAGS instead" << std::endl;
      vars["LDFLAGS"] = vars["LFLAGS"];
    }

should be changed to something like:
"LDFLAGS is now used instead of the now deprecated LFLAGS. To avoid this warning, set both to the same value."

Can I hope that DL_LDFLAGS won't be removed?

Having said this, I've doubts if the current usage of LDFLAGS in place of the former LFLAGS is reasonable. Formerly, if I understand it correctly, LDFLAGS contained the LDFLAGS with which Octave had been linked. This information is not provided any more now at all.

Olaf Till <i7tiol>
Project Member
Fri 11 Jan 2019 10:19:08 PM UTC, comment #3: 

That is the problem with providing backwards compatibility on LFLAGS.  If no change had been made to mkoctfile then

env LDFLAGS=-Wl,--stats mkoctfile foo.cc

would result in LDFLAGS being ignored.

But, we now pay attention to LDFLAGS.  mkoctfile sees that, however bass-ackwards it is, the compilation today would not be the same as the one issued in Octave 4.4.  Because we are prioritizing backwards compatibility, the LDFLAGS settings needs to be removed so the compilation is identical.

But I support anyone who wants to make this more intelligent.

Rik <rik5>
Project Administrator
Fri 11 Jan 2019 07:23:21 PM UTC, comment #2: 

I also think this warning is overzealous and the logic is actually wrong now that I look at it. With the following

env LDFLAGS=-Wl,--stats mkoctfile foo.cc

The LDFLAGS options should be picked up and passed on to the linker. But when mkoctfile gets to the part of the code quoted by Olaf, it sees that LDFLAGS differs from the legacy LFLAGS and actually overwrites it, so my choice has now been erased, in addition to the warning message making no sense.

Mike Miller <mtmiller>
Project Administrator
Fri 11 Jan 2019 04:33:11 PM UTC, comment #1: 

The idea is actually to detect when compilation options have changed.  In the past, if you configured LDFLAGS they were ignored.  So if you never used LFLAGS, but had LDFLAGS set in the environment, you will now be using those flags and getting different compilation behavior. 

The solution for identical results could be 1) set LFLAGS = LDFLAGS, 2) unset LFLAGS and LDFLAGS and rely on defaults.  There might be a third solution.

Rik <rik5>
Project Administrator
Fri 11 Jan 2019 02:39:46 PM UTC, original submission:  

(tested with changeset 826d4c26e5e2 of 5.0.1 (2019-01-10))

The lines in src/mkoctfile.in.cc:

  if (vars["LFLAGS"] != vars["LDFLAGS"])
    {
      std::cerr << "warning: ...

warn if LDFLAGS is passed in the environment to mkoctfile. Surely it was meant to test if 'vars["LFLAGS"]' is non-empty.

Olaf Till <i7tiol>
Project Member

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by opoplawski (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by i7tiol (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-01-26 mtmiller StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
    2019-01-23 mtmiller StatusConfirmed => Ready For Test
    2019-01-11 mtmiller StatusNone => Confirmed
        Summarymkoctfile: wrong LFLAGS warning => mkoctfile: wrong LFLAGS warning, resets LDFLAGS

    Back to the top


    Powered by Savane 3.5