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

[20.09] libretro.mame2016: fix build with gnumake-4.3 #102078

Merged

Conversation

AluisioASG
Copy link
Contributor

Motivation for this change

Backport #101949 to unbreak libretro cores.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@AluisioASG AluisioASG changed the title libretro.mame2016: fix build with gnumake-4.3 [20.09] libretro.mame2016: fix build with gnumake-4.3 Oct 29, 2020
@ofborg ofborg bot requested review from hrdinka, MP2E and edwtjo October 29, 2020 17:35
@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 102078 run on x86_64-darwin 1

1 package marked as broken and skipped:
  • libretro.mame2016

@AluisioASG
Copy link
Contributor Author

AluisioASG commented Oct 29, 2020

1 package marked as broken and skipped:

  • libretro.mame2016

Stale cache somehow? I explicitly unmarked it as broken.

I'm running nixpkgs-review on x86_64-linux, but it's taking its time.

@AluisioASG
Copy link
Contributor Author

Result of nixpkgs-review pr 102078 1

1 package built:
  • libretro.mame2016

@SuperSandro2000
Copy link
Member

Stil the same:

Result of nixpkgs-review pr 102078 run on x86_64-darwin 1

1 package marked as broken and skipped:
  • libretro.mame2016

@AluisioASG
Copy link
Contributor Author

nix-repl> (import ./. { system = "x86_64-darwin"; }).libretro.mame2016
error: nixPackage ‘alsa-lib-1.2.3’ in .../pkgs/os-specific/linux/alsa-lib/default.nix:31 is not supported on ‘x86_64-darwin’, refusing to evaluate.

Looks like MAME works on macOS, so we need someone who can run it to correct the dependencies. Our current MAME derivation might be of some help.

SuperSandro2000 added a commit to NuschtOS/NuschtOS that referenced this pull request Oct 30, 2020
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Oct 30, 2020

I used nix-tree $(nix-instantiate -A libretro.mame2016) to find from where it is using libAlsa and wayland.

With this patch it starts to compile on darwin 🎉 :

diff --git a/pkgs/misc/emulators/retroarch/cores.nix b/pkgs/misc/emulators/retroarch/cores.nix
index 4a8ca8713e3..6e970098a4b 100644
--- a/pkgs/misc/emulators/retroarch/cores.nix
+++ b/pkgs/misc/emulators/retroarch/cores.nix
@@ -549,7 +549,9 @@ in with stdenv.lib.licenses;
     description = "Port of MAME to libretro";
     license = gpl2Plus;
 
-    extraBuildInputs = [ alsaLib libGLU libGL portaudio python27 xorg.libX11 ];
+    extraBuildInputs = [ libGLU libGL portaudio python27 xorg.libX11 ]
+      ++ stdenv.lib.optional stdenv.isLinux alsaLib;
+
     postPatch = ''
       # Prevent the failure during the parallel building of:
       # make -C 3rdparty/genie/build/gmake.linux -f genie.make obj/Release/src/host/lua-5.3.0/src/lgc.o
@@ -618,7 +620,7 @@ in with stdenv.lib.licenses;
     description = "Port of MAME ~2015 to libretro";
     license = gpl2Plus;
     extraNativeBuildInputs = [ python27 ];
-    extraBuildInputs = [ alsaLib ];
+    extraBuildInputs = stdenv.lib.optional stdenv.isLinux alsaLib;
     makefile = "Makefile";
   };
 
@@ -639,7 +641,7 @@ in with stdenv.lib.licenses;
     description = "Port of MAME ~2016 to libretro";
     license = gpl2Plus;
     extraNativeBuildInputs = [ python27 ];
-    extraBuildInputs = [ alsaLib ];
+    extraBuildInputs = stdenv.lib.optional stdenv.isLinux alsaLib;
     postPatch = ''
       # Prevent the failure during the parallel building of:
       # make -C 3rdparty/genie/build/gmake.linux -f genie.make obj/Release/src/host/lua-5.3.0/src/lgc.o
diff --git a/pkgs/misc/emulators/retroarch/default.nix b/pkgs/misc/emulators/retroarch/default.nix
index bfc2c338769..9b2a1db9462 100644
--- a/pkgs/misc/emulators/retroarch/default.nix
+++ b/pkgs/misc/emulators/retroarch/default.nix
@@ -32,7 +32,8 @@ stdenv.mkDerivation rec {
     rev = "v${version}";
   };
 
-  nativeBuildInputs = [ pkgconfig wayland ]
+  nativeBuildInputs = [ pkgconfig ]
+                      ++ optional stdenv.isLinux wayland
                       ++ optional withVulkan makeWrapper;
 
   buildInputs = [ ffmpeg_3 freetype libxml2 libGLU libGL python3 SDL2 which ]

I moved all alsaLibs to Linux only but did not test if this is required.

But then it fails to compile because it can't find gcc

no configure script, doing nothing
building
build flags: -j4 -l4 SHELL=/nix/store/smimdn17mp2ih6nfxwl78mswwdkmcm56-bash-4.4-p23/bin/bash platform=osx ARCH=x86_64
make REGENIE=1 VERBOSE=1 NOWERROR=1 OSD="retro" PYTHON_EXECUTABLE=python CONFIG=libretro LIBRETRO_OS="osx" ARCH="" LIBRETRO_CPU="x86_64"
make[1]: Entering directory '/private/var/folders/r4/k85s52jx3555ckj0wty615s00000gp/T/nix-build-libretro-mame2016-2020-03-06.drv-0/mame2016-libretro-02987af'
GCC  detected
make -R verbose=1 -C 3rdparty/genie/build/gmake.darwin -f genie.make
mkdir -p "build/generated/mame/layout/"
mkdir -p "build/generated/mame/mame/"
mkdir -p "build/generated/mame/drivers/"
make[2]: Entering directory '/private/var/folders/r4/k85s52jx3555ckj0wty615s00000gp/T/nix-build-libretro-mame2016-2020-03-06.drv-0/mame2016-libretro-02987af/3rdparty/genie/build/gmake.darwin'
Creating obj/Release
Converting translation language/Afrikaans/strings.po...
mkdir -p "obj/Release"
python scripts/build/msgfmt.py --output-file language/Afrikaans/strings.mo language/Afrikaans/strings.po
Converting translation language/Albanian/strings.po...
Creating obj/Release/src/host/lua-5.3.0/src
python scripts/build/msgfmt.py --output-file language/Albanian/strings.mo language/Albanian/strings.po
mkdir -p "obj/Release/src/host/lua-5.3.0/src"
Creating obj/Release/src/host
mkdir -p "obj/Release/src/host"
os_getcwd.c
gcc   -MMD -MP -MP -DNDEBUG -DLUA_COMPAT_MODULE -DLUA_USE_MACOSX -I../../src/host/lua-5.3.0/src  -Wall -Wextra -Os -mmacosx-version-min=10.4  -o "obj/Release/src/host/os_getcwd.o" -c "../../src/host/os_getcwd.c"
/nix/store/smimdn17mp2ih6nfxwl78mswwdkmcm56-bash-4.4-p23/bin/bash: gcc: command not found
make[2]: *** [genie.make:406: obj/Release/src/host/os_getcwd.o] Error 127
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory '/private/var/folders/r4/k85s52jx3555ckj0wty615s00000gp/T/nix-build-libretro-mame2016-2020-03-06.drv-0/mame2016-libretro-02987af/3rdparty/genie/build/gmake.darwin'
make[1]: *** [makefile:1453: 3rdparty/genie/bin/darwin/genie] Error 2
make[1]: *** Waiting for unfinished jobs....
Converting translation language/Arabic/strings.po...
python scripts/build/msgfmt.py --output-file language/Arabic/strings.mo language/Arabic/strings.po
make[1]: Leaving directory '/private/var/folders/r4/k85s52jx3555ckj0wty615s00000gp/T/nix-build-libretro-mame2016-2020-03-06.drv-0/mame2016-libretro-02987af'
make: *** [Makefile.libretro:197: build] Error 2
builder for '/nix/store/w1nz7ldp5vh1x1q9fddc6v6ha2c0bf8z-libretro-mame2016-2020-03-06.drv' failed with exit code 2
error: build of '/nix/store/w1nz7ldp5vh1x1q9fddc6v6ha2c0bf8z-libretro-mame2016-2020-03-06.drv' failed

Builds currently fail with `ar` trying to operate on what are clearly
two paths concatenated together.  It stems from a backward-incompatible
change in Make:

> Previously appending using '+=' to an empty variable would result in
> a value starting with a space.  Now the initial space is only added
> if the variable already contains some value.  Similarly, appending an
> empty string does not add a trailing space.

This issue was first reported on the MAME repository proper
(mamedev/mame#6248), and affects libretro's
2016 snapshot as well.  A fix that is reported to work with previous
versions of Make was upstreamed to:
- GENie, the build system: bkaradzic/GENie#493
- MAME: mamedev/mame#6262
- libretro: libretro/mame2016-libretro#47

The fetched patch comes from the last of these.

(cherry picked from commit 8880179)
@AluisioASG AluisioASG force-pushed the aasg/20.09-libretro-mame2016 branch from 2fdf99f to 22873ba Compare October 30, 2020 18:29
@AluisioASG
Copy link
Contributor Author

AluisioASG commented Oct 30, 2020

I've rebased onto #102145 and marked it as broken on Darwin only. I can reproduce the error on Linux using clangStdenv, so I'll try to make that work on Linux and open a new PR to fix it on master.

@SuperSandro2000
Copy link
Member

@AluisioASG pinge me if you need help testing on darwin

@AluisioASG AluisioASG mentioned this pull request Oct 31, 2020
10 tasks
AluisioASG added a commit to AluisioASG/nixpkgs that referenced this pull request Oct 31, 2020
It was found out in the course of NixOS#102078 that the libretro.mame*
packages did not build on macOS, both because of an unconditional
dependency on alsa-lib (which is exclusive to Linux) and references
to GCC scattered across makefiles.

This commit makes alsa-lib a dependency only on Linux, and tries to
force usage of the generic `cc`/`c++` commands.  Ideally we'd refer
to the `$CC` and `$CXX` environment variables, but that seems to
introduce recursive expansion problems for make.

Thanks to @SuperSandro2000 for suggestions and actual testing.
@hrdinka hrdinka merged commit 10b97bd into NixOS:release-20.09 Nov 1, 2020
@hrdinka
Copy link
Contributor

hrdinka commented Nov 1, 2020

Thanks for the backport as well 👍

@AluisioASG AluisioASG deleted the aasg/20.09-libretro-mame2016 branch November 1, 2020 22:02
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.

3 participants