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

alsa-firmware: enable cross compilation #63866

Merged

Conversation

dingxiangfei2009
Copy link
Contributor

Motivation for this change

The Autoconf and Automake scripts in alsa-firmware incorrectly build the firmware writers to the host platform, which leads to incorrect Exec format if they are invoked by these scripts later to transform the firmware blobs. This PR will properly build the writers to the build platform instead.

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/)
  • 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.

Copy link
Member

@JohnAZoidberg JohnAZoidberg left a comment

Choose a reason for hiding this comment

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

For my host system it builds fine but cross compiling fails:

builder for '/nix/store/gb1blacz8nlm8srjznpl7njspq74g51z-alsa-firmware-1.0.29-aarch64-unknown-linux-gnu.drv' failed with exit code 2; last 10 log lines:
  make[1]: Leaving directory '/build/alsa-firmware-1.0.29/korg1212'
  Making all in maestro3
  make[1]: Entering directory '/build/alsa-firmware-1.0.29/maestro3'
  aarch64-unknown-linux-gnu-gcc -DPACKAGE_NAME=\"alsa-firmware\" -DPACKAGE_TARNAME=\"alsa-firmware\" -DPACKAGE_VERSION=\"1.0.29\" -DPACKAGE_STRING=\"alsa-firmware\ 1.0.29\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DSTDC_HEADERS=1 -DPAC
KAGE=\"alsa-firmware\" -DVERSION=\"1.0.29\" -I.     -g -O2 -c -o fw_writer.o fw_writer.c
  aarch64-unknown-linux-gnu-gcc  -g -O2   -o fw_writer fw_writer.o  
  ./fw_writer
  /nix/store/x6b81sfmbmkmcyqp2rmk6jgsmzvyrjj0-bash-4.4-p23/bin/bash: ./fw_writer: cannot execute binary file: Exec format error
  make[1]: *** [Makefile:612: maestro3_assp_kernel.fw] Error 126
  make[1]: Leaving directory '/build/alsa-firmware-1.0.29/maestro3'
  make: *** [Makefile:361: all-recursive] Error 1
[0 built (1 failed)]
error: build of '/nix/store/gb1blacz8nlm8srjznpl7njspq74g51z-alsa-firmware-1.0.29-aarch64-unknown-linux-gnu.drv' failed

Looks like you need to do the same for fw_writer as for tobin.

@@ -0,0 +1,347 @@
--- a/hdsploader/Makefile.am 2015-02-26 20:36:03.000000000 +0800
Copy link
Member

Choose a reason for hiding this comment

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

Where did you get this patch from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I composed this patch myself. 😅

@@ -8,6 +8,19 @@ stdenv.mkDerivation rec {
sha256 = "0gfcyj5anckjn030wcxx5v2xk2s219nyf99s9m833275b5wz2piw";
};

patches = [ ./cross.patch ];

nativeBuildInputs = [ automake autoconf buildPackages.stdenv.cc ];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nativeBuildInputs = [ automake autoconf buildPackages.stdenv.cc ];
nativeBuildInputs = [ autoreconfHook buildPackages.stdenv.cc ];

This hook does all the autotools steps, so you don't need the preConfigure either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is neat!

@@ -1,4 +1,4 @@
{stdenv, fetchurl}:
{stdenv, buildPackages, autoconf, automake, fetchurl}:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{stdenv, buildPackages, autoconf, automake, fetchurl}:
{ stdenv, buildPackages, autoreconfHook, fetchurl }:

Spaces + autoreconfHook

@JohnAZoidberg
Copy link
Member

You need to do the same to fw_writer in {maestro3,sb16,wavefront,ymfpci}/Makefile.am

