Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

octavePackages.ltfat: Set LD_LIBRARY_PATH for building packages #368376

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KarlJoad
Copy link
Contributor

@KarlJoad KarlJoad commented Dec 26, 2024

Some Octave packages ship C/C++/Fortran code that needs to be compiled and linked against Octave's internal shared objects. Previously, ld could not find these packages because $LD_LIBRARY_PATH was empty.

Closes #368187.

nixpkgs-review marked some new packages as broken (bim, fits, msh), but ltfat builds correctly.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Some Octave packages ship C/C++/Fortran code that needs to be compiled
and linked against Octave's internal shared objects. Previously, ld
could not find these packages because $LD_LIBRARY_PATH was empty.
# Add Octave's shared objects to ld library path for some packages which
# need to link their C/C++/Fortran code against Octave's internals.
LD_LIBRARY_PATH = lib.makeLibraryPath nativeBuildInputs';

Copy link
Contributor

@paparodeo paparodeo Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused about how LD_LIBRARY_PATH helps.

however, was able to get the build to work on darwin with this

diff --git a/pkgs/development/octave-modules/ltfat/default.nix b/pkgs/development/octave-modules/ltfat/default.nix
index 43fe020b322a..d22a78938b5a 100644
--- a/pkgs/development/octave-modules/ltfat/default.nix
+++ b/pkgs/development/octave-modules/ltfat/default.nix
@@ -9,6 +9,7 @@
   lapack,
   blas,
   portaudio,
+  octave,
   jdk,
 }:
 
@@ -21,6 +22,8 @@ buildOctavePackage rec {
     sha256 = "sha256-FMDZ8XFhLG7KDoUjtXvafekg6tSltwBaO0+//jMzJj4=";
   };
 
+  NIX_LDFLAGS = "-L${lib.getLib octave}/lib/octave/9.3.0";
+
   buildInputs = [
     fftw
     fftwSinglePrec

tho didn't test the plugin to see if it actually works -- there were some linker warnings.

it seems linux is building fine on master tho: https://hydra.nixos.org/build/282688853

[edit] on master octave on darwin needs #367594 to build

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at the darwin vs linux outputs with NIX_DEBUG=1 can see that linux doesn't link to -loctave or -octinterp. but sometimes darwin is fine. eg polyboolmex.mex seems to add -L/nix/store/svnxlwq9p3lpl3wjx70kx03qzf5zlhkw-octave-9.3.0/lib/octave/9.3.0 and builds but then comp_atrousfilterbank_td.oct comp_cellcoef2tf.oct comp_chirpzt.oct comp_col2diag.oct comp_dct.oct comp_dst.oct comp_dwilt.oct and comp_dwiltiii.oct don't and fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok looking at the bundle in oct/Makefile_unix it seems to export LDFLAGS

export LDFLAGS := -L$(shell $(MKOCTFILE) -p LIBDIR) $(FLIBS) $(LAPACK_LIBS) $(BLAS_LIBS) $(FFTW_LIBS)

and thru some experimentation when LDFLAGS are set I guess that overrides the flags used by mkoctfile because

make -f Makefile_unix comp_atrousfilterbank_td.oct
mkoctfile -strip -I../src/modules/libltfat/include -DNDEBUG -L../lib -lltfat comp_atrousfilterbank_td.cc
mkoctfile: stripping disabled on this platform
ld: library not found for -loctinterp

but just can comment out LDFLAGS in the makefile and then the command succeeds.

so this seems like an upstream issue.

Copy link
Contributor

@paparodeo paparodeo Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok -- sorry for all the posts. I just realized that can use patches in the octave builder adding -L$(mkoctfile -p OCTLIBDIR) to LDFLAGS in the makefile is sufficient.

should file an issue: https://github.com/ltfat/ltfat/issues

don't clobber ldflags.diff
index 43fe020b322a..c5c48e0dc5e0 100644
--- a/pkgs/development/octave-modules/ltfat/default.nix
+++ b/pkgs/development/octave-modules/ltfat/default.nix
@@ -32,6 +32,11 @@ buildOctavePackage rec {
     jdk
   ];
 
+  patches = [
+    # add OCTLIBDIR to ldflags
+    ./octlibdir-ldflags.diff
+  ];
+
   meta = with lib; {
     name = "The Large Time-Frequency Analysis Toolbox";
     homepage = "https://octave.sourceforge.io/ltfat/index.html";
diff --git a/pkgs/development/octave-modules/ltfat/octlibdir-ldflags.diff b/pkgs/development/octave-modules/ltfat/octlibdir-ldflags.diff
new file mode 100644
index 000000000000..c73f5701b8a9
--- /dev/null
+++ b/pkgs/development/octave-modules/ltfat/octlibdir-ldflags.diff
@@ -0,0 +1,13 @@
+diff --git a/oct/Makefile_unix b/oct/Makefile_unix
+index 21e026b..c9d767a 100644
+--- a/oct/Makefile_unix
++++ b/oct/Makefile_unix
+@@ -36,7 +36,7 @@ endif
+ export CXXFLAGS := $(shell $(MKOCTFILE) -p CXXFLAGS) -std=gnu++11 -Wall -DLTFAT_LARGEARRAYS $(OPTCXXFLAGS)
+ # export is necessary, otherwise LFLAGS won't have any effect
+ # at least on Windows and on Mac
+-export LDFLAGS := -L$(shell $(MKOCTFILE) -p LIBDIR) $(FLIBS) $(LAPACK_LIBS) $(BLAS_LIBS) $(FFTW_LIBS) 
++export LDFLAGS := -L$(shell $(MKOCTFILE) -p LIBDIR) -L$(shell $(MKOCTFILE) -p OCTLIBDIR) $(FLIBS) $(LAPACK_LIBS) $(BLAS_LIBS) $(FFTW_LIBS)
+ undefine LFLAGS
+ unexport LFLAGS
+ 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure: octavePackages.ltfat (darwin)
2 participants