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 drush local support to the Pantheon Backdrop CMS upstream. #6

Open
serundeputy opened this issue Aug 22, 2016 · 41 comments · May be fixed by #65
Open

Add drush local support to the Pantheon Backdrop CMS upstream. #6

serundeputy opened this issue Aug 22, 2016 · 41 comments · May be fixed by #65

Comments

@serundeputy
Copy link
Member

serundeputy commented Aug 22, 2016

This adds:

  • drush toBACKDROP_ROOT/drush/drush
  • backdrop drush extension in BACKDROP_ROOT/drush/drush/commands/backdrop
  • Drush executable is at BACKDROP_ROOT/drush/drush/drush

PR from @serundeputy at #7
PR from @jenlampton at #8
PR from @herbdool at #25
PR from @jenlampton at #65

@jenlampton
Copy link
Member

jenlampton commented Sep 23, 2016

After applying the PR as-is and trying a drush cc all on my local I get:

sh: /.../backdrop/drush/drush/drush.launcher: Permission denied

so I tried changing permissions on my local /drush/ to 775, and then got this error with a drush cc all:

Fatal error: Class 'Robo\Robo' not found in /.../backdrop/drush/drush/includes/preflight.inc on line 243

pushing to pantheon and running the same command terminus drush cc all gives an equivalent error:

Fatal error: Uncaught Error: Class 'Robo\Robo' not found in /srv/bindings/67212ef6bf624df8922f0463000bd130/code/drush/drush/includes/preflight.inc:243
Stack trace:
#0 /srv/bindings/67212ef6bf624df8922f0463000bd130/code/drush/drush/includes/preflight.inc(187): drush_init_dependency_injection_container()
#1 /srv/bindings/67212ef6bf624df8922f0463000bd130/code/drush/drush/includes/preflight.inc(35): drush_preflight_prepare()
#2 /srv/bindings/67212ef6bf624df8922f0463000bd130/code/drush/drush/drush.php(12): drush_main()
#3 {main}
  thrown in /srv/bindings/67212ef6bf624df8922f0463000bd130/code/drush/drush/includes/preflight.inc on line 243
Ok

Reverting this commit (but leaving the pantheon.yml file) and running terminus drush cc all behaves as expected, can't find "Drupal":

No Drupal site found, only 'drush' cache was cleared.

and to confirm we have drush 8 I did drush @pantheon.mysite.dev status and got

PHP executable      :  /srv/bindings/67212ef6bf624df8922f0463000bd130/php/php 
 PHP configuration   :  /srv/bindings/67212ef6bf624df8922f0463000bd130/php.ini 
 PHP OS              :  Linux                                                  
 Drush script        :  /opt/pantheon/drush-8/drush.php                        
 Drush version       :  8.0.5                                                  
 Drush temp          :  /tmp                                                   
 directory                                                                     
 Drush               :  /srv/bindings/67212ef6bf624df8922f0463000bd130/.drush/ 
 configuration          drushrc.php /etc/drush/drush8rc.php                    

Also, my Local site is functional again, drush cc all clears caches:

'all' cache was cleared.   

@jenlampton
Copy link
Member

jenlampton commented Sep 23, 2016

Digging deeper, the version of drush that I am running on my local (8.2-dev) doesn't have use Robo\Robo; in its preflight.inc. What I see in that file is use Drush\Log\LogLevel; and nothing after that. I can't get this version of drush to tell me which version it is, but I suspect it might be Drush 9? Here's drush 8 from the source: https://github.com/drush-ops/drush/blob/8.x/includes/preflight.inc

jenlampton pushed a commit to jenlampton/backdrop-pantheon that referenced this issue Sep 23, 2016
…board, and terminus, can run drush commands.
@jenlampton
Copy link
Member

jenlampton commented Sep 23, 2016

I just filed a new PR with the 8.x branch of drush. this one actually clears my caches! :)

[2016-09-23 04:18:36] [info] Running drush cc all on mysite-dev
    cmd: 'cc all'
    site: 'mysite'
    env: 'dev'
'all' cache was cleared.  

@jenlampton
Copy link
Member

jenlampton commented Sep 23, 2016

sadly, it still won't update my database :( I pushed this version of drush and the pantheon.yml to a site that needed an update for devel module, and even though I can use drush via terminus, the After the Clone Finishes Successfully: then Run update.php checkbox in the pantheon dashboard UI does not run update.php.

via terminus:

[2016-09-23 05:42:38] [info] Running drush updb -y on mysite-dev
    cmd: 'updb -y'
    site: 'mysite'
    env: 'dev'
Performed update: devel_update_1002                                         [ok]
Performed update: devel_update_1003                                         [ok]
Performed update: tablefield_update_1000                                    [ok]
Finished performing updates.                                                [ok]
 Devel  1002  1002 - Delete the Development menu and just put menu items in the 
              Admin Bar.                                                        
 Devel  1003  1003 - Remove option for Krumo skin.                              
 Table  1000  1000 - Update schema to handle machine names of input filter      
 field        formats.
Do you wish to run all pending updates? (y/n): y

Via the UI:
screen shot 2016-09-22 at 10 46 52 pm

@jenlampton
Copy link
Member

jenlampton commented Sep 23, 2016

It looks like the update-with-deploy feature on the Pantheon dashboard is also not working. :(
screen shot 2016-09-22 at 10 46 27 pm

But drush updb works via terminus here too:

terminus drush "updb -y" --site="mysite" --env="test"
[2016-09-23 05:53:11] [info] Running drush updb -y on norcalhj-test
    cmd: 'updb -y'
    site: 'mysite'
    env: 'test'
Performed update: devel_update_1002                                         [ok]
Performed update: devel_update_1003                                         [ok]
Performed update: tablefield_update_1000                                    [ok]
Finished performing updates.                                                [ok]
 Devel  1002  1002 - Delete the Development menu and just put menu items in the 
              Admin Bar.                                                        
 Devel  1003  1003 - Remove option for Krumo skin.                              
 Table  1000  1000 - Update schema to handle machine names of input filter      
 field        formats.
Do you wish to run all pending updates? (y/n): y

@jenlampton
Copy link
Member

jenlampton commented Sep 23, 2016

Looks like the cache clear feature on the Pantheon dashboard doesn't work either:
screen shot 2016-09-22 at 10 55 24 pm

@jenlampton
Copy link
Member

As best I can tell it looks like both terminus and drush using site aliases both recognize the new version of Drush, but the dashboard does not. I'm not sure if that's because the dashboard is hard-wired to use a different version of drush, but it would be great if we could get @greg1anderson or @populist to take a look and see if we are doing something incorrectly.

@jenlampton
Copy link
Member

jenlampton commented Aug 22, 2017

I think maybe we should try this again, now that we can use the pantheon.upstream.yml and see if we can have more success: https://pantheon.io/docs/pantheon-yml/

@greg-1-anderson
Copy link

Drush will detect a site-local Drush if there is a drush in vendor/bin. Your site will need to have a vendor directory with all of Drush's dependencies in any event. If both of these things are true, then any time you use the global Drush on Pantheon, then it will detect your site's Drush and call it.

My prediction is that if you do these two things, then the dashboard cc button will work, and the "urn update.php" checkbox should work. I don't know if site-audit works with Backdrop, so the "Status" panel might not work or might not contain useful information.

@jenlampton
Copy link
Member

Bah that dreaded vendor directory... I never thought to try that...

@herbdool
Copy link
Contributor

Tested this on a local site using composer require "drush/drush:^8" and it does work for clearing cache from the dashboard. I haven't tested anything else.

Not sure best way to include it -- make a PR for the upstream with the vendor folder and composer.lock?

@jenlampton
Copy link
Member

jenlampton commented Aug 23, 2018

I'm of two minds about including a vendor directory in the repo.

On one hand: devs who use Pantheon are probably going to want Drush, and those who don't may never notice it's there.

On the other: I really don't want to add a vendor directory :( -- am I alone in feeling this way?

@greg-1-anderson
Copy link

Over at Pantheon, we have the drops-8 repository, which does not include vendor, and we have example-drops-8-composer which provides an example Composer-managed site with a relocated document root and a vendor directory.

If you only need a standard Drush (e.g. "drush/drush:^8"), then it should work fine for you to use the Pantheon-installed global Drush, I would think. If you include BACKDROP_ROOT/drush/drush/commands/backdrop without Drush, does that work?

@herbdool
Copy link
Contributor

Plus it makes the dashboard work.

I'm ok with the vendor's directory. Maybe we can leave out the composer files?

@greg-1-anderson
Copy link

... or would it be BACKDROP_ROOT/drush/commands/backdrop? Not sure why there is a double-drush in there.

Note that if you include Drush in a vendor directory, then you need to include everything in that vendor directory as well. Otherwise, Drush won't be able to load its dependencies.

@herbdool
Copy link
Contributor

herbdool commented Sep 5, 2018

Alex, have you tested BACKDROP_ROOT/drush/backdrop location as mentioned above by Greg?

kreynen pushed a commit to kreynen/backdrop-pantheon that referenced this issue Sep 6, 2018
@herbdool
Copy link
Contributor

herbdool commented Feb 5, 2020

Got a first client's Backdrop site that's going to be hosted on Pantheon so I'm interested in this again.

I'm attempting to get BACKDROP_ROOT/drush/example to work. A super simple drush command. So far it doesn't recognize it. I'm going to keep playing around with it. Is there an even easier way to test if a command in that folder is recognized?

@serundeputy
Copy link
Member Author

serundeputy commented Feb 5, 2020

@herbdool I think your best bet is drush site local on the site you have.
https://github.com/backdrop-contrib/drush/blob/1.x-1.x/README.md#install-as-site-local

I'm pretty sure i had drush commands working for me on individual sites w/ that setup, but I've not played w/ Pantheon for a few years now.

testing

I would always do terminus SITESTUFF drush cc all or similar.

@herbdool
Copy link
Contributor

herbdool commented Feb 6, 2020

thanks @serundeputy. I'll do it for a quick fix. I'm hoping to have something we can merge upstream here if possible. So we'll see.

@serundeputy
Copy link
Member Author

yeah; we can do that in the upstream, but make sure it works at all or on your site first i think.

@herbdool
Copy link
Contributor

herbdool commented Mar 9, 2020

@greg-1-anderson @serundeputy I've been trying a few different configurations both with a site-local drush and using the default drush (ideally I'd like to get it working with default drush and without needing composer). I've put the Backdrop drush project in these locations with no luck:

  • BACKDROP_ROOT/drush/backdrop
  • BACKDROP_ROOT/drush/commands/backdrop
  • BACKDROP_ROOT/drush/drush/backdrop
  • BACKDROP_ROOT/drush/drush/commands/backdrop

It does recognize the site-local drush in BACKDROP_ROOT/drush/drush/drush but it doesn't help with the backdrop commands. I've assumed that the structure of the backdrop directory is such that the boot file is like: /backdrop/BackdropBoot.php

I've also tried putting an example drush command in BACKDROP_ROOT/drush/drush/commands/example just to see if I can isolate the issue without backdrop and it can't find the example command either.

Thank you for any help.

jenlampton pushed a commit to jenlampton/backdrop-pantheon that referenced this issue Jun 18, 2020
…board, and terminus, can run drush commands.
@herbdool
Copy link
Contributor

@joelsteidl I was just reading how you've set up Stanford sites on Pantheon. I was wondering if you'd be interested in taking this issue on, to get drush working on Pantheon with Backdrop. Or any other in this issue backlog.

Thanks for the articles by the way. Interesting to read about the move to Backdrop.

@greg-1-anderson
Copy link

Pantheon assumes that if you have a site-local Drush, that you're using Composer. If you're not using Composer, you should by hand put a relative symlink from vendor/bin/drush to vendor/drush/drush/drush. That should work.

If it doesn't, I'd be willing to make changes to Drush 8 to help make things work better with Backdrop. I thought that many years ago, we set things up so that you could use Drush 8 + backdrop with only a Drush extension in your Backdrop site. Don't remember where that ended up; maybe Backdrop has introduced more changes that make that harder. I don't know, but I'm willing to work with someone on it. Maybe DrupalCon Portland?

@herbdool
Copy link
Contributor

Awesome @greg-1-anderson - you've been super helpful so far. I hope someone else in the thread will be able to work with you at DrupalCon if things get stuck with this. I'm not sure if I'll have a chance to test your suggestion, but if I do hopefully that does the trick.

@joelsteidl
Copy link

Thanks @herbdool!

I'd be happy to help brainstorm the best approach. This is how I got Backdrop and drush working on Pantheon:

  1. In pantheon.yml I specified drush_version: 8 (not sure if this step is needed)
  2. I ran composer require "drush/drush:^8" in the web root of my Backdrop project. This added /vendor/drush/drush to my project.
  3. I manually placed the backdrop commands in vendor/drush/drush/commands/backdrop (not sure if this is the best place)

We could just do this and commit this to the Pantheon Backdrop Upstream but I'm not sure it is the most sustainable to keep everything updated.

The symlink idea that @greg-1-anderson proposed is interesting because it would avoid the composer files in the upstream. We would just need to update the upstream if there are drush or drush backdrop commands change.

I will unfortunately not be at DrupalCon this year, but happy to collaborate here.

@greg-1-anderson
Copy link

To be clear, my suggestion for making the symlink would be in the case where you were copying Drush to vendor/drush/drush rather than running composer require. If you use composer require, then Composer should create the symlink automatically. Just be sure that the vendor/bin directory is committed to the repository. The symlink is not a substitute for having the Drush files in the repository.

The main problem with putting the backdrop command in vendor/drush/drush/commands is that it will be removed if you ever use Composer to upgrade to a newer version of Drush. It's not a bad thing to do as a workaround / interim measure, especially if you're putting Drush in the upstream, so that only the upstream maintainer need worry about the repair step.

In my message above, I was making a reference to the fact that I thought that we had made changes in Drush to allow the backdrop command to be placed somewhere inside __PROJECT_ROOT__/drush (probably __PROJECT_ROOT__/drush/commands/backdrop). If that worked, then you wouldn't need the site-local Drush; you could use the Pantheon-supplied Drush. Not sure about the current status of that.

