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

cv with drupal7 multisite #54

Open
pcurrier opened this issue Nov 8, 2019 · 3 comments
Open

cv with drupal7 multisite #54

pcurrier opened this issue Nov 8, 2019 · 3 comments

Comments

@pcurrier
Copy link

pcurrier commented Nov 8, 2019

I've been playing around with cv in a drupal7 multisite setup, and I've run into two related problems:

  1. The only bootstrap levels that work are classloader/settings; booting drupal will fail (at the http redirect in includes/install.inc:install_goto()). It works if I set $_SERVER[HTTP_HOST] = 'mysite.org' in Bootstrap.php+CmsBootstrap.php, before they simulate the web environment. (Is there a way to detect this kind of redirect failure? Right now it fails silently which is not ideal.)

  2. cv's search for civicrm.settings.php does not find anything. I'd expect cv to find the settings file if I first cd into [cmsroot]/sites/mysite (or one of its children), similarly to how it works with drush. But findDrupalDirs() tries to guess folder names under sites/ based on $options['httpHost'] (pulled from $_SERVER[HTTP_HOST]) which is empty. So it ends up checking just [cmsroot]/sites/ and [cmsroot]/sites/default.

So it seems like cv needs a "--hostname" option, that would be used to set $_SERVER[HTTP_HOST]. This would be analogous to drush's --uri option, and would address both problems.

I also wonder if the settings file search procedure should be changed to make findCivicrmSettingsPhp() also check all ancestor directories (starting with cwd) for the settings file. This will help with drupal's sites.php aliasing, where the site directory name and host name might be different.

Does something along these lines seem reasonable? You can see a preliminary version of it here: https://github.com/pcurrier/cv/commits/master

@totten
Copy link
Member

totten commented Jan 9, 2020

Yeah, cv does need better support for multisite!

For a bit of background - there are two boot protocols in cv:

  • The Bootstrap.php protocol is a refactored version of civicrm-core's civicrm.config.php files, which means that it first reads civicrm.settings.php and then asks CRM_Utils_System to call the CMS's boot system. If you want to change the boot options persistently, you can set CIVICRM_SETTINGS=/srv/buildkit/build/dmaster/web/sites/default/civicrm.settings.php to point to a different file.
  • The CmsBootstrap.php protocol is a rewrite that tries to improve some things - namely:
    • (1) it boots the CMS first (s.t. CLI-logic better resembles web-logic)
    • (2) the the env-var CIVICRM_BOOT addresses some chicken/egg issues ("What if civicrm.settings.php doesn't exist yet?") and accepts a more useful range of inputs than CIVICRM_SETTINGS. (Ex: CIVICRM_BOOT="Drupal://srv/buildkit/build/dmaster/web?host=dmaster.example.org")

What I like about CIVICRM_BOOT is the adaptability...

  • With things like D7's multisite or WP's split-up file-hierarchies or (god-forbid) deployments that have multiple CMSs, I imagine it's hard (impossible?) to get autodetection working for all use-cases. In cases where the CMS needs some funky bits to boot correctly, you can toss those into CIVICRM_BOOT as extra params.
  • Adding some more params to CIVICRM_BOOT should be easier than adding new CLI options.
  • When scripting, the env-var propagates automatically.
  • There are many tools for managing env-vars ( ~/.bashrc, https://direnv.net/, ad nauseum)

So... I've been angling to recommend CIVICRM_BOOT as the main knob for fine-tuning the bootstrap. Naturally, that hasn't happened... cv still defaults to Bootstrap.php because I've been scared of breaking things and prefer an opt-in. I think cv is gonna have both protocols for the foreseeable future.

I also wonder if the settings file search procedure should be changed to make findCivicrmSettingsPhp() also check all ancestor directories (starting with cwd) for the settings file. This will help with drupal's sites.php aliasing, where the site directory name and host name might be different.

Heh, I hadn't read this issue and was playing with your PR and tried e0639c8. Great minds think alike? Or is that a different spin on similar idea? It allows:

$ cd /var/www/sites/example.com/files
$ cv url civicrm/admin
"http://example.com/civicrm/admin"

I have to admit... the sites.php/aliasing stuff scares me. 🙃 Maybe it's just because I've never used them. Or maybe it's a nagging sensation of non-determinism. It might help if you could explain this bit more. (Or post code... whatever's easiest.)

Tangentially: Do you think it's possible to take a CWD like /var/www/sites/example.com/files and startup Drupal... without starting up Civi (e.g. allowing CmsBootstrap.php to autodetect the subsite)?

@pcurrier
Copy link
Author

pcurrier commented Jan 9, 2020

Heh, I hadn't read this issue and was playing with your PR and tried e0639c8. Great minds think alike? Or is that a different spin on similar idea?

Yeah, originally I was testing something similar to this (searching ancestor directories for the settings file). But in the time between opening issue #54 and submitting PR #57, I changed direction and decided to teach findDrupalDirs() about sites.php (this is what you see in 7f01345). Originally I wanted to avoid any unnecessary duplication of drupal logic in cv... but findDrupalDirs() is already copied directly from the D7 function for finding settings.php, so that didn't seem like much of an objection. But really I think either approach is fine.

I have to admit... the sites.php/aliasing stuff scares me. 🙃 Maybe it's just because I've never used them. Or maybe it's a nagging sensation of non-determinism. It might help if you could explain this bit more. (Or post code... whatever's easiest.)

There's really not much to it. sites.php is a mapping of site URLs to directory names. By default, if I have the site "foo.example.org" then drupal will expect to find the files for that site in the "sites/foo.example.org" directory. But if I want to name the directory "sites/myFolder", then in sites.php I would put:

$sites = array (
  'foo.example.org' => 'myFolder',
);

That's it -- it contains the $sites array, nothing else. It just allows site admins more control over how the site is structured. So technically, the current code in findDrupalDirs() is incomplete if it doesn't account for sites.php (in fact, it's likely that whoever wrote findDrupalDirs stripped out the sites.php parts when they pulled the logic from the drupal function).

Tangentially: Do you think it's possible to take a CWD like /var/www/sites/example.com/files and startup Drupal... without starting up Civi (e.g. allowing CmsBootstrap.php to autodetect the subsite)?

Yes, I think it should be possible, and in fact I was initially looking into doing something like that -- if CWD is /var/www/html/sites/example.com/files, then cv should be able to find the settings file without any hints from --hostname etc. What stopped me IIRC was that it seemed like there would be some sequence-of-events issues -- i.e. making sure that you've always figured out the CMS info before you look for civicrm.settings.php. Given that the CMS and Civi can currently boot in either order, this looked like it might require more elaborate surgery in cv's boot process, probably better done by someone who's not a cv neophyte like me. So I punted on that approach. I think this is exactly what you're getting at when you mention always booting the CMS first, and other chicken-or-the-egg issues? Perhaps I should revisit this -- do you think it would be simpler than I'm picturing?

On a somewhat related note, I've recently been testing the upgrade:get and upgrade:dl commands (after uncommenting them), and they work as expected, but they do need some minor tweaks for multisite. Basically, the --hostname param (and probably others like --level I think?) need to be passed down to sub-commands (e.g. when upgrade:dl calls vars:show), so that drupal can boot. Would you like to address this as part of this PR, or handle it when the upgrade commands eventually get enabled? I have some test code here: c4c0e54 (might have merge conflicts with your multisite branch but I can rebase if it would help).

@totten
Copy link
Member

totten commented Jan 10, 2020

On a somewhat related note, I've recently been testing the upgrade:get and upgrade:dl commands (after uncommenting them), and they work as expected, but they do need some minor tweaks for multisite. Basically, the --hostname param (and probably others like --level I think?) need to be passed down to sub-commands (e.g. when upgrade:dl calls vars:show), so that drupal can boot.

I suspect it would be cleanest to switch those commands from --level=full to --level=cms-only or --level=cms-full and then do any fine-tuning of bootstrap via env-var (CIVICRM_BOOT). The env-vars are propagated to subcommands automatically.

. Given that the CMS and Civi can currently boot in either order, this looked like it might require more elaborate surgery in cv's boot process, ...

Yeah, that is a curveball. It helps me to think of those as two entirely separate protocols and deal with them at different times. So do some work on the Civi-first protocol (Bootstrap.php); do a mental reset; and then separately work on the CMS-first protocol (CmsBootstrap.php).

But if I want to name the directory "sites/myFolder", then in sites.php I would put:

$sites = array (
 'foo.example.org' => 'myFolder',
);

OK, seeing it in that format makes it easier to recognize the ambiguity, e.g. this looks to be valid:

$sites = array (
  'foo.example.org' => 'myFolder',
  'bar.example.org' => 'myFolder',
);

If you're in the folder /var/www/sites/myFolder/files and you inspect the $sites array, then you could guess that foo.example.org or bar.example.org would be legit values of HTTP_HOST (but you wouldn't know which is preferred). That's probably OK -- provided that both values are actually legal HTTP_HOST values.

The rub is that these rules are complicated. The path ./sites/123.a.b.c (or alias $sites['123.a.b.c']) could correspond to http://a.b.c:123 or http://123.a.b.c or http://123.a.b/c or (I think) http://z.y.a.b.c:123. I see how to infer some URI that will provoke Drupal into loading the intended settings.php, but that doesn't mean we'll get the preferred HTTP_HOST or even a valid one. (It's nice if URLs generated in CLI tasks are valid...)

So I punted on that approach. I think this is exactly what you're getting at when you mention always booting the CMS first, and other chicken-or-the-egg issues? Perhaps I should revisit this -- do you think it would be simpler than I'm picturing?

I think there are two basic options here:

  • Support automatic/implicit multisite for specific/limited cases (i.e. exact matching of hostname). If you're willing to limit to this use-case, then I do think it's doable for CmsBootstrap.php and Bootstrap.php to both autoboot multisite.
    • Pro: Works for a lot of people in a common case?
    • Con: For folks in the other cases, it's probably going to fail in quirky ways. (I don't see how to proactively detect that automatic multisite is working correctly or incorrectly.)
  • Do not support automatic/implicit multisite. Instead, anyone using multisite must do some extra config (e.g. setting CIVICRM_BOOT).
    • Pro: Single procedure to document for anyone using multisite
    • Con: Requires configuration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants