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

Migrate Tools update broke our CI #1994

Closed
seth-shaw-unlv opened this issue Dec 2, 2021 · 14 comments
Closed

Migrate Tools update broke our CI #1994

seth-shaw-unlv opened this issue Dec 2, 2021 · 14 comments

Comments

@seth-shaw-unlv
Copy link
Contributor

Two weeks ago the Migrate Tools dev branch removed support for Drupal 8. Our testing matrix still includes testing on Drupal 8 which now fails. See @ajstanley 's recent PR build failure.

So, we can either drop Drupal 8 support in our testing matrix OR lock down Migrate Tools in our composer to 5.0 rather than ^5.

Do we have a preference between these two, @Islandora/8-x-committers?

@whikloj
Copy link
Member

whikloj commented Dec 2, 2021

If we are not dropping support for Drupal 8, then I vote for locking it for the time being.

@seth-shaw-unlv
Copy link
Contributor Author

There are several committers on the Sprint Call right now that vote for dropping D8 from the testing matrix since it is EOL anyway.

@dflitner
Copy link

dflitner commented Dec 2, 2021

I agree that we should drop D8 since it's EOL.

I don't know how much it might be used in the Islandora community so do we need to lock it for now and phase it out with an announcement? I think it could be a fairly short phase out period but if folks don't think that's needed then yes we should just get rid of it.

@rosiel
Copy link
Member

rosiel commented Dec 2, 2021

I'm for we drop D8.

@seth-shaw-unlv
Copy link
Contributor Author

So, if we drop D8, is that a major version change, even though our API didn't change? Or something else?

@bseeger
Copy link
Member

bseeger commented Dec 2, 2021

I'm for dropping 8 and for a major version change - is this just for the islandora module (for starters)?

@seth-shaw-unlv
Copy link
Contributor Author

Bringing over observations made in a Slack thread:

Although, is there a reason why we have to include migrate-tools? Would this better be placed in an install profile?
Yes, we use the Migrate API to pull [islandora model terms] in, but that is part of an installation process, not how the code works. Migrate-tools is simply a useful tool for using the Migrate API.

So, we could just remove the dependency from islandora's composer and info.yml, relying instead on deployment methods to include it.

@seth-shaw-unlv
Copy link
Contributor Author

@bseeger: I'm for dropping 8 and for a major version change - is this just for the islandora module (for starters)?

Yes, only islandora would need to be 3.0. However, modules that include islandora as a dependency would also have to update their composer to be islandora/islandora:^2 || ^3 or drop D8 themselves and shift their major versions as well.

@seth-shaw-unlv
Copy link
Contributor Author

seth-shaw-unlv commented Dec 3, 2021

Building on my earlier comment about the third option, removing migrate_tools as an islandora dependency, I made PR 859 (linked above). It appears that this fixes the CI for the islandora module.

However, this puts the onus on deployment methods (e.g. ansible and isle) to install and enable migrate_tools themselves if they use a Drush version less than 10.4.

I know the playbook uses Geerling Guy's drush role which is configured to use Drush 9, so it would either need to override the Geerling Guy config or add migrate_tools to the drupal_composer_dependencies config and drupal_enable_modules config.

It looks like, for isle, we would just need to update the composer.json for the islandora_install_profile_demo which is currently set to use Drush 9 as well. Someone correct me if I'm wrong on that. Edit: I just spun up a fresh make local and it is giving me Drush 10.3.6. I'm guessing that is because we don't preemptively install Drush and composer respected the request for migrate tools conflicting with later Drush versions.

Oh, side-note: it appears that simply dropping D8 from the matrix won't help us without locking down composer in the CI to Drush < 10.4 because, even with D9, migrate_tools ^5 will still conflict with Drush 10.6, which the CI is grabbing before checking any other composer requirements and currently isn't us letting us downgrade Drush.

@alxp
Copy link
Contributor

alxp commented Dec 3, 2021

The Drush GeerlingGuy role may default to version 9 but it is overridden in the Playbook or the Drupal install itself is hosting Drush in its own structure, which the Drush launcher is happily running.

vagrant@islandora8:/var/www/html/drupal/web/sites/default$ drush --version
Drush Launcher Version: 0.6.0
Drush Commandline Tool 10.6.1

This is in the install-profile branch, so it should be merged to dev soon.

@seth-shaw-unlv
Copy link
Contributor Author

@alxp, is that with the current dev branch or the new install-profile one?

@alxp
Copy link
Contributor

alxp commented Dec 3, 2021

The new one, there was some conflict that prevented an update to Drupal 9 if Drush 9 was pegged.

@seth-shaw-unlv
Copy link
Contributor Author

Looks like the dev branch is also asking for 10.3+.

@seth-shaw-unlv
Copy link
Contributor Author

Fixed with Islandora/islandora@90d6795.

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

6 participants