Skip to content

squirrel-sql: fix the launcher script to allow JDK 19#235096

Closed
khumba wants to merge 1 commit intoNixOS:masterfrom
khumba:squirrel-sql-launcher-fix
Closed

squirrel-sql: fix the launcher script to allow JDK 19#235096
khumba wants to merge 1 commit intoNixOS:masterfrom
khumba:squirrel-sql-launcher-fix

Conversation

@khumba
Copy link
Contributor

@khumba khumba commented May 31, 2023

Description of changes

Squirrel SQL 4.5.1 added support for JDK 19, but the launcher script still requires JDK <=17. This patches the launcher script to check for JDK <=19. This is a belated follow-up to #209299, prompted by #235015.

I confirm that with this change, Squirrel starts now, whereas it errored out before.

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
    • (No reverse dependencies via grep.)
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 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.

Squirrel SQL 4.5.1 added support for JDK 19, but the launcher scripts weren't
updated and still check for JDK <=17.  This patches the scripts to check allow
JDK <=19.
@khumba khumba mentioned this pull request May 31, 2023
12 tasks
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 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. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels May 31, 2023
@raboof
Copy link
Member

raboof commented May 31, 2023

Result of nixpkgs-review pr 235096 run on x86_64-linux 1

1 package built:
  • squirrel-sql

Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Seems to work!

Has this problem/patch been reported upstream? Perhaps we should link to that?

@khumba
Copy link
Contributor Author

khumba commented May 31, 2023 via email

@raboof
Copy link
Member

raboof commented May 31, 2023

Thanks for testing this. I haven't reported it upstream yet. I am planning to do so, but I wanted to respond here first.

👍 (I didn't do any in-depth tests, just start the program)

@khumba
Copy link
Contributor Author

khumba commented Jun 1, 2023

I think something got mixed up in their release process. Upstream has commits adding 18 and 19 to the some but not all of the launcher scripts on the 4.5.1 branch (7978596a, f869b3a1). And for some reason, the released -standard.zip doesn't have the updated launcher in the root of the ZIP. So this patch should be fine to apply.

@mweinelt mweinelt added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 2, 2023
@khumba
Copy link
Contributor Author

khumba commented Jun 9, 2023

This PR is obsoleted by bumping to the fixed version here: #236771

@khumba khumba closed this Jun 9, 2023
@raboof
Copy link
Member

raboof commented Jun 9, 2023

even better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants