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

openexr: fix cross-compilation #61245

Merged
merged 2 commits into from May 15, 2019

Conversation

Synthetica9
Copy link
Member

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
    • They seem to work, but I'm not sure what exactly they do, so I haven't tested them very thoroughly
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@globin
Copy link
Member

globin commented May 13, 2019

@GrahamcOfBorg build pkgsCross.aarch64-multiplatform.openexr

@Ericson2314 Ericson2314 added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label May 13, 2019
@Synthetica9
Copy link
Member Author

I think I see what's going wrong here. On my machine, I had binfmt_misc support for the platform I was trying to compile to, so I was able to run the ./b44ExpLogTable fine. However, on ofBorg, it is trying to run it without binfmt_misc. So we need to compile part of this project on the host platform, to generate all the sources. I'll see what I can do.

@Ericson2314
Copy link
Member

@Synthetica9 FYI you mean "So we need to compile part of this project on the build platform, to generate all the sources."

@Synthetica9 Synthetica9 changed the title openexr: fix cross-compilation [WIP] openexr: fix cross-compilation May 13, 2019
@Synthetica9
Copy link
Member Author

@GrahamcOfBorg build pkgsCross.aarch64-multiplatform.openexr

@Ericson2314
Copy link
Member

I might take a look at using $CC_FOR_BUILD for this. It's a bit lower tech solution that will work for non-linux.

@matthewbauer
Copy link
Member

I wonder if Linux has a way to disable binfmt in a process? That would help avoid cases like this.

@Ericson2314
Copy link
Member

Ericson2314 commented May 14, 2019

OK try this:

diff --git a/OpenEXR/IlmImf/Makefile.am b/OpenEXR/IlmImf/Makefile.am
index c047b1e..c341eee 100644
--- a/OpenEXR/IlmImf/Makefile.am
+++ b/OpenEXR/IlmImf/Makefile.am
@@ -196,14 +196,14 @@ INCLUDES = @ILMBASE_CXXFLAGS@ \
 
 CLEANFILES = b44ExpLogTable b44ExpLogTable.h dwaLookups dwaLookups.h
 
-b44ExpLogTable_SOURCES = b44ExpLogTable.cpp
-b44ExpLogTable_LDADD = @ILMBASE_LDFLAGS@ $(ILMBASE_LIBS)
+b44ExpLogTable$(EXEEXT_FOR_BUILD): b44ExpLogTable.cpp
+       $(CXX_FOR_BUILD) $(CXXFLAGS_FOR_BUILD) $< -o $@
 
 b44ExpLogTable.h: b44ExpLogTable
        ./b44ExpLogTable > b44ExpLogTable.h
 
-dwaLookups_SOURCES = dwaLookups.cpp
-dwaLookups_LDADD = @ILMBASE_LDFLAGS@ $(ILMBASE_LIBS)
+dwaLookups$(EXEEXT_FOR_BUILD): dwaLookups.cpp
+       $(CXX_FOR_BUILD) $(CXXFLAGS_FOR_BUILD) $< -o $@
 
 dwaLookups.h: dwaLookups
        ./dwaLookups > dwaLookups.h
diff --git a/OpenEXR/configure.ac b/OpenEXR/configure.ac
index 067f9d0..5a9c4dd 100644
--- a/OpenEXR/configure.ac
+++ b/OpenEXR/configure.ac
@@ -30,6 +30,8 @@ AC_PROG_CC
 AC_PROG_LN_S
 AC_PROG_LIBTOOL
 AC_PROG_MAKE_SET
+AX_CC_FOR_BUILD dnl for EXEEXT_FOR_BUILD not CC_FOR_BUILD
+AX_PROG_CXX_FOR_BUILD
 
 dnl
 dnl PKGCONFIG preparations

which I through together (no testing) based off of 694b4d2 using AcademySoftwareFoundation/openexr#221. I think ilmbase will need to be a nativeBuildInput because of a #include <half.h>.

If that works, please submit upstream! Maybe include the earlier patch with EXEEXT_FOR_BUILD too in the PR for consistency and to/from windows support (i.e. where EXEEXT`` and EXEEXT_FOR_BUILD` would differ).

@Synthetica9
Copy link
Member Author

@Ericson2314

Looks promising, but I don't think I'm comfortable enough with Automake to make it work.

Makefile:1332: warning: overriding recipe for target 'b44ExpLogTable'
Makefile:751: warning: ignoring old recipe for target 'b44ExpLogTable'
Makefile:1338: warning: overriding recipe for target 'dwaLookups'
Makefile:755: warning: ignoring old recipe for target 'dwaLookups'
make[1]: *** No rule to make target 'b44ExpLogTable.c', needed by 'b44ExpLogTable.o'.  Stop.
make[1]: Leaving directory '/build/openexr-2.3.0/IlmImf'
make: *** [Makefile:502: all-recursive] Error 1
note: keeping build directory '/tmp/nix-build-openexr-2.3.0.drv-8'
builder for '/nix/store/fnwxk8b7jmgzalb2wp8v12m4w8gigd5z-openexr-2.3.0.drv' failed with exit code 2
error: build of '/nix/store/fnwxk8b7jmgzalb2wp8v12m4w8gigd5z-openexr-2.3.0.drv' failed

@Synthetica9
Copy link
Member Author

I wonder if Linux has a way to disable binfmt in a process? That would help avoid cases like this.

I dug around for a bit, and it doesn't look like it. This might be worth filing a kernel feature request for.

@Synthetica9
Copy link
Member Author

@GrahamcOfBorg build pkgsCross.aarch64-multiplatform.openexr

@Synthetica9 Synthetica9 changed the title [WIP] openexr: fix cross-compilation openexr: fix cross-compilation May 14, 2019
@Synthetica9
Copy link
Member Author

Synthetica9 commented May 14, 2019

I might take a look at using $CC_FOR_BUILD for this. It's a bit lower tech solution that will work for non-linux.

@Ericson2314 Would you mind merging this in the meantime? I couldn't get your solution to work, and this already better than the current situation.

@Synthetica9
Copy link
Member Author

@GrahamcOfBorg build openexr

@Ericson2314
Copy link
Member

AcademySoftwareFoundation/openexr#384 I got sucked in and then stuck. This is definitely good enough :).

@edolstra
Copy link
Member

BTW this sort of patch is the sort that really makes me doubt whether it's a good idea to support cross-compilation, because it adds considerable complexity and makes it harder to maintain/update the openexr package.

Synthetica9 pushed a commit to Synthetica9/nixpkgs that referenced this pull request May 15, 2019
@Ericson2314
Copy link
Member

@edolstra I don't know how you can draw a general story about cross compilation support from this. Maybe openexr is a bridge to far, but other packages aren't? Then again, openexr has barely changed in years. Per the upcoming RFC, I think people have a right to rip out any per-package cross patch that interferes with something important for a high tier, so what's the practical damage then?

I'm basically finished making big infra overhauls for cross; it works now. If somebody else wants a package to work a take that as external demand, not my own dream, and try to assist them. I'm obviously biased, but I think the feature is pretty popular?

@matthewbauer
Copy link
Member

matthewbauer commented May 15, 2019

It's definitely ugly, but I would hope we can improve it over time. It is possible to write build script that work elegantly, openexr just doesn't. I meant to approve a025f85 not a347ec5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants