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.

Sign up for GitHub

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

Jump to bottom

tomcat service: fix webapps default option #40657

Merged
merged 1 commit into from Jun 10, 2018
Merged

tomcat service: fix webapps default option #40657

merged 1 commit into from Jun 10, 2018

Conversation

fadenb
Copy link
Contributor

@fadenb fadenb commented May 17, 2018

The package referenced in the old default value does not exist in nixpkgs.
If accepted this should also be applied to release-18.03 branch.

Motivation for this change

Users were confused

/cc @armin1402

@fadenb
tomcat service: fix webapps default option
b5854b7 
The old package tomcat.webapps does not exist
@xeji xeji merged commit 1daa771 into NixOS:master Jun 10, 2018
xeji pushed a commit that referenced this pull request Jun 10, 2018
@fadenb @xeji
tomcat service: fix webapps default option (#40657)
3939055 
The old package tomcat.webapps does not exist

(cherry picked from commit 1daa771)
@xeji
Copy link
Contributor

xeji commented Jun 10, 2018

backported in 3939055

@andir
Copy link
Member

andir commented Jun 11, 2018

This breaks evaluation on master (and 18.03) since tomcat85 is not defined in that context. The previous tomcat reference would still work since tomcat referes to cfg.package which (per default) points to pkgs.tomcat85.

Running the bundled nixos-test would have shown:

⇒  nix-build -I nixpkgs=. ./nixos/tests/tomcat.nix --no-out-link
error: undefined variable 'tomcat85' at …/nixpkgs/nixos/modules/services/web-servers/tomcat.nix:112:21
(use '--show-trace' to show detailed location information)

The output from hydra can be found at https://hydra.nixos.org/jobset/nixos/release-18.03#tabs-errors.

Just to be sure lets ask what @GrahamcOfBorg thinks about this.
@GrahamcOfBorg test tomcat

@GrahamcOfBorg
Copy link

No attempt on x86_64-linux (full log)

The following builds were skipped because they don't evaluate on x86_64-linux: tests.tomcat

Partial log (click to expand)

while evaluating the attribute 'optionalValue' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/modules.nix:352:5:
while evaluating the attribute 'values' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/modules.nix:338:9:
while evaluating the attribute 'values' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/modules.nix:434:7:
while evaluating 'concatMap' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/lists.nix:104:18, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/modules.nix:434:16:
while evaluating 'concatMap' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/lists.nix:104:18, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/modules.nix:324:17:
while evaluating anonymous function at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/modules.nix:324:28, called from undefined position:
while evaluating 'dischargeProperties' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/modules.nix:392:25, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/modules.nix:325:62:
while evaluating the attribute 'value' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/types.nix:248:40:
undefined variable 'tomcat85' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/nixos/modules/services/web-servers/tomcat.nix:112:21

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: tests.tomcat

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

@andir
Copy link
Member

andir commented Jun 11, 2018 •
edited

A potential solution would look like this:

--- a/nixos/modules/services/web-servers/tomcat.nix
+++ b/nixos/modules/services/web-servers/tomcat.nix
@@ -109,8 +109,8 @@ in

       webapps = mkOption {
         type = types.listOf types.package;
-        default = [ tomcat85.webapps ];
-        defaultText = "[ tomcat85.webapps ]";
+        default = [ tomcat.webapps ];
+        defaultText = "[ pkgs.tomcat85.webapps ]";
         description = "List containing WAR files or directories with WAR files which are web applications to be deployed on Tomcat";
       };

(It is a bit hacky as it hides the truth about the webapps being picked from the selected tomcat by default... Is there a more elegant way?)

@grahamc grahamc mentioned this pull request Jun 11, 2018
Open
xeji added a commit that referenced this pull request Jun 11, 2018
@xeji
nixos/tomcat: fix eval error introduced by #40657
19d0402 
(cherry picked from commit 93cbb9b)
xeji added a commit that referenced this pull request Jun 11, 2018
@xeji
nixos/tomcat: fix eval error introduced by #40657
93cbb9b
@xeji
Copy link
Contributor

xeji commented Jun 11, 2018

Thanks @andir for catching this and your fix. Applied in 93cbb9b and 19d0402 .
This option probably needs a longer description including an explanation of how the default value is derived. The current description text doesn't fit well anyway (it's a list of packages, not WAR files or directories).

@fadenb fadenb deleted the tomcat-options-fix_master branch August 17, 2019 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@fadenb @xeji @andir @GrahamcOfBorg