Skip to content

jetbrains-jdk-17: fix build#425814

Closed
aoli-al wants to merge 1 commit intoNixOS:masterfrom
aoli-al:jdk-fix
Closed

jetbrains-jdk-17: fix build#425814
aoli-al wants to merge 1 commit intoNixOS:masterfrom
aoli-al:jdk-fix

Conversation

@aoli-al
Copy link
Member

@aoli-al aoli-al commented Jul 16, 2025

Things done

This adopts the fix from #419026 and fixes the build for jetbrains.jdk17 as well.

  • 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/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Jul 16, 2025
@nix-owners nix-owners bot requested a review from edwtjo July 16, 2025 15:25
@nixpkgs-ci nixpkgs-ci bot added 9.needs: reviewer This PR currently has no reviewers requested and needs attention. and removed 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Jul 16, 2025
@aoli-al aoli-al mentioned this pull request Jul 16, 2025
Merged
13 tasks
@MattSturgeon
Copy link
Contributor

MattSturgeon commented Jul 16, 2025

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 425814

Logs: https://github.com/MattSturgeon/nixpkgs-review-gha/actions/runs/16329530220

x86_64-linux

❌ 2 packages failed to build:
  • jetbrains.idea-community (jetbrains.idea-community-src)
  • jetbrains.pycharm-community (jetbrains.pycharm-community-src)
✅ 2 packages built:
  • jetbrains.jdk-no-jcef-17
  • jetbrains.jdk-no-jcef-17.debug

aarch64-linux

❌ 2 packages failed to build:
  • jetbrains.idea-community-src
  • jetbrains.pycharm-community (jetbrains.pycharm-community-src)
✅ 2 packages built:
  • jetbrains.jdk-no-jcef-17
  • jetbrains.jdk-no-jcef-17.debug

@aoli-al
Copy link
Member Author

aoli-al commented Jul 16, 2025

Note that this PR itself does not fix the build :(. Need to merge it together with #419026.

@Janrupf
Copy link
Contributor

Janrupf commented Jul 18, 2025 •
edited
Loading

I'm trying to understand whats going on and I'm confused what this PR does. Does the export BOOT_JDK have the desired effect or does it just export an empty variable?

What is the reason for the change in the configure args escaping?

EDIT: I think I figured it out and I believe the changes in this PR aren't correct, at least not all the way. The reason BOOT_JDK now needs to be exported is because due to __structuredAttrs its not passed as an environment variable anymore, but instead as a shell variable directly. IMO the better fix would be to move it to env.BOOT_JDK instead.

I also think its better to export NIX_CONFIGURE_FLAGS="$configureFlags" instead of directly referencing them in the sed command, because if a | somehow ever ends up in the configure flags the sed command will break. This can be solved by substituting the way this was originally done via an environment variable, ie NIX_CONFIGURE_FLAGS

@aoli-al aoli-al mentioned this pull request Jul 18, 2025
Closed
3 tasks
@Janrupf Janrupf mentioned this pull request Jul 18, 2025
Merged
13 tasks
@aoli-al aoli-al closed this Jul 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edwtjo edwtjo Awaiting requested review from edwtjo

Assignees

No one assigned

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aoli-al @MattSturgeon @Janrupf