bugGNU Octave - Bugs: bug #59373, [MXE] for dependencies that use...

 
 

bug #59373: [MXE] for dependencies that use cmake, don't build static libraries

Submitted by:  None
Submitted on:  Wed 28 Oct 2020 07:26:40 AM UTC  
 
Category:  Configuration and Build System Severity:  5 - Blocker
Priority:  7 - High Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  Closed Release:  6.0.92
Operating System:  Microsoft Windows

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 01 Nov 2020 09:11:11 AM UTC, comment #33: 

The buildbots are through a complete cycle now. And all MXE Octave builds are green.

Closing as fixed.

Markus Mützel <mmuetzel>
Project Member
Fri 30 Oct 2020 07:40:05 PM UTC, comment #32: 

Can we consider this issue fixed?  OK to close?

John W. Eaton <jwe>
Project Administrator
Fri 30 Oct 2020 07:39:18 PM UTC, comment #31: 

John: Thanks for the change.  Maybe it doesn't matter much for this library, but if I understand correctly, setting CFLAGS this way replaces the default "-g -O2" with just " -fstack-protector" because $(CFLAGS) is not set in the mxe-octave Makefile.  I can't think of a better way to fix this so I checked in the following additional change:

https://hg.octave.org/mxe-octave/rev/3c05fc63c6dd

John W. Eaton <jwe>
Project Administrator
Fri 30 Oct 2020 06:16:28 PM UTC, comment #30: 
John Donoghue <lostbard>
Project Member
Fri 30 Oct 2020 04:55:29 PM UTC, comment #29: 

OK, I backed out the change.  I wrong about LLVM.dll being created even without those options.

John W. Eaton <jwe>
Project Administrator
Fri 30 Oct 2020 04:26:12 PM UTC, comment #28: 

Hmm, it also failed for me in a fresh build.  I removed the files installed for the llvm package, reverted the change, and I'm trying again.  If that works (and I expect it will) then I'll undo the change in the mxe-octave repo.

John W. Eaton <jwe>
Project Administrator
Fri 30 Oct 2020 03:59:37 PM UTC, comment #27: 

Mesa no longer builds for me at hg id 3588be46d1d9.
It looks like something from LLVM is wrong or missing.
It still builds for me at hg id b19fb3ed330c.

So this change seems to cause the issue:

changeset 5572:3588be46d1d9 default tip @

don't use LLVM_BUILD_LLVM_DYLIB or LLVM_LINK_LLVM_DYLIB for Windows builds

* llvm.mk: The LLVM_BUILD_LLVM_DYLIB and LLVM_LINK_LLVM_DYLIB options
are documented to be unsupported for Windows builds of LLVM.  Without
these options, the Windows build still creates an LLVM.dll file, but
also creates .a files for the components.  Update comments.
author        John W. Eaton <jwe@octave.org>
date        Fri, 30 Oct 2020 10:11:58 -0400 (106 minutes ago)

Markus Mützel <mmuetzel>
Project Member
Fri 30 Oct 2020 03:24:18 PM UTC, comment #26: 

@jwe: I used the "Dependency Walker" in Windows:
https://www.dependencywalker.com/

Markus Mützel <mmuetzel>
Project Member
Fri 30 Oct 2020 02:57:19 PM UTC, comment #25: 

I think I have the fix of cairo

build-markupsafe didnt build for me with the cmake patch - Im rebuilding again with the change to the repo

John Donoghue <lostbard>
Project Member
Fri 30 Oct 2020 02:28:24 PM UTC, comment #24: 

Yes, for pango and cairo it seems to be caused by the linker not finding a shared version of libssp, but I don't understand how or why that is happening.

Markus, in comment #12 you wrote "With the attached patch, liboctinterp depends on hdf5.dll for me."  How are you determining the dependencies for a DLL?

John W. Eaton <jwe>
Project Administrator
Fri 30 Oct 2020 02:08:18 PM UTC, comment #23: 

Markus: Thanks.  I pushed the following change to make this setting global for all cmake projects we use and to choose the setting once at the beginning of the Makefile instead of at each cmake invocation using $(if ...).

https://hg.octave.org/mxe-octave/rev/b19fb3ed330c

John W. Eaton <jwe>
Project Administrator
Fri 30 Oct 2020 01:34:54 PM UTC, comment #22: 

@jwe: You still might have been spot on.
With the attached patch, static libraries are no longer built for sundials.
There might have been a change (in cmake?) that broke setting BUILD_SHARED_LIBS and BUILD_STATIC_LIBS from the toolchain file.

(file #50169)

Markus Mützel <mmuetzel>
Project Member
Fri 30 Oct 2020 01:31:10 PM UTC, comment #21: 

sundials looks like the BUILD_STATIC_LIBS issue.

cairo looks like it has undefined references so is forced to build static - pango uses cairo.

I'm rebuilding cairo to check.

John Donoghue <lostbard>
Project Member
Fri 30 Oct 2020 01:09:31 PM UTC, comment #20: 

I'll have a look at the build rule for sundials.

Markus Mützel <mmuetzel>
Project Member
Fri 30 Oct 2020 12:58:32 PM UTC, comment #19: 

I think I may have misunderstood a lot of things during the course of this discussion.

In any case, I now understand that the variables used for selecting static libraries in cmake projects are not uniform.

As I understand it, there are many .a libraries in MinGW that correspond to Windows DLLs and that are only there to allow linking to succeed but don't provide the actual functions.  Those files are not a problem.

Also if I understand correctly, the .dll.a files are just present to provide linking info for the corresponding .dll files so they are not a problem.

What are the new/correct(?) variable to avoid creating .a files for sundials, cairo, and pango?

For hdf5, it is apparently the ONLY_SHARED_LIBS variable.

Other than those, I think the other big generator of .a libraries is LLVM where the component libraries (that I don't think we need) are not created as DLLs in the Windows build.  I don't know whether it is easy or possible to fix that directly using LLVM build options.

John W. Eaton <jwe>
Project Administrator
Fri 30 Oct 2020 12:42:41 PM UTC, comment #18: 

Doing a quick diff of .a files in 5.2 vs 6.0 I get:

-\mingw64/lib/libcairo-gobject.dll.a
-\mingw64/lib/libcairo-script-interpreter.dll.a
-\mingw64/lib/libcairo.dll.a
+\mingw64/lib/libcairo-gobject.a
+\mingw64/lib/libcairo-script-interpreter.a
+\mingw64/lib/libcairo.a
-\mingw64/lib/libcloog-isl.a
+\mingw64/lib/libctf-nobfd.a
+\mingw64/lib/libctf.a
+\mingw64/lib/libdcomp.a
+\mingw64/lib/libh5bzip2.dll.a
-\mingw64/lib/libhdf5.dll.a
-\mingw64/lib/libhdf5_hl.dll.a
+\mingw64/lib/libhdf5.a
+\mingw64/lib/libhdf5_hl.a
+\mingw64/lib/libhdf5_tools.a
+\mingw64/lib/libLLVMAggressiveInstCombine.a
+\mingw64/lib/libmsoledbsql.a
+\mingw64/lib/libntdllcrt.a
-\mingw64/lib/libpangocairo-1.0.dll.a
+\mingw64/lib/libpangocairo-1.0.a
+\mingw64/lib/libshcore.a
+\mingw64/lib/libsundials_nvecmanyvector.a
+\mingw64/lib/libsundials_nvecmanyvector.dll.a
\mingw64/lib/libsundials_nvecserial.a
\mingw64/lib/libsundials_nvecserial.dll.a
+\mingw64/lib/libsundials_sunlinsolband.a
+\mingw64/lib/libsundials_sunlinsolband.dll.a
+\mingw64/lib/libsundials_sunlinsoldense.a
+\mingw64/lib/libsundials_sunlinsoldense.dll.a
+\mingw64/lib/libsundials_sunlinsolklu.a
+\mingw64/lib/libsundials_sunlinsolklu.dll.a
+\mingw64/lib/libsundials_sunlinsolpcg.a
+\mingw64/lib/libsundials_sunlinsolpcg.dll.a
+\mingw64/lib/libsundials_sunlinsolspbcgs.a
+\mingw64/lib/libsundials_sunlinsolspbcgs.dll.a
+\mingw64/lib/libsundials_sunlinsolspfgmr.a
+\mingw64/lib/libsundials_sunlinsolspfgmr.dll.a
+\mingw64/lib/libsundials_sunlinsolspgmr.a
+\mingw64/lib/libsundials_sunlinsolspgmr.dll.a
+\mingw64/lib/libsundials_sunlinsolsptfqmr.a
+\mingw64/lib/libsundials_sunlinsolsptfqmr.dll.a
+\mingw64/lib/libsundials_sunmatrixband.a
+\mingw64/lib/libsundials_sunmatrixband.dll.a
+\mingw64/lib/libsundials_sunmatrixdense.a
+\mingw64/lib/libsundials_sunmatrixdense.dll.a
+\mingw64/lib/libsundials_sunmatrixsparse.a
+\mingw64/lib/libsundials_sunmatrixsparse.dll.a
+\mingw64/lib/libsundials_sunnonlinsolfixedpoint.a
+\mingw64/lib/libsundials_sunnonlinsolfixedpoint.dll.a
+\mingw64/lib/libsundials_sunnonlinsolnewton.a
+\mingw64/lib/libsundials_sunnonlinsolnewton.dll.a
+\mingw64/lib/libtasn1.dll.a
+\mingw64/lib/libwslapi.a

So looks like the main culprits are hdf5, cairo, pango and sundials

John Donoghue <lostbard>
Project Member
Fri 30 Oct 2020 07:27:03 AM UTC, comment #17: 

@John D: I agree that there is probably an error in the cmake build rule for hdf5 that causes it to build static libraries when it shouldn't. That error was most probably introduced during the transition from a configure to a cmake build rule in the changeset you and jwe pointed out.
The patch in comment #12 should address that.

But IIUC, jwe also observed a lot of new static libraries (not only hdf5) that we didn't build before.

Markus Mützel <mmuetzel>
Project Member
Fri 30 Oct 2020 12:50:55 AM UTC, comment #16: 

Previously hd5 win windows was building with configure not cmake [1] so thats probally the difference between the 5.2 and 6.0 octave libs.

[1] https://hg.octave.org/mxe-octave/rev/98bcc037fad7

John Donoghue <lostbard>
Project Member
Thu 29 Oct 2020 02:29:29 PM UTC, comment #15: 

It is quite possible that BUILD_STATIC_LIBS is a variable that just "happens" to be used by many packages.

I saw ONLY_SHARED_LIBS in the CMakeLists.txt file of hdf5. Its description is "Only Build Shared Libraries". So I figured this is probably what we want. (Given that I'm still struggling to correctly interpret the messages about BUILD_STATIC_LIBS and BUILD_SHARED_LIBS.)
I don't remember having seen that particular variable in other CMakeLists.txt files. But tbh, I didn't focus on that then.

It's probably because I'm no native speaker, so please correct me if my interpretation is wrong. But what I gather from `./usr/bin/cmake --help-policy CMP0077` is: The OLD behavior (which currently is used) ignores the value we set for e.g. BUILD_STATIC_LIBS. Instead a new (cached) variable is created that has the value defined in the subproject. The NEW behavior (which could be activated by a policy) lets our value override the value set by the subproject.

If that is the case (fall back to OLD behavior), why do we see a change in behavior?

Markus Mützel <mmuetzel>
Project Member
Thu 29 Oct 2020 01:54:46 PM UTC, comment #14: 

Hmm, I don't see BUILD_STATIC_LIBS documented for cmake, so this appears to maybe just be a variable that is (was?) used by some packages?  Likewise for ONLY_SHARED_LIBS, is that a variable that is specific to the hdf5 build system?

John W. Eaton <jwe>
Project Administrator
Thu 29 Oct 2020 01:34:19 PM UTC, comment #13: 

I'm also a bit confused now about what is happening.

Previously, I thought we could just specify the BUILD_SHARED/BUILD_STATIC options in the cmake toolchain file, so if we need to use the -D options on the command line, I guess the options in the toolchain file are ignored now?  I also thought that if BUILD_STATIC was off, then we wouldn't be building static libs.  Now we need to also use -DONLY_SHARED_LIBS=ON?

In any case, it would be good to have a consistent solution for all packages that use cmake and to avoid creating and distributing static library files that aren't needed.

I'm testing the attached change to see whether it avoids the policy warning and does the right thing for creating shared but not static libraries.

(file #50162)

John W. Eaton <jwe>
Project Administrator
Thu 29 Oct 2020 11:52:33 AM UTC, comment #12: 

With the attached patch, liboctinterp depends on hdf5.dll for me.

I always thought that the linker would prefer the dynamic library if both a static and a dynamic library were available. But either something goes wrong if both the static and the dynamic library are built, or that assumption was wrong in the first place.

In any case, I agree that we shouldn't have built a static version of the hdf5 libraries if BUILD_STATIC is "no".

With that patch, there is still no "libhdf5.dll.a". But as stated before, liboctinterp depends on hdf5.dll for me. I guess something like "libhdf5.lib" is equivalent.
To be honest, all of these (differing?) conventions with library file extensions are confusing me...

With the patch, I see the following files in "bin" (file sizes are without stripping symbols):

-rwxr-xr-x 1 5206904 Oct 29 08:35 hdf5.dll
-rwxr-xr-x 1  474983 Oct 29 08:35 hdf5_hl.dll
-rwxr-xr-x 1  652232 Oct 29 08:35 hdf5_tools.dll

And in "lib":

-rw-r--r-- 1  114440 Oct 29 08:35 libhdf5_hl.lib
-rw-r--r-- 1 2248250 Oct 29 08:35 libhdf5.lib
-rw-r--r-- 1    3441 Oct 29 08:33 libhdf5.settings
-rw-r--r-- 1  109526 Oct 29 08:35 libhdf5_tools.lib

I didn't yet have time to completely understand the implications of policy CMP0077. But from what I gather from a first look at it, I'm not sure if we have to revisit how we are setting BUILD_SHARED_LIBS and BUILD_STATIC_LIBS?

It's true that there are a lot of .a files in "lib". But there also were a lot for 5.2.0. Is there an easy way to find out which ones are new?

(file #50161)

Markus Mützel <mmuetzel>
Project Member
Wed 28 Oct 2020 06:33:42 PM UTC, comment #11: 

Changing title again and marking this problem as a blocker, at least for the release of Windows binaries for 6.1.

John W. Eaton <jwe>
Project Administrator
Wed 28 Oct 2020 06:06:46 PM UTC, comment #10: 

Markus: If we link using -lhdf5, then the linker for MinGW systems is looking for a file with a name like libhdf5.a or libhdf5.dll.a.  But with the current rules, we are generating libhdf5.a but not libhdf5.dll.a.

In 5.2.0, we had these files:


   136704  2020-02-03 20:14   octave-5.2.0-w64/mingw64/bin/libhdf5_hl-9.dll
  3704832  2020-02-03 20:14   octave-5.2.0-w64/mingw64/bin/libhdf5-9.dll
      938  2020-02-03 20:13   octave-5.2.0-w64/mingw64/lib/libhdf5_hl.la
      906  2020-02-03 20:13   octave-5.2.0-w64/mingw64/lib/libhdf5.la
   107986  2020-02-03 17:04   octave-5.2.0-w64/mingw64/lib/libhdf5_hl.dll.a
  1552544  2020-02-03 17:04   octave-5.2.0-w64/mingw64/lib/libhdf5.dll.a

Now we have

   289792  2020-10-24 02:26   octave-6.0.92-w64/mingw64/bin/hdf5_tools.dll
  4311552  2020-10-24 02:26   octave-6.0.92-w64/mingw64/bin/hdf5.dll
   132096  2020-10-24 02:26   octave-6.0.92-w64/mingw64/bin/hdf5_hl.dll
   457344  2020-10-23 14:00   octave-6.0.92-w64/mingw64/lib/libhdf5_tools.a
   194468  2020-10-23 14:00   octave-6.0.92-w64/mingw64/lib/libhdf5_hl.a
  8151460  2020-10-23 14:00   octave-6.0.92-w64/mingw64/lib/libhdf5.a
   109526  2020-10-23 14:00   octave-6.0.92-w64/mingw64/lib/libhdf5_tools.lib
  2248250  2020-10-23 14:00   octave-6.0.92-w64/mingw64/lib/libhdf5.lib
   114440  2020-10-23 14:00   octave-6.0.92-w64/mingw64/lib/libhdf5_hl.lib

I see now that we are also generating a lot of '.a' libraries where before I think we weren't generating them because of our rules to enable shared and disable static libraries.

In the log files for libraries that we build with cmake, I am now seeing the following info:

CMake Warning (dev) at CMakeLists.txt:444 (option):
  Policy CMP0077 is not set: option() honors normal variables.  Run "cmake
  --help-policy CMP0077" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  For compatibility with older versions of CMake, option is clearing the
  normal variable 'BUILD_STATIC_LIBS'.
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) at CMakeLists.txt:446 (option):
  Policy CMP0077 is not set: option() honors normal variables.  Run "cmake
  --help-policy CMP0077" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  For compatibility with older versions of CMake, option is clearing the
  normal variable 'BUILD_SHARED_LIBS'.
This warning is for project developers.  Use -Wno-dev to suppress it.

I'm not sure how best to fix this, but the problem is not specific to HDF5 and there should be a single fix that we can make for all of the libraries that we build using cmake.  They should all be using the

set(BUILD_SHARED_LIBS ON)
set(BUILD_STATIC_LIBS OFF)

settings in the CMAKE_TOOLCHAIN_FILE that we generate.

John W. Eaton <jwe>
Project Administrator
Wed 28 Oct 2020 06:01:22 PM UTC, comment #9: 

Aaaah. OK. I see. I'll have a look at how to fix the build rule.

Markus Mützel <mmuetzel>
Project Member
Wed 28 Oct 2020 05:41:44 PM UTC, comment #8: 

The most important difference that is missed in comment #5:

octave 5-2-0  : liboctinterp-7.dll depends on libhdf5-9.dll
octave 6-0-93 : hdf5.dll cannot be seen in the dependency list of liboctinterp-8.dll

Anonymous
Wed 28 Oct 2020 05:40:48 PM UTC, comment #7: 

Please forgive my ignorance. But why does that mean that we are linking to a static library now? "hdf5.dll" is a .dll after all...

Markus Mützel <mmuetzel>
Project Member
Wed 28 Oct 2020 05:20:28 PM UTC, comment #6: 

Thanks.  I assume this change is what caused the problem:

https://hg.octave.org/mxe-octave/rev/98bcc037fad7

Previously, I don't think we were building static libraries and we shouldn't be doing that now either.

John W. Eaton <jwe>
Project Administrator
Wed 28 Oct 2020 05:07:31 PM UTC, comment #5: 

Some problems may be related to visibility of symbols and "-Wl,--export-all-symbols" but I believe that liboctinterp has linked statically against hdf library. One reason is that liboctinterp now depends on symbols on third libraries that hdf library depends on them.
Some differences between current 6.0.93 alpha and octave-5.2.0:

octave 5-2-0  : libhdf5-9.dll in bin directory
octave 6-0-93 : hdf5.dll in bin directory

octave 5-2-0  : libhdf5.la and libhdf5.settings in lib directory
octave 6-0-93 : only libhdf5.settings in lib directory

octave 5-2-0  :
In lib/octave/5.2.0/liboctinterp.la the file /usr/lib/libhdf5.la is listed for dependency_libs.

octave 6-0-93 :
In lib/octave/6.0.93/liboctinterp.la the option -lhdf5 is listed for dependency_libs.

Anonymous
Wed 28 Oct 2020 03:27:35 PM UTC, comment #4: 

I remember now that there is also patch #8919 that is about getting Octave to compile with "-fvisibility=hidden".

IIUC, this bug depends on having that patch completed first.

Re-titling and setting release to "dev" because this shouldn't block 6.1.

Markus Mützel <mmuetzel>
Project Member
Wed 28 Oct 2020 03:03:35 PM UTC, comment #3: 

Separately, we might consider limiting the visibility of Octave functions, but that's a pretty big undertaking as Octave is a large project and we currently export all non-static symbols.  Determining which symbols should be exported will likely take a lot of time and effort.  See also bug #34301.

There's not much we can do about other libraries that Octave depends on unless those projects have defined lists of symbols to be exported.

John W. Eaton <jwe>
Project Administrator
Wed 28 Oct 2020 02:56:50 PM UTC, comment #2: 

See the mailing list thread that contains this message:

https://lists.gnu.org/archive/html/octave-maintainers/2013-07/msg00171.html

John W. Eaton <jwe>
Project Administrator
Wed 28 Oct 2020 02:14:51 PM UTC, comment #1: 

Afaict, liboctinterp is not statically linked against hdf5.
However, we are linking with "-Wl,--export-all-symbols". That might be the reason why all of these symbols (probably not limited to but including the hdf5 symbols) are exported.

That switch is probably picked up from here:
https://hg.savannah.gnu.org/hgweb/octave/file/b3cd6eb1cca5/configure.ac#l2472

The switch was added there a while back with this change:
https://hg.savannah.gnu.org/hgweb/octave/rev/db4453f40550
configure.ac: fix  building Java support for MinGW
author Philip Nienhuis <prnienhuis@users.sf.net>
date Thu, 11 Jul 2013 22:47:02 +0200 (2013-07-11)

Does anyone remember why that was added?
Is it worth to try and limit the scope of that switch? Could exporting many symbols be problematic?

CC'ing Philip as the original author.

Markus Mützel <mmuetzel>
Project Member
Wed 28 Oct 2020 07:26:40 AM UTC, original submission:  

When checking exported symbols of liboctinterp-8.dll it appears that liboctinterp is statically linked against hdf5 library and all HDF related functions (more than 3350 functions) are parts of export table of liboctinterp.
Thanks.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50169:  bug59373_sundials-ida_sharedlibs.patch added by mmuetzel (880B - application/octet-stream)
file #50162:  cmake-shared-diffs.txt added by jwe (17KiB - text/plain)
file #50161:  bug59373_hdf5_sharedlib.patch added by mmuetzel (1KiB - application/octet-stream)

 

Digest:
   patch dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by lostbard (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by mmuetzel
  •  

    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 16 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-11-01 mmuetzel StatusConfirmed => Fixed
        Open/ClosedOpen => Closed
    2020-10-30 mmuetzel Attached File- => Added bug59373_sundials-ida_sharedlibs.patch, #50169
    2020-10-29 jwe Attached File- => Added cmake-shared-diffs.txt, #50162
    2020-10-29 mmuetzel Attached File- => Added bug59373_hdf5_sharedlib.patch, #50161
    2020-10-28 jwe Severity3 - Normal => 5 - Blocker
        Priority5 - Normal => 7 - High
        Item GroupOther => Regression
        Releasedev => 6.0.92
        SummaryDon't link liboctinterp with "-Wl,--export-all-symbols" => [MXE] for dependencies that use cmake, don't build static libraries
    2020-10-28 mmuetzel StatusNone => Confirmed
        Release6.0.92 => dev
        Summaryliboctinterp statically linked against hdf5 => Don't link liboctinterp with "-Wl,--export-all-symbols"
        Dependencies- => Depends on patch #8919
    2020-10-28 mmuetzel CategoryLibraries => Configuration and Build System
        Carbon-Copy- => Added philipnienhuis

    Back to the top


    Powered by Savane 3.5