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.plugins: fix darwin builds #384811

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

thiagokokada
Copy link
Contributor

@thiagokokada thiagokokada commented Feb 24, 2025

There were 2 issues for usage of jetbrains.plugins.addPlugins in darwin:

  • Some IDE products have spaces in name (e.g.: "IntelliJ IDEA CE"). The current code didn't escape or quoted the strings together, so it failed during build
  • After fixing the issue above, opening the resulting app bundle (e.g.: open result/Applications/IntelliJ IDEA CE) would result in an error because we modified the bundle but kept the old signature

This PR fixes both those issues, the first one by quoting the IDE name using lib.escapeShellArg and the second one by using darwin.autoSignDarwinBinariesHook.

Also it formats the code using the nixfmt-rfc-style and does some general clean-up in the plugin derivation.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Feb 24, 2025
@thiagokokada
Copy link
Contributor Author

Not sure who to ping here, but @GenericNerdyUsername @liff @theCapypara.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Feb 24, 2025
@thiagokokada
Copy link
Contributor Author

CC @leona-ya since you review #321247.

@liff
Copy link
Contributor

liff commented Feb 24, 2025

Looks reasonable, but I don’t have a Mac to test with.

There were 2 issues for usage of `jetbrains.plugins.addPlugins` in
darwin:

- Some IDE products have spaces in name (e.g.: "IntelliJ IDEA CE"). The
  current code didn't escape or quoted the strings together, so it
  failed during build
- After fixing the issue above, opening the resulting app bundle (e.g.:
  `open result/Applications/IntelliJ IDEA CE`) would result in an error
  because we modified the bundle but kept the old signature

This commit fixes both those issues, the first one by quoting the IDE
name using `lib.escapeShellArg` and the second one by using
`darwin.autoSignDarwinBinariesHook`.
@thiagokokada thiagokokada force-pushed the fix-idea-community branch 2 times, most recently from 8293099 to cef5daa Compare February 24, 2025 21:44
@github-actions github-actions bot added the 6.topic: teams Relating to team creation, updates, other management actions label Feb 24, 2025
Copy link
Contributor Author

@thiagokokada thiagokokada left a comment

Choose a reason for hiding this comment

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

Did some more cleanups and add myself as a maintainer. I think the plugins derivation looks much better now.

For reviewers: review by commit there is a commit that is just formatting with nixfmt-rfc-style.

@thiagokokada

This comment was marked as outdated.

@thiagokokada

This comment has been minimized.

2 similar comments
@thiagokokada

This comment has been minimized.

@thiagokokada

This comment has been minimized.

While NixOS#321247 technically "fixed" JAR plugins, it fixed by creating a
broken symlink. The reason it wasn't causing issues before is because
the derivation before set `donFixup = true`, skipping the checks for
broken symlinks in the derivation.

Now that the fixup phase is re-enabled, this is causing the build to
fail if you have a JAR plugin like `wakatime`. Now the symbolic links
are correct, but I am not sure if this is working correctly or not.
@thiagokokada
Copy link
Contributor Author

@ofborg build jetbrains.plugins.tests.idea-ce-with-plugins

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: teams Relating to team creation, updates, other management actions 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants