error when mailx package already defined

[wyardley]

I had an issue where even when invoking with
class { 'postfix': mailx_ensure => False }
I got the error:
Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Duplicate declaration: Package[mailx] is already declared in file /etc/puppet/modules/base/manifests/install.pp:36; cannot redeclare at /etc/puppet/modules/postfix/manifests/packages.pp:11 on node xxxx Warning: Not using cache on failed catalog Error: Could not retrieve catalog; skipping run
Didn't seem to work to just wrap in a
if $postfix::mailx_ensure {
so this seems to be the easiest way to work around a situation where the mailx package is already defined (I tried using ensure_packages() instead in the place where we define it, but that didn't work either).

[wyardley]

Not sure about the ones that failed, looking through the log, seems like not related to this change, but an ssh connection error?

[raphink]

It doesn't shock me that the postfix module would install the mailx package. I would recommend that you remove it from your base module instead.

[wyardley]

The PR doesn't stop installing mailx, it just makes it a softer dependency.

Just because it "doesn't shock" you, doesn't mean it's a good idea, unless it's depended on for some functionality of the module, having mailx isn't necessary for installing / configuring Postfix, and I would argue that they serve different functions (MUA vs. MTA).

People may not be including the Postfix module on all their systems (as is the case for us), or may have other modules which also install it mailx.

I think the failing tests are due to an unrelated issue (you'll see that all recent PRs are failing checks).

[raphink]

OK. In that case, please add a $manage_mailx boolean parameter to the class to opt-out from managing this package altogether. This will avoid "magic" dynamic management of resources, which makes debugging a headache.

[wyardley]

Sure, makes sense. Will make a new PR for that

[wyardley]

#143