-$(firmware_files): fw_writer
-	./fw_writer
+LINK_FOR_BUILD.c = $(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(CPPFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $(TARGET_ARCH_FOR_BUILD)
+
+$(fw_writer_OBJECTS) : CC=$(CC_FOR_BUILD)
+$(fw_writer_OBJECTS) : CFLAGS=$(CFLAGS_FOR_BUILD)
+$(fw_writer_OBJECTS) : CPPFLAGS=$(CPPFLAGS_FOR_BUILD)
+
+fw_writer$(BUILD_EXEEXT): $(tobin_OBJECTS)
+	$(LINK_FOR_BUILD.c) $^ $(LOADLIBES_FOR_BUILD) $(LDLIBS_FOR_BUILD) -o $@
+
+$(firmware_files): fw_writer$(BUILD_EXEEXT)
+	./fw_writer$(BUILD_EXEEXT)```

Then it builds fine for me :)

@dingxiangfei2009
Copy link
Contributor Author

@JohnAZoidberg I don't know. I built pkgsCross.raspberryPi.alsa-firmware and maestro3 did not complain. Also, maestro3/Makefile.am is indeed in the patch.

@JohnAZoidberg
Copy link
Member

Thanks, now your command for building for raspi and nix build -f . alsa-firmware --arg crossSystem '{ config = "aarch64-unknown-linux-gnu"; }' works.

@JohnAZoidberg
Copy link
Member

Have you tried getting the patch merged upstream? :)

@dingxiangfei2009
Copy link
Contributor Author

@JohnAZoidberg Oh, good point. I should try upstreaming it, too.

@lopsided98
Copy link
Contributor

I see that you tried to upstream this patch, but it looks like it was ignored. Maybe you should replace the patch with a fetchpatch invocation that pulls from patchwork (https://patchwork.kernel.org/patch/11028517/mbox/), if only to document that an attempt was made to upstream it?

In any case, I tested cross builds for armv6l, armv7l and aarch64 and they all succeeded. I would love to get this merged, as it is one of the few blockers for basic cross-compiled NixOS.

@matthewbauer matthewbauer added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Aug 28, 2019
@dingxiangfei2009
Copy link
Contributor Author

@lopsided98 sad to hear. Okay, I will use a fetchpatch then.

@erictapen
Copy link
Member

This is also working fine for me on aarch64. Would love to see this merged soon!

@dingxiangfei2009
Copy link
Contributor Author

@JohnAZoidberg @matthewbauer Okay, I think I need to update this PR to the latest master (?) to keep it fresh, but should I rebase or merge?

@dingxiangfei2009
Copy link
Contributor Author

dingxiangfei2009 commented Oct 7, 2019

@lopsided98 Maybe it was my fault. I guess the email client I used to submit this patch did something funny with the patch file, and it looks corrupted. I will try to rectify this upstream patch.

Now by the look of the patch content it is definitely wrong: there is a reference to my Nix store path. 😂

@JohnAZoidberg
Copy link
Member

What's the status of this @dingxiangfei2009?
Did you fix the patch?

Please rebase it on top of the lastest master.

@infinisil
Copy link
Member

Any interest in this still?

@Atemu
Copy link
Member

Atemu commented Feb 2, 2020

I just ran into this issue while trying to cross-compile a very basic sd-card image for armv7l-hf-multiplatform and this patch solved it

@samueldr samueldr added this to the 20.03 milestone Feb 2, 2020
@disassembler disassembler modified the milestones: 20.03, 20.09 Feb 5, 2020
@matthewbauer
Copy link
Member

It looks like this no longer applies as of alsa-1.2.1.

matthewbauer added a commit to matthewbauer/alsa-firmware that referenced this pull request Mar 3, 2020
Rebased version of original patch by Ding Xiang Fei found in
NixOS/nixpkgs#63866. This version applies in
alsa-firmware-1.2.1.

Co-authored-by: Ding Xiang Fei <dingxiangfei2009@users.noreply.github.com>
@matthewbauer
Copy link
Member

matthewbauer commented Mar 3, 2020

Updated version at matthewbauer/alsa-firmware@a8a4784. @dingxiangfei2009 would it be okay to send this upstream?

matthewbauer added a commit to matthewbauer/alsa-firmware that referenced this pull request Mar 3, 2020
Rebased version of original patch by Ding Xiang Fei found in
NixOS/nixpkgs#63866. This version applies in
alsa-firmware-1.2.1.

Co-authored-by: Ding Xiang Fei <dingxiangfei2009@users.noreply.github.com>
matthewbauer added a commit to matthewbauer/alsa-firmware that referenced this pull request Mar 3, 2020
Rebased version of original patch by Ding Xiang Fei found in
NixOS/nixpkgs#63866. This version applies in
alsa-firmware-1.2.1.

Co-authored-by: Ding Xiang Fei <dingxiangfei2009@users.noreply.github.com>
matthewbauer added a commit to matthewbauer/alsa-firmware that referenced this pull request Mar 3, 2020
Rebased version of original patch by Ding Xiang Fei found in
NixOS/nixpkgs#63866. This version applies in
alsa-firmware-1.2.1.

Co-authored-by: Ding Xiang Fei <dingxiangfei2009@users.noreply.github.com>
matthewbauer added a commit to matthewbauer/alsa-firmware that referenced this pull request Mar 3, 2020
Rebased version of original patch by Ding Xiang Fei found in
NixOS/nixpkgs#63866. This version applies in
alsa-firmware-1.2.1.

Co-authored-by: Ding Xiang Fei <dingxiangfei2009@users.noreply.github.com>
Co-authored-by: Matthew Bauer <mjbauer95@gmail.com>
@matthewbauer matthewbauer merged commit f9b4b89 into NixOS:master Mar 6, 2020
@erictapen
Copy link
Member

Thanks for merging, @matthewbauer. Any chance this could get backported to nixos-20.03?

perexg pushed a commit to alsa-project/alsa-firmware that referenced this pull request Oct 19, 2020
Rebased version of original patch by Ding Xiang Fei found in
NixOS/nixpkgs#63866. This version applies in
alsa-firmware-1.2.1.

Co-authored-by: Ding Xiang Fei <dingxiangfei2009@users.noreply.github.com>
From: Matthew Bauer <mjbauer95@gmail.com>
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
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: 0 10.rebuild-linux: 1-10
Projects
No open projects
NixOS on ARM
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

9 participants