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: add support for plugins on darwin #242562

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

FabianPonce
Copy link
Contributor

@FabianPonce FabianPonce commented Jul 10, 2023

Description of changes

This change fixes up support for installing plugins with JetBrains IDEs on macOS. Presently on master, this falls with the following:

error: attribute 'mainProgram' missing

       at /nix/store/apa072dsj782n2r23his9y40nm8nkkrm-source/pkgs/applications/editors/jetbrains/plugins/default.nix:84:15:

           83|     stdenv.mkDerivation rec {
           84|       pname = meta.mainProgram + "-with-plugins";
             |               ^
           85|       version = ide.version;
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Copy link
Member

@leona-ya leona-ya left a comment

Choose a reason for hiding this comment

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

Hey! Thanks for the PR. I tested this on aarch64-darwin and it works fine.

I have some remarks about code style / simplifications

pkgs/applications/editors/jetbrains/plugins/default.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/jetbrains/plugins/default.nix Outdated Show resolved Hide resolved
@FabianPonce
Copy link
Contributor Author

Thanks for the feedback, @leona-ya. I've applied those suggestions.

@leona-ya
Copy link
Member

One thing that needs to be done, before (from my side) this could be merged: Please squash all commits into one with a commit message like jetbrains: add plugins support for darwin or jetbrains: fix plugin support for darwin.

Also I think the plugin updates (d1fecc3c9d65d55dc81c8aaa217ca9354da084f0, 0af0af96a51c3a444d02bdb198148d62f878fadf and a part of c05db562799477fc45421ce713f3a33b914b9416) shouldn't be included in this PR, as they'll be adressed in #242493 and might cause strange merge conflicts then

@FabianPonce
Copy link
Contributor Author

Good point, I'd been using this locally while I waited for the original plugins PR to be merged, but that noise isn't necessary in the git changeset. I've squashed this all down to 1 commit for you, let me know how that looks.

Copy link
Member

@leona-ya leona-ya left a comment

Choose a reason for hiding this comment

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

LGTM now :) As mentioned: tested on aarch64-darwin

@Janik-Haag
Copy link
Member

pinging @GenericNerdyUsername for review since they initially wrote the plugin support.

@GenericNerdyUsername
Copy link
Contributor

Why not add meta.mainProgram to darwin builds?

Copy link
Member

@Janik-Haag Janik-Haag left a comment

Choose a reason for hiding this comment

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

Just the one question everything else looks good to me.

pkgs/applications/editors/jetbrains/darwin.nix Outdated Show resolved Hide resolved
@Janik-Haag
Copy link
Member

I'm converting this to a draft PR (as I was suggested in the nixpkgs dev matrix room) until you start responding to questions and mark finished conversations as resolved.

@Janik-Haag Janik-Haag marked this pull request as draft September 12, 2023 18:19
@wegank
Copy link
Member

wegank commented Oct 13, 2023

Merge conflict, please rebase.

@wegank wegank closed this Oct 13, 2023
@tymscar
Copy link
Contributor

tymscar commented Oct 31, 2024

If there any update on this? Seems to still be broken on darwin

@wegank
Copy link
Member

wegank commented Oct 31, 2024

Um, I wonder why I closed the PR instead of rebasing it. I'll see what I can do.

@wegank wegank reopened this Oct 31, 2024
@wegank wegank force-pushed the darwin-idea branch 3 times, most recently from 31d7079 to 2d2b3b1 Compare October 31, 2024 11:22
@wegank wegank marked this pull request as ready for review October 31, 2024 20:05
@wegank wegank merged commit 4765af7 into NixOS:master Nov 1, 2024
26 checks passed
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.

6 participants