sous-chefs / php

Development repository for the php cookbook

https://supermarket.chef.io/cookbooks/php

Apache License 2.0

443 stars 491 forks source link

fix: Prevent Apache from being pulled in as a dep on Ubuntu 20.04 (#311) #312

Closed hrak closed 3 years ago

hrak commented 4 years ago

Description

This PR prevents Apache from being pulled in as a dependency on Ubuntu 20.04.

A unit test could be added to make sure Apache doesn't get installed, but i didn't go that far.

Issues Resolved

311

Check List

[X] All tests pass. See TESTING.md for details.
[X] New functionality includes testing.
[ ] New functionality has been documented in the README if applicable.

hrak commented 3 years ago

A unit test could be added to make sure Apache doesn't get installed, but i didn't go that far.

I think it might be a good idea to add this as an InSpec test to ensure this doesn't happen again on future releases. Can you please do that?

@ramereth No problem. You want me to add a new describe block to the existing file test/integration/resource/inspec/php_spec.rb or add a new file there (called bug311_spec.rb or something) with the test?

ramereth commented 3 years ago

I think adding it to test/integration/resource/inspec/php_spec.rb would make the most sense. You'll want to make sure you get the correct name for the package on each platform we test against. Thanks!

hrak commented 3 years ago

I think adding it to test/integration/resource/inspec/php_spec.rb would make the most sense. You'll want to make sure you get the correct name for the package on each platform we test against. Thanks!

This bug only exists on Debian based OS's though, due to how php is packaged (Actually, i just tested Debian 10 and this does not exhibit the same behavior, so it even seems limited to how Ubuntu packages php).

For this reason, i was planning on doing a unless os[:family] == 'redhat' block similar to what is already present in the current php_spec.rb if that's ok.

ramereth commented 3 years ago

I think adding it to test/integration/resource/inspec/php_spec.rb would make the most sense. You'll want to make sure you get the correct name for the package on each platform we test against. Thanks!

This bug only exists on Debian based OS's though, due to how php is packaged (Actually, i just tested Debian 10 and this does not exhibit the same behavior, so it even seems limited to how Ubuntu packages php).

For this reason, i was planning on doing a unless os[:family] == 'redhat' block similar to what is already present in the current php_spec.rb if that's ok.

FWIW it seems RHEL pulls in httpd regardless of what you intend so this seems to be the correct thing to do.