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

jetbrains.idea-community: add darwin support #64467

Merged
merged 1 commit into from Sep 23, 2019

Conversation

uri-canva
Copy link
Contributor

@uri-canva uri-canva commented Jul 9, 2019

Motivation for this change

Add darwin support to jetbrains.idea-community.

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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@uri-canva uri-canva requested a review from edwtjo as a code owner July 9, 2019 07:19
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jul 9, 2019
@uri-canva
Copy link
Contributor Author

cc @Mic92

@uri-canva
Copy link
Contributor Author

ping @edwtjo

1 similar comment
@uri-canva
Copy link
Contributor Author

ping @edwtjo

@uri-canva
Copy link
Contributor Author

cc @sonercirit @baracoder @c0bw3b

Copy link
Contributor

@baracoder baracoder left a comment

Choose a reason for hiding this comment

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

I am not an idea user, but, so I did not test in depth, but the changes look good to me.

@@ -28,7 +28,7 @@ with stdenv; lib.makeOverridable mkDerivation rec {

nativeBuildInputs = [ makeWrapper patchelf p7zip unzip ];

patchPhase = ''
patchPhase = if stdenv.hostPlatform.system == "x86_64-darwin" then "" else ''
Copy link
Member

@Mic92 Mic92 Sep 23, 2019

Choose a reason for hiding this comment

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

Suggested change
patchPhase = if stdenv.hostPlatform.system == "x86_64-darwin" then "" else ''
patchPhase = optionalString (!stdenv.isDarwin) ''

@@ -63,7 +63,7 @@ with stdenv; lib.makeOverridable mkDerivation rec {
item=${desktopItem}

makeWrapper "$out/$name/bin/${loName}.sh" "$out/bin/${execName}" \
--prefix PATH : "$out/libexec/${name}:${stdenv.lib.makeBinPath [ jdk coreutils gnugrep which git ]}" \
--prefix PATH : "$out/libexec/${name}:${jdk}/jdk/Contents/Home/bin:${stdenv.lib.makeBinPath [ jdk coreutils gnugrep which git ]}" \
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
--prefix PATH : "$out/libexec/${name}:${jdk}/jdk/Contents/Home/bin:${stdenv.lib.makeBinPath [ jdk coreutils gnugrep which git ]}" \
--prefix PATH : "$out/libexec/${name}:${optionalString (!stdenv.isDarwin) "${jdk}/jdk/Contents/Home/bin"}:${stdenv.lib.makeBinPath [ jdk coreutils gnugrep which git ]}" \

@@ -29,20 +34,20 @@ let drv = stdenv.mkDerivation rec {
jrePath=$out/jre
'';

postFixup = ''
postFixup = if stdenv.hostPlatform.system == "x86_64-darwin" then "" else ''
Copy link
Member

Choose a reason for hiding this comment

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

see optionalString

find $out -type f -perm -0100 \
-exec patchelf --interpreter "$(cat $NIX_CC/nix-support/dynamic-linker)" \
--set-rpath "$rpath" {} \;
find $out -name "*.so" -exec patchelf --set-rpath "$rpath" {} \;
'';

rpath = lib.makeLibraryPath ([
rpath = if stdenv.hostPlatform.system == "x86_64-darwin" then "" else (lib.makeLibraryPath ([
Copy link
Member

Choose a reason for hiding this comment

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

see optionalString

@uri-canva
Copy link
Contributor Author

Addressed comments.

@Mic92 Mic92 merged commit 11e3888 into NixOS:master Sep 23, 2019
@vcunat
Copy link
Member

vcunat commented Sep 24, 2019

jetbrains.rider wasn't among your aims, right? 9892d5a

@Mic92
Copy link
Member

Mic92 commented Sep 24, 2019

I don't think rider was tested. But it might just work as it is on macOS.

@Mic92
Copy link
Member

Mic92 commented Sep 24, 2019

Thanks for fixing the evaluation!

@vcunat
Copy link
Member

vcunat commented Sep 24, 2019

It wouldn't evaluate, so I don't think it was really tested :-) Anyway, feel free to fix and test, of course.

@uri-canva
Copy link
Contributor Author

No it wasn't, but I can have a quick look, if it's simple enough I'll fix that.

@uri-canva uri-canva deleted the intellij-community-darwin branch September 24, 2019 11:03
@uri-canva
Copy link
Contributor Author

Yeah looks like it's an easy fix: #69349

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-do-you-discover-the-override-attributes-for-a-derivation/5214/23

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

5 participants