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

dolphinEmuMaster: Enable Darwin building #31546

Closed

Conversation

lukeadams
Copy link
Contributor

@lukeadams lukeadams commented Nov 12, 2017

Motivation for this change

This PR enables Darwin building for Dolphin.

Things done

It also adds the required WXGTK31 package and cleans up how we patch mbedtls on darwin.

Dolphin generates an application bundle instead of a binary in bin on Darwin, but I can add a symlink to the binary if needed.

Potential improvement: go through the list of External deps and use nix-provided ones to reduce the binary size.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@LnL7 LnL7 added the 6.topic: darwin Running or building packages on Darwin label Nov 13, 2017

enableParallelBuilding = true;

nativeBuildInputs = [ pkgconfig ];
buildInputs = [ gcc cmake bluez ffmpeg libao mesa gtk2 glib pcre
nativeBuildInputs = [ cmake gcc pkgconfig ];
Copy link
Member

Choose a reason for hiding this comment

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

why is gcc in here?

@@ -8,7 +8,7 @@ stdenv.mkDerivation rec {
sha256 = "042q1l4708zjn5v72sa9qdvgx173kmy4hbcd23wj5vqd6vbmk6d9";
};

nativeBuildInputs = [ perl ];
nativeBuildInputs = [ perl ] ++ stdenv.lib.optionals stdenv.isDarwin [ fixDarwinDylibNames ];

patchPhase = stdenv.lib.optionalString stdenv.isDarwin ''
substituteInPlace library/Makefile --replace "-soname" "-install_name"
Copy link
Member

Choose a reason for hiding this comment

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

fixing the install_name here to use the absolute store path should allow us to get rid of the postInstall patching.

, common
}:

common.overrideDerivation (super : rec {
Copy link
Member

Choose a reason for hiding this comment

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

not sure about using overrideDerivation in of nixpkgs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah... on second thought the override should be in top-level like this:

wxGTK31 = wxGTK30.overrideDerivation (super : rec {
...
)};

Or are you saying override should be avoided entirely? It seems to be used a handful of times in top-level.

@lukeadams
Copy link
Contributor Author

lukeadams commented Nov 23, 2017

  • Updated to 2017-11-22
  • Removed need for mbed/gcc - remnants from initial tests
  • moved wxgtk31 into all-packages

@lukeadams lukeadams force-pushed the dolphin-master-darwin-enable branch 2 times, most recently from d18344e to f215ede Compare December 18, 2017 23:11
@lukeadams
Copy link
Contributor Author

  • Rebased onto master
  • Bumped to 2017-12-18
  • use overrideAttrs instead of overrideDerivation, moving wxGTK31 out of all-packages

@MP2E
Copy link

MP2E commented Dec 24, 2017

Thanks for your patience, been meaning to check out this PR for some time. I went through the changes and it all looks good to me, I also tested the changes on my NixOS machine and Dolphin is working well. Pushed to master!

@MP2E MP2E closed this Dec 24, 2017
@lukeadams lukeadams deleted the dolphin-master-darwin-enable branch May 26, 2021 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants