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

added macOS compatibility to portmidi-3.17 #24855

Closed
wants to merge 3 commits into from
Closed

added macOS compatibility to portmidi-3.17 #24855

wants to merge 3 commits into from

Conversation

plumps
Copy link
Contributor

@plumps plumps commented Apr 12, 2017

Motivation for this change

Needed portmidi for pygame. Haven't changed a thing about the missing java builds.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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.

@mention-bot
Copy link

@plumps, thanks for your PR! By analyzing the history of the files in this pull request, we identified @aszlig, @wkennington and @fpletz to be potential reviewers.

@plumps plumps changed the title added macOS compatibility to portmidi-3.17, java situation is unchanged added macOS compatibility to portmidi-3.17 Apr 12, 2017
@LnL7 LnL7 added the 6.topic: darwin Running or building packages on Darwin label Apr 13, 2017
url = "https://raw.github.com/Rogentos/argent-gentoo/master/media-libs/"
+ "portmidi/files/portmidi-217-cmake-libdir-java-opts.patch";
sha256 = "1jbjwan61iqq9fqfpq2a4fd30k3clg7a6j0gfgsw87r8c76kqf6h";
});
}) ++ optional stdenv.isDarwin ./darwin.patch;
Copy link
Member

@LnL7 LnL7 Apr 13, 2017

Choose a reason for hiding this comment

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

Looks like you forgot to commit this file

error: getting status of ‘/nix/store/hljk2wdrny0rzdx9krl1zrqh2disv0ha-git-export/pkgs/development/libraries/portmidi/darwin.patch’: No such file or directory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are so right o.O., fixed that

substituteInPlace ./${path}/CMakeLists.txt --replace "CoreMIDI.framework" \
"${CoreMIDI}/Library/Frameworks/CoreMIDI.framework"
substituteInPlace ./${path}/CMakeLists.txt --replace "CoreServices.framework" \
"${CoreServices}/Library/Frameworks/CoreServices.framework"
Copy link
Member

Choose a reason for hiding this comment

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

This seems to replace the same things as the patch, why not replace it directly here?

Copy link
Member

Choose a reason for hiding this comment

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

To clarify, I think something like this should make most of the patch file redundant.

substituteInPlace pm_dylib/CMakeLists.txt \
    --replace '${FRAMEWORK_PATH}/CoreAudio.framework' "${CoreAudio}/Library/Frameworks/CoreAudio.framework"
...

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 13, 2017

@plumps are you still working on this?

@plumps
Copy link
Contributor Author

plumps commented Nov 13, 2017

@c0bw3b nope

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 14, 2017

@NixOS/darwin-maintainers does someone feel like resuming/completing this or should we close for now?

Copy link
Member

@matthewbauer matthewbauer left a comment

Choose a reason for hiding this comment

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

See @LnL7's suggestions

@c0bw3b
Copy link
Contributor

c0bw3b commented May 18, 2019

Stalled

@c0bw3b c0bw3b closed this May 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict 6.topic: darwin Running or building packages on Darwin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants