bugGNU Octave - Bugs: bug #48678, mkoctfile returns linker flags on...

 
 

bug #48678: mkoctfile returns linker flags on LFLAGS instead of LDFLAGS

Submitter:  Carnë Draug <carandraug>
Submitted:  Tue 02 Aug 2016 01:15:32 AM UTC
   
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  3 - Low Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 10 Jan 2019 05:30:00 PM UTC, comment #24: 

Since Octave now provides a .pc file to be used with pkg-config, should the deprecationg be recommending users to replace 'mkoctfile -p LFLAGS' with 'pkg-config --libs-only-L octave' instead of 'mkoctfile -p LDFLAGS'?

Carnë Draug <carandraug>
Group Member
Fri 04 Jan 2019 04:19:14 PM UTC, comment #23: 

Yes, so confusing that I don't mind causing a little pain on the upgrade from 4.4 to 5.0 to sort this out.  Users can always choose not to upgrade if thy have really severe requirements for continuity.  For the rest, it will be a few warnings that require fixing and then we can be through with this issue.

Rik <rik5>
Group administrator
Fri 04 Jan 2019 09:27:24 AM UTC, comment #22: 

Hm, nevermind, I now see that the value of LDFLAGS from configure is also reflected in the OCT_LINK_OPTS variable which is already used in the mkoctfile link stage, so no problem. These variable names are very confusing and could use some more cleaning up, but I think we can call this one done.

Mike Miller <mtmiller>
Group Member
Fri 04 Jan 2019 07:51:18 AM UTC, comment #21: 

I think you lost something important in this change.

Previously, let's say I build Octave with LDFLAGS="-Wl,-z,relro". That option was encoded into mkoctfile as LDFLAGS, while LFLAGS was "-L/usr/lib/octave/4.4.1". Both of these options were added into the mkoctfile linker stage and both were important. The original value of LDFLAGS needs to be appended or prepended somewhere.

Also it looks like LDFLAGS is showing up twice in the link command, once as part of ALL_LDFLAGS and once as itself.

I'm finishing something else at the moment but I will try to spend some time on this.

Mike Miller <mtmiller>
Group Member
Fri 04 Jan 2019 05:45:10 AM UTC, comment #20: 

Fixed in this changeset (https://hg.savannah.gnu.org/hgweb/octave/rev/26be3dc2900f).  Closing report.

Rik <rik5>
Group administrator
Thu 03 Jan 2019 11:26:35 PM UTC, comment #19: 

Sure, with a warning and deprecation that seems fine to do for this release.

Affected Forge packages seem to include control, ltfat, odepkg, optim, optiminterp, and sparsersb, as well as the external (non-Octave Forge) packages dynare and libsbml.

Mike Miller <mtmiller>
Group Member
Tue 01 Jan 2019 04:06:40 PM UTC, comment #18: 

@Mike: The discussion stopped on this bug back in July.  Do we want to start using LDFLAGS as the proper name for linker flags in 5.0?  It seems like we probably need to deprecated LFLAGS over two releases as you pointed out that the Octave-Forge packages are making use of the variable.

Rik <rik5>
Group administrator
Thu 12 Jul 2018 10:52:47 PM UTC, comment #17: 

No doubt it has been widely used.  How should this tie in with deprecating octave_config?  Are we going to get the value for LFLAGS from pkg-config now?  If that is the case then we might be asking people to rewrite their Makefiles anyways in which case case we should ask them to also switch to grabbing LDFLAGS.

Rik <rik5>
Group administrator
Thu 12 Jul 2018 10:39:55 PM UTC, comment #16: 

Several Octave Forge packages prepend or append to the 'LFLAGS' variable specifically in order to modify the behavior of mkoctfile in their makefiles. For example in the control package


LFLAGS := $(shell $(MKOCTFILE) -p LFLAGS) $(LAPACK_LIBS) $(BLAS_LIBS) $(FLIBS)


If we think we can solve all of the packages and external projects using LFLAGS cleanly and quickly by the next release, then ok. But we might want to help other developers transition. In other words, this is not just an internal variable or something for users to query if they are curious, this is a somewhat widely used interface.

Mike Miller <mtmiller>
Group Member
Thu 12 Jul 2018 09:20:58 PM UTC, comment #15: 

@Mike: Is it even a good idea to keep LFLAGS for backward compatibility?  LFLAGS is supposed to be for lex flags.  Checking my Makefile I see


Makefile:2648:        $(LIBTOOLFLAGS) --mode=compile $(LEX) $(AM_LFLAGS) $(LFLAGS)


So far we don't seem to have had any problems, but it is terribly confusing.  I'm voting for just fixing it now with LDFLAGS representing linker flags.

Rik <rik5>
Group administrator
Thu 12 Jul 2018 08:09:26 PM UTC, comment #14: 

pkg-config is extensible, with the --variable option for example, so we could start dumping a whole bunch of build-time configuration variables into the pkg-config files as well. We could make things like 'pkg-config octave --variable=VERSION' or 'pkg-config octave --variable=BLAS_LIBS' work.

If we go in this direction, we could leave all of the default values encoded in pkg-config, not do any substitutions at all into mkoctfile, and have it rely on pkg-config for all of the default variables when building.

Or we could pick some balance or some overlap between mkoctfile and pkg-config.

As to this particular issue, I agree with Carnë, the variable naming scheme in mkoctfile is the core problem in this bug report. The standard name LDFLAGS should be used for options that modify the behavior of the link stage, like -L, -R, -rpath, or -Wl,foo, etc.

For now should we add a LDFLAGS variable to mkoctfile and retain LFLAGS as an alias for backwards compatibility?

Mike Miller <mtmiller>
Group Member
Thu 12 Jul 2018 11:26:40 AM UTC, comment #13: 

What should be in LFLAGS?

For now, I think we should just make sure that the outputs of pkg-config and mkoctfile agree.

Longer term, we could have mkoctfile call pkg-config to get the info it needs, and to handle the -p options.  However, it will take some time to eliminate the -p option from mkoctfile since it was recommended in the past and is widely used.

John W. Eaton <jwe>
Group administrator
Thu 12 Jul 2018 10:15:33 AM UTC, comment #12: 

I just gave it a go and the issue remains. That is, the variables LDFLAGS and LFLAGS still do not return what one would expect from their names.


$ mkoctfile -p LDFLAGS

$ mkoctfile -p LFLAGS
 -L/home/carandraug/.local/lib



>> But then the purpose of "mkoctfile -p" overlaps with the purpose of
>> "octave-config" and the newly reported bug about supporting
>> pkg-config. So why do we even need "mkoctfile -p"?
>
> The answer going forward appears to be that octave-config will be
> deprecated.


I have added support for pkg-config already with

http://hg.savannah.gnu.org/hgweb/octave/rev/26070f36f1e7

to replace octave-config. So if `mkoctfile -p` was overlapping with octave-config and was not needed, then I guess the same remains but for pkg-config files.

Carnë Draug <carandraug>
Group Member
Thu 12 Jul 2018 05:22:13 AM UTC, comment #11: 

Very recently, jwe modified mkoctfile to contain all of octave-config's variables


changeset:   25556:cbef9fc53fef
user:        John W. Eaton <jwe@octave.org>
date:        Fri Jul 06 19:20:40 2018 -0400
summary:     include all octave-config vars in mkoctfile (bug #54260)


This addresses the last question in comment #10,


But then the purpose of "mkoctfile -p" overlaps with the purpose of "octave-config" and the newly reported bug about supporting pkg-config. So why do we even need "mkoctfile -p"?


The answer going forward appears to be that octave-config will be deprecated.

I don't know how that influences the rest of this bug report.


Rik <rik5>
Group administrator
Sun 14 Aug 2016 03:57:17 PM UTC, comment #10: 

carandraug said:

> [...] therefore "-p" would be the flags that mkoctfile uses. [...] in my view that includes the flags required by Octave plus whatever the user has set in the environment if any.


I thought about this again last night and I'm not so sure if that's the most obvious behaviour after all.

Mike Miller said:

> Agree with jwe, I think it's important that mkoctfile treats CFLAGS, CPPFLAGS, CXXFLAGS, LDFLAGS, etc as user variables that can be overridden from the environment, and default to empty (in the case of LDFLAGS) or to a trivial default (in the case of CXXFLAGS="-g -O2").
>
> But do we want to append the built-in defaults to the user values and allow the full set of flags to be exported? Or would that be too confusing?


Yes, mkoctfile should account for XFLAGS set in the environment but it doesn't have to echo them back when using "mkoctfile -p".

If mkoctfile is called to make an oct file, then it should use env variables.  If it is called with "-p", then the user will be handling those flags and it is up to him to append any environment variables that it sees fit, and it will be up to him to ignore the environment variables as well. pkg-config also does this, it returns only the flags required and ignores environment variables.

But then the purpose of "mkoctfile -p" overlaps with the purpose of "octave-config" and the newly reported bug about supporting pkg-config.  So why do we even need "mkoctfile -p"?

Carnë Draug <carandraug>
Group Member
Sun 14 Aug 2016 02:01:06 PM UTC, comment #9: 

I don't see the reason to delay.  I find it awfully confusing that mkoctfile -p doesn't return the actual set of values that it uses internally to compile objects.

Rik <rik5>
Group administrator
Sat 13 Aug 2016 11:41:46 PM UTC, comment #8: 

jwe said on comment #7:

> That we provide octave-config and not a pkg-config file is really just historical (pkg-config wasn't around yet?). Switching to pkg-config and deprecating octave-config is probably worthy of a separate bug report/feature request.


New bug report opened as bug #48775

Should then fixing this be delayed until we provide a pkg-config file then?

Carnë Draug <carandraug>
Group Member
Sat 13 Aug 2016 11:20:03 PM UTC, comment #7: 

That we provide octave-config and not a pkg-config file is really just historical (pkg-config wasn't around yet?).  Switching to pkg-config and deprecating octave-config is probably worthy of a separate bug report/feature request.

John W. Eaton <jwe>
Group administrator
Sat 13 Aug 2016 11:09:05 PM UTC, comment #6: 

That explains the large set of flags.  I never quite understood why we had so many and why they looked kind of redundant.

I thought the purpose of mkoctfile was to build oct files and that therefore "-p" would be the flags that mkoctfile uses. Because mkoctfile should not ignore user environment variables, in my view that includes the flags required by Octave plus whatever the user has set in the environment if any.

If a user would want to get only the flags that Octave requires (instead of the flags that mkoctfile will use), then it should use octave-config (which I now realize doesn't really have any flags and is not an Octave specific version of pkg-config).

Carnë Draug <carandraug>
Group Member
Sat 13 Aug 2016 10:44:32 PM UTC, comment #5: 

Agree with jwe, I think it's important that mkoctfile treats CFLAGS, CPPFLAGS, CXXFLAGS, LDFLAGS, etc as user variables that can be overridden from the environment, and default to empty (in the case of LDFLAGS) or to a trivial default (in the case of CXXFLAGS="-g -O2").

But do we want to append the built-in defaults to the user values and allow the full set of flags to be exported? Or would that be too confusing?

For example could we combine the user's CPPFLAGS with what is currently called INCFLAGS, and export that from "mkoctfile -p CPPFLAGS"? Same with user's CXXFLAGS and XTRA_CXXFLAGS for "mkoctfile -p CXXFLAGS"?

Another enhancement might be to work on the naming convention of the built-in compiler/linker options. It usually takes me a few tries of guessing to figure out which of LFLAGS, OCTAVE_LIBS, DL_LDFLAGS, I need to export to build something.
It might be a nice enhancement to work on the naming scheme for exporting the default flags from mkoctfile.

Mike Miller <mtmiller>
Group Member
Sat 13 Aug 2016 10:27:10 PM UTC, comment #4: 

The LDFLAGS variable is normally empty and allows the user to add options by setting LDFLAGS in the environment.

It looks like the LFLAGS variable is just there to ensure that -L$(octlibdir) is included in the linker options when it is not a directory that is normally searched by the linker.  I suppose it is similar to AM_LDFLAGS in automake.  LFLAGS may not be the best name, but this job needs to be done in some way.  It's probably best to keep it separate from LDFLAGS so that if someone sets LDFLAGS in the environment, they don't have to also include -L/usr/local/octave/lib/.../whatever.  Users should be able to set LDFLAGS independently and still expect things to work.

John W. Eaton <jwe>
Group administrator
Sat 13 Aug 2016 02:52:15 PM UTC, comment #3: 

Adding jwe to the CC list.  I think this is still some sort of simple misunderstanding.

Rik <rik5>
Group administrator
Sat 13 Aug 2016 02:46:15 PM UTC, comment #2: 

Looking into mkoctfile.in.cc, I see that LFLAGS is supposedly being set as something from configure.ac


  vars["LDFLAGS"] = get_variable ("LDFLAGS", %OCTAVE_CONF_LDFLAGS%);


While LFLAGS is being set "manually"


  std::string DEFAULT_LFLAGS = "-L" + quote_path (vars["OCTLIBDIR"]);
  if (vars["LIBDIR"] != "/usr/lib")
    DEFAULT_LFLAGS += " -L" + quote_path (vars["LIBDIR"]);

[...]

  vars["LFLAGS"] = get_variable ("LFLAGS", DEFAULT_LFLAGS);


I don't quite get it, there's no tests units for this, and I'm not comfortable tweaking there.  Could anyone more familiar have its hand at it?

Carnë Draug <carandraug>
Group Member
Fri 12 Aug 2016 12:24:50 AM UTC, comment #1: 

This is us just being stupid.  If you try 'mkoctfile -h' the documentation clearly says that LDFLAGS should be a configuration variable.

I would say go ahead and make the change.  We can keep LFLAGS for another two releases.

Rik <rik5>
Group administrator
Tue 02 Aug 2016 01:15:32 AM UTC, original submission:  

From the Make manual https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html


LDFLAGS

    Extra flags to give to compilers when they are supposed to invoke the linker, ‘ld’, such as -L.

[...]

 LFLAGS

    Extra flags to give to Lex.


However, mkoctfile includes the LDFLAGS into LFLAGS instead


$ mkoctfile -p LFLAGS
-L/home/carandraug/.local/lib/octave/4.1.0+ -L/home/carandraug/.local/lib
$ mkoctfile -p LDFLAGS


This is bug is also present in Octave 4.0.1 (and possibly in previous versions)


$ /usr/local/bin/mkoctfile -p LFLAGS
-L/usr/local/lib/octave/4.0.1 -L/usr/local/lib
$ /usr/local/bin/mkoctfile -p LDFLAGS


and so there's backwards compatibility implications about fixing this. But since mkoctifle does not handle lex, it would be possible to keep using LDFLAGS incorrectly for a few versions, before removing LFLAGS completely.

Carnë Draug <carandraug>
Group 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 jwe (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by carandraug (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 group members can vote.

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-01-10 mtmiller Carbon-CopyRemoved 80942 -
    2019-01-04 mtmiller StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2019-01-04 mtmiller StatusFixed In Progress
        Open/ClosedClosed Open
    2019-01-04 rik5 Open/ClosedOpen Closed
    2019-01-04 rik5 StatusConfirmed Fixed
    2018-05-17 mtmiller CategoryNone Configuration and Build System
        Priority5 - Normal 3 - Low
    2016-08-13 rik5 Carbon-Copy- Added jwe
    2016-08-12 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code