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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

geoserver: add extensions and update script #281739

Merged
merged 1 commit into from Feb 20, 2024

Conversation

rollf
Copy link
Contributor

@rollf rollf commented Jan 18, 2024

Description of changes

This PR adds the possibility to run geoserver with extensions. This is achieved by adding the function geoserver.[passthru.]withExtensions that expects a selector function that selects extensions among a given list and returns a geoserver derivation with the selected extensions installed. Example:

geoserverWithImporterExtension = pkgs.geoserver.withExtensions (ps: with ps; [ importer ]);

All extensions are listed in a new file geoserver/extensions.nix. They are fed to withExtensions upon invocation. The standard procedure to install extensions is followed (i.e. unpack extension content into geoserver/webapps/geoserver/WEB-INF/lib). My initial implementation used symlinkJoin. While the server starts, trying to access it results in java.io.FileNotFoundException: Could not open ServletContext resource [/WEB-INF/dispatcher-servlet.xml]. Besides, the wrappers (geoserver-startup, geoserver-shutdown) would have to be rewritten. So the current implementation copies the files from the extension's $out into geoserver's $out. Some extensions have dependencies. This is coped with. Some extensions do not work currently. Those are commented out for now.

Since there are >50 extensions, an update.sh script has been added to automatically update both geoserver as well as all extensions. The script is designed to work by either fetching the latest Geoserver version from GitHub or by providing specific versions. Example:

cd nixpkgs
export DEBUG=1
export UPDATE_NIX_OLD_VERSION=2.24.1
export UPDATE_NIX_NEW_VERSION=2.24.2
./pkgs/servers/geospatial/geoserver/update.sh # update geoserver + all plugins
./pkgs/servers/geospatial/geoserver/update.sh importer # update geoserver + importer plugin

passthru.updateScript is set to this new script and things should work as intended (see Nixpkgs manual section on passthru.updateScript)

nixos/tests/geoserver.nix has been adapted to test several geoserver installations. Crucially, a test is run with (nearly) all extensions enabled. This test checks for known issues in the log file.

I think there is room for improvement in this PR and I'm happy to work in comments. Overall, I think the approach is sound.

ToDos:

  • Add entry to "Other Notable Changes" in the 24.05 release notes?

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.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.

Add a 馃憤 reaction to pull requests you find important.

@rollf rollf mentioned this pull request Jan 18, 2024
13 tasks
@rollf rollf changed the title Geoserver with extensions geoserver: add extensions and update script Feb 10, 2024
@rollf rollf force-pushed the geoserver-with-extensions branch 3 times, most recently from 541c243 to ecacf26 Compare February 10, 2024 06:43
@rollf rollf force-pushed the geoserver-with-extensions branch 2 times, most recently from b339653 to d836af2 Compare February 10, 2024 12:58
@rollf rollf marked this pull request as ready for review February 11, 2024 18:39
@imincik
Copy link
Contributor

imincik commented Feb 12, 2024

Looks like a good job @rollf ! I'll review it soon.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/week-in-geospatial-team/37035/7

Copy link
Contributor

@imincik imincik left a comment

Choose a reason for hiding this comment

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

I tried to build the package with and without extensions, tried to run update script and checked the code. I like the usage of the geoserver.[passthru.]withExtensions.

Looks good OK to me. Thank you @rollf .

@imincik imincik mentioned this pull request Feb 14, 2024
13 tasks
@rollf
Copy link
Contributor Author

rollf commented Feb 15, 2024

Okay, thank you @imincik . I had seen before the label of borg internal error. I think this was me when I polished the PR (I don't remember correctly what it was, maybe I added a syntax error or so). I will remove this label now by hand because the ofborg-eval has been "pending" now for several days.

@rollf
Copy link
Contributor Author

rollf commented Feb 19, 2024

With the release of nix 2.20.0, the invocation of nix hash to-sri ... could be modified to nix hash convert --to sri ... (see release docs). Should I do this right now? I wouldn't know how, though. I'd need to force the script to run nix >= 2.20.0.

@rollf
Copy link
Contributor Author

rollf commented Feb 19, 2024

ofborg-eval doesn't seem to run at all.

@imincik
Copy link
Contributor

imincik commented Feb 19, 2024

ofborg-eval doesn't seem to run at all.

Can you try to force-push to this PR ? It might help to restart eval.

@imincik
Copy link
Contributor

imincik commented Feb 19, 2024

With the release of nix 2.20.0, the invocation of nix hash to-sri ... could be modified to nix hash convert --to sri ... (see release docs). Should I do this right now? I wouldn't know how, though. I'd need to force the script to run nix >= 2.20.0.

Oh, I missed this command. I think we shouldn't use new/experimental CLI in nixpkgs. I suggest to replace it with nix-hash --to-sri. This is actually a good example of the reason why we should stick with old CLI until the new one is stable.

@das-g das-g removed their request for review February 20, 2024 09:30
@ofborg ofborg bot requested a review from imincik February 20, 2024 10:28
@imincik imincik merged commit 35521ef into NixOS:master Feb 20, 2024
23 of 24 checks passed
@imincik
Copy link
Contributor

imincik commented Feb 20, 2024

@rollf , thanks for your work !

@rollf rollf deleted the geoserver-with-extensions branch February 22, 2024 07:30
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/week-in-geospatial-team/37035/8

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

3 participants