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.

Sign up for GitHub

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

Jump to bottom

Support Drush 9.0.0-rc1 + #656

Merged
merged 5 commits into from
Dec 5, 2017
Merged

Support Drush 9.0.0-rc1 + #656

merged 5 commits into from
Dec 5, 2017

Conversation

pjcdawkins
Copy link
Collaborator

@pjcdawkins pjcdawkins commented Dec 4, 2017 •
edited

Fixes issue #655

hanoii
hanoii reviewed Dec 4, 2017
View reviewed changes
src/Service/Drush.php Outdated
[$this->getDrushExecutable(), 'version', '--format=string'],
// Run the version check in the home directory, to avoid a Drupal
// installation giving a misleading result.
$this->getHomeDir()
Copy link
Contributor

Choose a reason for hiding this comment

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

Was peeking a bit on this PR. Wouldn't this be inconsistent with site-local drush. You could or could not have a global drush installed. And even if you do, what you do for a project is specific to the local version of drush, not the global one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm. This is running the site-local Drush (if available), because the getDrushExecutable() returns an absolute path, but it's running it with the home directory as the CWD. Although I can't remember exactly what problem this solves anymore (it seems like a workaround for earlier Drush 9 versions again).

@pjcdawkins
Fix Drush 9 alias filename
38bd072
@pjcdawkins
Fix Drush 9 alias not found check
ed3bf21
@pjcdawkins
Avoid ensureInstalled() when checking for Drush version
8e4e312
@pjcdawkins
Remove CWD workaround in Drush version check
ab412da
@pjcdawkins
Drush service clean-up
45f1592 
* Move to class properties instead of static variables for caches
* Cache the Drush executable (in a class property)
* Fix aliases cache property, to run "drush sa" less often
@pjcdawkins pjcdawkins merged commit 61d881f into platformsh:development Dec 5, 2017
@pjcdawkins pjcdawkins deleted the fix-drush-9-aliases branch December 5, 2017 12:43
@pjcdawkins
Copy link
Collaborator Author

Merged for release on approx. 6th December

@hanoii
Copy link
Contributor

hanoii commented Dec 9, 2017

Not related to this issue, but just mentioning. Drush 9-rc1 doesn't seem to work properly on platform. Running a drush cim is giving me:

 [error]  Directory /app exists, but is not writable. Please check directory permissions.


  [Symfony\Component\Console\Exception\CommandNotFoundException]
  Command "cim" is not defined.

Apparently as /app is not writable, config commands are not even exposed to drush.

@pjcdawkins
Copy link
Collaborator Author

pjcdawkins commented Dec 9, 2017 •
edited

@hanoii add a mount for .drush (e.g. https://github.com/platformsh/platformsh-example-drupal8/pull/48/files). Drush now wants to cache files in $HOME/.drush/cache (in platform.sh apps, $HOME is /app).

@hanoii
Copy link
Contributor

hanoii commented Dec 11, 2017

@pjcdawkins thanks!

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

@hanoii hanoii hanoii left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pjcdawkins @hanoii