The platform config is pretty crazy, here is a snippet from my config

  Options Indexes FollowSymLinks MultiViews
  AllowOverride All
  Order allow,deny
  Allow from all
[...]
Options -Indexes
[...]
Options +FollowSymLinks
[...]
AllowOverride none

I know the final result is the same, but it would make the config files cleaner if just the desired config was generated.

CommentFileSizeAuthor
#4 platform_AllowOverride.patch1.26 KBguypaddock

Comments

Anonymous’s picture

Anonymous (not verified) commented 
li94-211:~/.drush/provision$ grep -r AllowOverride *
platform/verify.provision.inc: * This basically loads the regular .htacess but adds an AllowOverride
platform/verify.provision.inc:AllowOverride none
web_server/provision_apache_platform.tpl.php:    AllowOverride All

So we have an inconsistency here, and more so, we probably shouldn't have duplication in the first place I think? 

function provision_drupal_provision_apache_dir_config($data = null) {
  $htaccess = file_get_contents(dirname(__FILE__) . "/provision_drupal_htaccess.tpl.php");
  $htaccess .= <<<EOF

# Do not read the platform's .htaccess
AllowOverride none

EOF;
  # the trailing newline above is necessary
  return $htaccess;
}

How come we don't just put that AllowOverride none into the provision_drupal_htaccess.tpl.php anyway?

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Fixed

Committed a fix. I left in the weird EOF AllowOverride thing, as there is obviously meant to be method in the madness. I dunno, taking all that out and sticking an AllowOverride none at the end of the provision_drupal_htaccess.tpl.php with a new line underneath it worked just as well... maybe I'm missing something.

guypaddock’s picture

guypaddock commented

I support this fix -- we have a custom-configured environment here, which means we're modifying a few of the template files. We need the ability for clients to use .htaccess files, but we don't want to modify the actual *code* of Provision. But, with the "AllowOverride none" option specified in code rather than the template, we have to modify the code.

guypaddock’s picture

guypaddock commented
Status: Fixed » Needs review
StatusFileSize
new platform_AllowOverride.patch1.26 KB

Correction: I mis-interpreted the fix. I don't think that the AllowOverride option should be specified in "verify.provision.inc" at all. All other relevant Apache options are specified in the template -- why put this one in code?

The attached patch moves the option out of code and into the template, while preserving the EOF functionality.

anarcat’s picture

anarcat commented
Status: Needs review » Needs work

The reason that AllowOverride is outside of the main template is that I aim to simply drop the template from provision and use the platform's .htaccess instead. We shouldn't have to maintain our own copy of Drupal's .htaccess: we should just use it and add our own customization, if possible. In this particular case, it is possible.

The right way to fix this while keeping the ability to modify this through templates (which are aimed to be pluggable, btw) is to make provision_drupal_provision_apache_dir_config() use an extra template for that particular setting *or* simply to bite the bullet and start using the platform .htaccess *plus* our local template.

anarcat’s picture

anarcat commented
Status: Needs work » Closed (won't fix)

So short of starting to parse the .htaccess, i consider this a non-issue. With the latest git (f9d17fffdbc7), we directly include Drupal's .htaccess instead of maintaining our own copy. We append to the end of that file the configuration we require for aegir to function properly, but that's all.

To remove duplicates, you'd had to parse the resulting config file and eliminate duplicates, which is no small feat. If you are interested in doing that or think it's really necessary, feel free to reopen. I think that Apache does a better job at this than we could ever do ourselves, so I am closing this for now.

The place that change would need to happen is here:

  $data['extra_config'] .= join("\n", drush_command_invoke_all('provision_apache_dir_config', $data));

...in _provision_apache_create_platform_config() in web_server/provision_apache.drush.inc:184.

  • Commit 94006b1 on debian, dev-dns, dev-envobject, dev-koumbit, dev-log_directory, dev-migrate_aliases, dev-multiserver-install, dev-newhooks, dev-nginx, dev-ports, dev-purgebackup, dev-restore, dev-services, dev-simplerinstaller, dev-site_rename, dev-ssl, dev_server_verify, prod-koumbit, dev-ssl-ip-allocation-refactor, dev-1205458-move_sites_out_of_platforms, 7.x-3.x, dev-subdir-multiserver, 6.x-2.x-backports, dev-helmo-3.x by mig5: 
    #728140 - try to wean the platform apache config off crack

  • mig5 committed 94006b1 on 
    #728140 - try to wean the platform apache config off crack

  • mig5 committed 94006b1 on 7.x-3.x-1966886-context-to-entity 
    #728140 - try to wean the platform apache config off crack

  • mig5 committed 94006b1 on 6.x-2.x-1995506-profile-option 
    #728140 - try to wean the platform apache config off crack