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

phpfpm: allow configuring PHP package per-pool #41440

Merged
merged 1 commit into from Jan 21, 2019
Merged

phpfpm: allow configuring PHP package per-pool #41440

merged 1 commit into from Jan 21, 2019

Conversation

wmertens
Copy link
Contributor

@wmertens wmertens commented Jun 4, 2018

all props to @4levels

Motivation for this change

This allows choosing the PHP version for a PHPFPM pool so that you can run multiple versions per system.

I took the work done by @4levels (see the discussion) and combined it into this PR.

Things done
  • 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/)
  • Fits CONTRIBUTING.md.

@wmertens wmertens requested a review from fpletz June 4, 2018 11:17
@4levels
Copy link

4levels commented Jun 4, 2018

Hi @wmertens,

Thanks for this!
I never created a PR for this as this is imo a backward incompatible change. Can you work your magic and add a compatibility update so that this won't break existing configurations when people don't need to specify a custom phpPackage?

Thanks!

@wmertens
Copy link
Contributor Author

wmertens commented Jun 4, 2018

Actually, I think it's backwards compatible. The only that changes is the intermediate representation of the php config, and that's only used in the file itself. The PHP package defaults to the phpfpm-configured package, so that should be ok too…

@wmertens
Copy link
Contributor Author

wmertens commented Jun 4, 2018

anyway it seems I broke eval ;) fixing

@wmertens wmertens force-pushed the php-per-pool branch 3 times, most recently from da406ba to ab0b4cc Compare June 4, 2018 12:20
@4levels
Copy link

4levels commented Jun 4, 2018 •
edited

Hi @wmertens,

Are you sure? I was forced to adjust my phpFpm configurations due to the new nested config attribute. I failed to use the passed set as the new config if no config attribute exists, hope this still makes sense 😉 pm me if it doesn't

@wmertens
Copy link
Contributor Author

wmertens commented Sep 5, 2018

Hmm indeed it might be not compatible. I'll need to do some tests.

@wmertens
Copy link
Contributor Author

Ok I finally realized what's wrong with this: the poolConfigs attribute is used directly in places, and it's a literal string.o

To fix it, I'll combine it in a separate attribute, so it can stay a string.

@4levels
Copy link

4levels commented Oct 26, 2018

Hi @wmertens,
That's exactly what I was trying to explain before, glad you figured it out ;-)
I was kind of hoping the poolConfig could be a string or an attribute set to keep bc compatibility, and would be converted to an attribute set ONLY if it was passed as string (instead of the expected set).

Thanks again for looking into this!

Erik aka 4levels

@wmertens
Copy link
Contributor Author

I now realize that there is no /nixos/tests for phpfpm. I'll see about adding one.

@Ekleog
Copy link
Member

Ekleog commented Nov 18, 2018

(triage) Reading the history is confusing… @wmertens, what is the state of this? I somehow get the feeling that it's ready except it has no test for phpfpm yet. And actually, there is a test for phpfpm: nextcloud uses it.

So, let's try:

@GrahamcOfBorg test nextcloud

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

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

Partial log (click to expand)

Cannot nix-instantiate `tests.nextcloud' because:
error: while evaluating 'recursiveUpdate' at /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/lib/attrsets.nix:415:26, called from /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/lib/attrsets.nix:148:28:
while evaluating 'recursiveUpdateUntil' at /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/lib/attrsets.nix:384:37, called from /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/lib/attrsets.nix:416:5:
while evaluating 'zipAttrsWith' at /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/lib/attrsets.nix:347:21, called from /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/lib/attrsets.nix:394:8:
while evaluating 'zipAttrsWithNames' at /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/lib/attrsets.nix:332:33, called from /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/lib/attrsets.nix:347:27:
while evaluating the attribute 'nextcloud' at /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/nixos/tests/all-tests.nix:145:3:
while evaluating 'handleTest' at /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/nixos/tests/all-tests.nix:17:22, called from /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/nixos/tests/all-tests.nix:145:15:
while evaluating 'discoverTests' at /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/nixos/tests/all-tests.nix:13:19, called from /var/lib/gc-of-borg/nix-test-rs-6/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-6/nixos/tests/all-tests.nix:18:5:
access to path '/nix/store/p8gf704gk28w7hdx82ppgy9xd6nc7xiw-grahamc-aarch64-community-6' is forbidden in restricted mode

@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.nextcloud

Partial log (click to expand)

error: while evaluating 'recursiveUpdate' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:415:26, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:148:28:
while evaluating 'recursiveUpdateUntil' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:384:37, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:416:5:
while evaluating 'zipAttrsWith' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:347:21, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:394:8:
while evaluating 'zipAttrsWithNames' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:332:33, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:347:27:
while evaluating anonymous function at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/lists.nix:113:41, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/lib/attrsets.nix:347:46:
while evaluating the attribute 'nextcloud' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/nixos/tests/all-tests.nix:145:3:
while evaluating 'handleTest' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/nixos/tests/all-tests.nix:17:22, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/nixos/tests/all-tests.nix:145:15:
while evaluating 'discoverTests' at /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/nixos/tests/all-tests.nix:13:19, called from /var/lib/gc-of-borg/nix-root/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/gleber-bastion/nixos/tests/all-tests.nix:18:5:
access to path '/nix/store/8gqkzl5wag8x446wsr6n720yjk9s9bf9-gleber-bastion' is forbidden in restricted mode

@Ekleog
Copy link
Member

Ekleog commented Nov 18, 2018

Oh yeah that issue is still there… second try.

@GrahamcOfBorg build nixosTests.nextcloud

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: nixosTests.nextcloud

Partial log (click to expand)

nextcloud: exit status 0
test script finished in 192.87s
cleaning up
killing client (pid 631)
killing nextcloud (pid 644)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/z0gzsisgd983qm4z6s58wgfypi5n8jdp-vm-test-run-nextcloud-basic
/nix/store/51myk7pgmvwdkga4dbbls3gq2rnyj09d-vm-test-run-nextcloud-with-mysql-and-memcached
/nix/store/jn3rjgrz1l2a4q7vvwkzdf6b5bkqd4gi-vm-test-run-nextcloud-with-postgresql-and-redis

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: nixosTests.nextcloud

Partial log (click to expand)

nextcloud: exit status 0
test script finished in 47.46s
cleaning up
killing client (pid 597)
killing nextcloud (pid 609)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/bhpqbb31w84z7f95c84y0wp6mwfwmxkw-vm-test-run-nextcloud-basic
/nix/store/5b6g38wlb72v3vf6xs53c06aib4khr5j-vm-test-run-nextcloud-with-mysql-and-memcached
/nix/store/sg53sjrbl53sk1ldsggyp0n0j8vk4hi3-vm-test-run-nextcloud-with-postgresql-and-redis

@wmertens
Copy link
Contributor Author

Sweet! Let's just merge this then, right @Ekleog @fpletz?

@wmertens
Copy link
Contributor Author

puts hands before eyes, peeks through fingers, pushes button

@wmertens wmertens merged commit e445eab into NixOS:master Jan 21, 2019
@wmertens wmertens deleted the php-per-pool branch January 21, 2019 07:35
@aanderse aanderse mentioned this pull request Feb 14, 2019
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpletz fpletz fpletz approved these changes

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
6 participants
@wmertens @4levels @Ekleog @GrahamcOfBorg @fpletz @aanderse