Islandora / islandora

Drupal modules for browsing and managing digital repositories.
http://islandora.ca/
GNU General Public License v2.0
152 stars 118 forks source link

[BUG] Dependency oddities from switch to drupal/islandora #1028

Open JojoVes opened 5 months ago

JojoVes commented 5 months ago

Summary of issue This is more of an issue with Drupal's packaging tripping over itself, but maybe the dependency specifications in the islandora.info.yml could be modified as a workaround.

What steps does it take to reproduce the issue?

Which version of Islandora are you using? This isn't specific to a certain version, but as an example, the latest version (drupal/islandora:2.12.2) can be used.

Any related open or closed issues to this bug report? Related to the move towards requiring the use of drupal/islandora instead of islandora/islandora https://github.com/Islandora/documentation/issues/2270 https://github.com/Islandora/islandora/pull/1020

rosiel commented 5 months ago

Are you getting error messages once you install drupal/action, that say that it's incompatible? I was just playing with a test site on 10.2.7 and I required drupal/action. It seems to be working fine for me.

I think >10.2 means >10.2.0 and not >=10.3 which is what I immediately assumed. https://getcomposer.org/doc/articles/versions.md#stability-constraints

All this to say, I think the solution is for us to require drupal/action in islandora's composer.json.

adam-vessey commented 5 months ago

So yeah, reproducing:

For attempting to install drupal/islandora on Drupal 10.1:

composer create-project drupal/recommended-project:10.1.x 10.1.x
cd 10.1.x
# XXX: Flipping stability, to avoid otherwise messing with `drupal/flysystem` being a beta release.
composer config -- minimum-stability dev
composer require -W drupal/islandora

indeed fails due to drupal/actions:

$ composer require -W drupal/islandora
./composer.json has been updated
Running composer update drupal/islandora --with-all-dependencies
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - drupal/islandora 1.x-dev is an alias of drupal/islandora dev-1.x and thus requires it to be installed too.
    - drupal/islandora[2.8.2, ..., 2.12.2] require drupal/action * -> satisfiable by drupal/action[dev-1.x, 0.0.1, ..., 0.2.1, 1.x-dev].
    - drupal/islandora[dev-1.x, 1.x-dev, 2.0.0, ..., 2.4.1] require drupal/core ^8 || ^9 -> found drupal/core[8.0.0-beta6, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.5.x-dev] but these were not loaded, likely because it conflicts with another require.
    - drupal/islandora 1.0.0 requires drupal/core ^8 -> found drupal/core[8.0.0-beta6, ..., 8.9.x-dev] but these were not loaded, likely because it conflicts with another require.
    - drupal/action 0.0.1 requires drupal/core >=10.3 -> found drupal/core[10.3.0-beta1, 10.3.0-rc1, 10.3.x-dev, 10.4.x-dev, 11.0.0-alpha1, 11.0.0-beta1, 11.0.x-dev, 11.x-dev] but these were not loaded, likely because it conflicts with another require.
    - drupal/action[dev-1.x, 0.0.2, ..., 0.2.1, 1.x-dev] require drupal/core >10.2 -> found drupal/core[10.2.1, ..., 10.4.x-dev, 11.0.0-alpha1, 11.0.0-beta1, 11.0.x-dev, 11.x-dev] but these were not loaded, likely because it conflicts with another require.
    - Root composer.json requires drupal/islandora * -> satisfiable by drupal/islandora[dev-1.x, 1.0.0, 1.x-dev (alias of dev-1.x), 2.0.0, ..., 2.12.2].

You can also try re-running composer require with an explicit version constraint, e.g. "composer require drupal/islandora:*" to figure out if any version is installable, or "composer require drupal/islandora:^2.1" if you know which you need.

Installation failed, reverting ./composer.json and ./composer.lock to their original content.

10.2, does install drupal/actions separately, ending up with both:

adam@compy:~/asdf/10.2.x$ ls web/core/modules/action
action.info.yml        action.links.task.yml  action.permissions.yml action.routing.yml     src
action.links.menu.yml  action.module          action.post_update.php help_topics            tests
adam@compy:~/asdf/10.2.x$ ls web/modules/contrib/action
LICENSE.txt            action.links.menu.yml  action.module          action.routing.yml     help_topics            tests
action.info.yml        action.links.task.yml  action.post_update.php config                 src
adam@compy:~/asdf/10.2.x$
JojoVes commented 5 months ago

The project page for the contrib actions module also mentions usable with Drupal 10.3+, so maybe the programmatic version constraint of >10.2 that they have is a mistake? Because yeah, the issue with 10.2.x is that the contrib actions module does install and results in code duplication as Adam noted.

rosiel commented 5 months ago

I was going to suggest we do the same thing as we did when HAL went to contrib (see jsonld), but it seems we drew a hard line between drupal 8 and 9 (it was at that time) where the version of jsonld that worked with D9 (only), had the contrib module.

It's a little too late to fix 2.12.2 which is installable on Drupal 10.2 but has code duplication 😣. We could create 2.12.3 which doesn't require contrib drupal/actions, then draw a line where newer versions of islandora are compatible with 10.3 and above, and requires the contrib module? I'm open to suggestions, what do you think we should do?

adam-vessey commented 5 months ago

@rosiel : I think the issue is more that: Our composer.json doesn't require drupal/actions (which makes sense, you're not supposed to explicitly require modules provided by Drupal core; we're referring to drupal:actions, meaning explicitly the version of actions from Drupal core, not contrib's naming of actions:actions), so how to remove the requirement that we don't have? Possibly gets into removing the dependency from our .info.yml: It being installed in Composer seems to be an artifact of d.o's funky packaging scripts which gets into adding entries to the composer.json it generates based on the .info.yml contents; however, this in turns gets into something of a conflict because we do require actions to be enabled, no?

adam-vessey commented 5 months ago

Is particularly strange. Throwing together a minimal "project", which can pull the module:

project's `composer.json` ```json { "name": "adam/asdf", "require": { "drupal/islandora": "^2.12" }, "repositories": [ { "type": "composer", "url": "https://packages.drupal.org/8", "canonical": false } ], "minimum-stability": "dev", "prefer-stable": true } ```

Installing it, and seeing in vendor/drupal/islandora/composer.json, it doesn't reference drupal/action (and is still named islandora/islandora, despite being installed into vendor/drupal/[...]; however, in the composer.lock file resulting from the installation, it does have it as a requirement:

"name": "drupal/islandora",
"version": "2.12.2",
[...]
"require": {
    "drupal/action": "*",
[...]
rosiel commented 5 months ago

I wonder what it did before there was a contrib drupal/action. Does it require drupal/media, or drupal/node (which are also in islandora.info.yml)?

Anyway, failing to see a good way that we can deal with this, what is the impact of duplicated code in /core/modules and /modules/contrib? I seem to have been running a dev site for a while, and am not inundated with PHP errors or anything.

rosiel commented 5 months ago

I've asked in drupal slack's #support channel how to deal with this. :shrug: I'm hoping they've got something already set up like "contrib modules override core" or vice versa (probably) or otherwise renders this less of a problem than it seems like.

adam-vessey commented 5 months ago

Suspecting that items pulled via contrib would take precedence over the version from core. Making reference to Drupal (10.2's) extension discovery, it seems like it might fall under the same ordering as project-wide vs site-specific "modules" directories.

Which is to say: Having the two versions of code (via both core and contrib) in the project might not be an actual issue?

More definitive?:

Extensions found later in the search will take precedence over extensions found earlier - unless they are not compatible with the current version of Drupal core.