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.

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

Add package parameters to the dav_svn mod. #750

Merged
merged 1 commit into from
Jun 30, 2014

Conversation

Arakmar
Copy link
Contributor

@Arakmar Arakmar commented Jun 1, 2014

Add package parameters to the dav_svn mod to be more convenient.

I also removed the custom loadfile_name as it produces a duplicate load file on Debian.
It's because load files installed by packages aren't purged on Debian.

@igalic
Copy link
Contributor

igalic commented Jun 3, 2014

Yeah, this code is the reason, init.pp

  if ! defined(File[$mod_dir]) {
    exec { "mkdir ${mod_dir}":
      creates => $mod_dir,
      require => Package['httpd'],
    }
# Don't purge available modules if an enable dir is used
    $purge_mod_dir = $purge_configs and !$mod_enable_dir
    file { $mod_dir:
      ensure => directory,
      recurse => true,
      purge => $purge_mod_dir,
      notify => Class['Apache::Service'],
      require => Package['httpd'],
    }
  }

I wonder if we should rethink that stance. By now it's pretty clear that most people want puppet to manage the configuration, rather than the package manager. Those who don't can set purge_configs => false


if $authz_svn_enabled {
::apache::mod { 'authz_svn':
loadfile_name => 'dav_svn_authz_svn.load',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is broken in Debian this needs to be updated with a selector or something, maybe

loadfile_name => $::osfamily ? {
  'Debian' => undef,
  default  => 'dav_svn_authz_svn.load',
},

At least on RHEL-based systems httpd wouldn't start without the loadfile_name due to authz_svn being loaded before dav_svn.

@Arakmar
Copy link
Contributor Author

Arakmar commented Jun 5, 2014

@igalic Yes, I think the same. It would be better to purge this directory even if an enable mechanism is available.

@mhaskel Thanks for the explanation. Nothing is broken on Debian but it was to be consistent with files added by the Debian package. So I'll add what you've suggested.

@Arakmar
Copy link
Contributor Author

Arakmar commented Jun 15, 2014

Pull request updated.
I removed package parameters as I didn't see that the package was defined in apache::params.

@apenney
Copy link
Contributor

apenney commented Jun 26, 2014

any chance you can rebase this one for us? It's not mergable as is.

Fix acceptance tests due to the new naming.
@Arakmar
Copy link
Contributor Author

Arakmar commented Jun 26, 2014

Rebase done

@hunner
Copy link
Contributor

hunner commented Jun 26, 2014

Thanks! Will wait for travis ci first.

@hunner
Copy link
Contributor

hunner commented Jun 27, 2014

Failures related to #778 ; will try to re-run them.

hunner added a commit that referenced this pull request Jun 30, 2014
Add package parameters to the dav_svn mod.
@hunner hunner merged commit 1449585 into puppetlabs:master Jun 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants