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

dotnet-sdk: add darwin support, set myself as maintainer #68709

Closed
wants to merge 1 commit into from
Closed

dotnet-sdk: add darwin support, set myself as maintainer #68709

wants to merge 1 commit into from

Conversation

reset
Copy link

@reset reset commented Sep 13, 2019

This commit adds support for the dotnet core dotnet-sdk to the Darwin platform. This is my first commit to the nixpkgs repository so it also includes a modification to the maintainers list to include an entry for myself.

Apologies if I've gotten any of the contributing guidelines wrong, I did read them, I promise!

  • 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)
  • Ensured that relevant documentation is up to date
  • Fits [CONTRIBUTING.md] https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md).

@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-linux: 0 labels Sep 13, 2019
Signed-off-by: Jamie Winsor <jamie@onemoregame.com>
Copy link
Member

@LnL7 LnL7 left a comment

Choose a reason for hiding this comment

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

This new expression has a lot of duplication with the default variant, conditionalizing the sources and patchelf calls would be better.

eg.

buildPhase = ''
  runHook preBuild
  ${stdenv.lib.optionalString stdenv.isLinux ''    
    patchelf --set-interpreter "${stdenv.cc.bintools.dynamicLinker}" ./dotnet
    patchelf --set-rpath "${rpath}" ./dotnet
    find -type f -name "*.so" -exec patchelf --set-rpath '$ORIGIN:${rpath}' {} \;
  ''}
  echo -n "dotnet-sdk version: "
  ./dotnet --version
  runHook postBuild
'';

-change /System/Library/Frameworks/Security.framework/Versions/A/Security \
${Security}/Library/Frameworks/Security.framework/Security \
-change /System/Library/Frameworks/GSS.framework/Versions/A/GSS \
${GSS}/Library/Frameworks/GSS.framework/GSS {} \;
Copy link
Member

Choose a reason for hiding this comment

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

Frameworks and things like libc are impure on darwin so rewriting references doesn't really do anything. It's fine to keep the system references since this is a precompiled binary anyway.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, thank you so much for the heads up there. It wasn't clear to me that the Darwin implementation wasn't impure. ❤️

@reset
Copy link
Author

reset commented Dec 11, 2019

The work in #73262 is actually exactly what I believe we should be building on. I'm going to close this in favor of conditionalizing that work as @LnL7 suggested above. Will re-open once that's merged!

@reset reset closed this Dec 11, 2019
@ldub
Copy link

ldub commented Mar 6, 2020

@reset looks like #73262 has been merged. Any chance you're still working on getting dotnet-sdk to work on darwin?

@reset
Copy link
Author

reset commented Mar 9, 2020

@ldub yes, the work continued here #78787. I haven't had the time to revisit the PR to clean it up but I'll make a note to do it this week!

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.

None yet

4 participants