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 path for Drush 12/Drupal 10. #9

Closed
wants to merge 1 commit into from

Conversation

rosiel
Copy link
Contributor

@rosiel rosiel commented Aug 3, 2023

I followed some instructions online to add the path to drush (and other vendored binaries) in the Dockerfile.

Solves Islandora-Devops/isle-buildkit#283.

@Islandora-Devops/committers

@rosiel
Copy link
Contributor Author

rosiel commented Aug 3, 2023

Oh no! On further investigation with @aOelschlager it should probably be done in buildkit.

@rosiel
Copy link
Contributor Author

rosiel commented Aug 4, 2023

Rather, it appears buildkit installs drush via the no-longer-supported drush-launcher in drupal/Dockerfile. It's also doing so with version 0.6.0 which is ~3y old.

On the other hand, both the starter site and the install profile (retro compatibility but ok for now) require drush/drush in their composer.json files. This implies to me that drush should, for those two install methods, resolve at /var/www/drupal/vendor/bin/drush. I propose we lean into this, by:

  • removing the drush installation in the drupal container (and by extension the code-server) in buildkit
  • replace it with putting /var/www/drupal/vendor/bin at the FRONT of the path (not the back as is currently done in the code-server image).

@rosiel
Copy link
Contributor Author

rosiel commented Aug 7, 2023

Maybe we can merge this PR as a temporary fix since our main build method is currently broken, and then as part of a larger initiative, investigate how we install drush/drush launcher in buildkit?

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

Successfully merging this pull request may close these issues.

1 participant