jenlampton pushed a commit to jenlampton/backdrop-pantheon that referenced this issue Aug 15, 2022
jenlampton pushed a commit to jenlampton/backdrop-pantheon that referenced this issue Aug 15, 2022
@jenlampton
Copy link
Member

The main problem with putting the backdrop command in vendor/drush/drush/commands is that it will be removed if you ever use Composer to upgrade to a newer version of Drush.

I don't think this scenario will be very likely since most Backdrop sites will not be using composer for anything else, and newer versions of Drush could potentially create other problems for Backdrop sites.

I have created a PR that adds drush using composer, but adds none of the composer files to the project. Drush is placed at __PROJECT_ROOT__/vendor/drush/drush, with the Backdrop Drush extension within.

Just for funzies, I also created an alternate PR that adds the Backdrop drush extension separately, at __PROJECT_ROOT__/drush/commands/backdrop.

Both locations for the Backdrop Drush commands appear to work seamlessly. 👍

herbdool added a commit that referenced this issue Sep 17, 2022
Get tests running and entity_access fix
@laryn
Copy link
Contributor

laryn commented Mar 13, 2023

@joelsteidl has packaged drush 8.4.11 in the vendor folder using composer with a few minor customizations as a patch that run after updating composer -- to make drush 8 work better with PHP 8. The patch is attached here.

drush8.4.11-backdrop-php8-hack.patch

@greg-1-anderson
Copy link

Can you explain the purpose of the two changes in that patch? I hope to have a new release of Drush 8 out shortly; if I can make any changes that make things better for Backdrop &/or PHP 8.2, I will. I don't want to remove the constants, but I am wondering about the addition of _drush_replace_query_placeholders() in drush_db_select().

@laryn
Copy link
Contributor

laryn commented Mar 14, 2023

@greg-1-anderson It's mostly due to the PHP8.x errors laid out in this issue thread:
backdrop-contrib/backdrop-drush-extension#254

The hacks in that patch prevent a few fatal errors and then drush functions again.

PHP Fatal error: Uncaught Error: Class "Drupal\Core\Logger\RfcLogLevel"

PDOException: SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens: SELECT name FROM {system} WHERE type='module' AND status=1; Array

@greg-1-anderson
Copy link

Ah, right, Backdrop does not have Drupal\Core class constants. Does Backdrop have equivalent identifiers? I'm not opposed to some sort of variable-defined conditional in that method to avoid patching for Backdrop. I'd like to avoid hardcoded constants, though. Since this code is being executed, I take it that means there is a watchdog module in Backdrop. Maybe in some backdrop-specific Drush bootstrap file, we could define the Drupal constants? Then we wouldn't even need a conditional in the Drush watchdog code.

Regarding _drush_replace_query_placeholders(), do you think it would likely be innocuous to include this adjustment in Drush core, such that it runs for Drupal code as well? Seems like it might be innocuous. Do you have an opinion on this? I'm still not sure why Backdrop is behaving differently than Drupal here.

@laryn
Copy link
Contributor

laryn commented Mar 14, 2023

@greg-1-anderson There is an equivalent set of identifiers in Backdrop within common.inc:
https://github.com/backdrop/backdrop/blob/269138964f70520e2a7e83cfcd0e04e061edc037/core/includes/common.inc#L8517-L8529

To your second question, I would think it would be innocuous but wouldn't be super confident about saying so for certain, I'm afraid. I am stumped as to why Backdrop is behaving differently here as well!

@greg-1-anderson
Copy link

OK I looked at the code you're patching, and that function starts off with a check:

if (drush_drupal_major_version() >= 7) {

It looks like Drush is going down the Drupal 6 code path when using Backdrop, whereas I would expect it to be more applicable to use the Drupal 7 code path. There are a lot of places in the code where Drush checks the Drupal major version. It seems like it would be a good idea to make Drupal7Boot::get_version() return 7.something when running with a Backdrop site.

@laryn
Copy link
Contributor

laryn commented Mar 15, 2023

Good detective work!

@herbdool
Copy link
Contributor

@greg-1-anderson I assume you mean BackdropBoot::get_version() should not return BACKDROP_VERSION but could hardcode returning 7.x or something like that. https://github.com/backdrop-contrib/backdrop-drush-extension/blob/af29e23aa7a428fb28b42923bf6405b01da4fa56/BackdropBoot.php#L98

@greg-1-anderson
Copy link

Yeah, that's essentially what I meant @herbdool, but that's sort of a hack. A better solution would be if the bootstrap classes introduced new methods e.g. "Drupal7SeriesSite" and etc. that could be version compares in the default implementations, but just be overridden to always return true or false in BackdropBoot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment