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

nixos/tomcat: correct type specification for virtualHosts #44365

Merged
merged 1 commit into from Aug 2, 2018
Merged

nixos/tomcat: correct type specification for virtualHosts #44365

merged 1 commit into from Aug 2, 2018

Conversation

pvgoran
Copy link
Contributor

@pvgoran pvgoran commented Aug 2, 2018 •
edited

The wrong specification was introduced as part of commit 472f16d.

Fixes #44361.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@pvgoran
nixos/tomcat: correct type specification for virtualHosts
7fb40c6 
The wrong specification was introduced as part of commit 472f16d.

Fixes NixOS#44361.
@pvgoran pvgoran force-pushed the tomcat-correct-virtualHosts branch from 3d53fac to 7fb40c6 Compare August 2, 2018 16:37
@infinisil
Copy link
Member

infinisil commented Aug 2, 2018 •
edited

As mentioned in the issue, the webapps option being a list of packaging is somewhat limiting. Can you change this to a list of paths instead? Same as how you defined the webapps option in your changes. Packages are always auto-converted to paths when necessary, so backwards compatibility won't be a problem.

@infinisil
Copy link
Member

Oh I see #44371, you didn't link to it

@infinisil infinisil merged commit 6b20531 into NixOS:master Aug 2, 2018
@pvgoran
Copy link
Contributor Author

pvgoran commented Aug 3, 2018

@infinisil Yes, somehow I didn't think about linking the issues. :)

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
3 participants
@pvgoran @infinisil @GrahamcOfBorg