Install dependencies when testing themes

Created on 13 November 2024, about 1 month ago

Problem/Motivation

I am trying to run tests on a theme that depends on a couple of modules. The dependencies are declared in info.yml and composer.json. The upgrade_status test fails with the error "Unable to install theme: '…' due to unmet module dependencies".

Proposed resolution

Install dependencies when testing themes.

Remaining tasks

Implement.

User interface changes

None.

API changes

None.

Data model changes

None.

πŸ› Bug report
Status

Active

Component

Container Build Process

Created by

πŸ‡¨πŸ‡¦Canada Liam Morland Ontario, CA πŸ‡¨πŸ‡¦

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Liam Morland
  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    This looks like a GitLab Templates concern. drupalci_environments manages the container images not the template jobs.

  • πŸ‡¬πŸ‡§United Kingdom jonathan1055

    The upgrade status jobs log shows

    The repository at "/builds/project/gcds" does not have the correct ownership and git refuses to use it:
    fatal: detected dubious ownership in repository at '/builds/project/gcds'

    not sure if that is relevant though.

    At the end we just have

    In ThemeInstaller.php line 176:
     Unable to install theme: 'gcds' due to unmet module dependencies: 'simplify_menu, twig_tools'.  

    I don't think this is a fault in gitlab templates, is it? simplify_menu β†’ does not have a Drupal 11 version yet.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    This might actually be a Drupal or drush thing.

    It seems that when we do drush -y en module_name, it automatically installs the dependencies.
    But when doing drush -y theme:enable theme_name, it does not.

    We should try that in isolation, I think that's the culprit here, because the files are present (see composer job log).

  • πŸ‡¬πŸ‡§United Kingdom jonathan1055

    the files are present (see composer job log).

    Ah, yes, of course. I've not worked on the 'Upgrade Status' job, so I'm not familiar with what it does. The pipeline is running with drupal_core 10.3.6 so it does not matter that simplify_menu has no D11 version. It is already loaded.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    Yeah, I really think that this should be a drush issue that we can open upstream.

    drush theme:install theme_name should behave as drush pm:install module_name and install the dependencies automatically, but it's not the case.

  • πŸ‡¬πŸ‡§United Kingdom jonathan1055

    @liammorland Testing with https://www.drupal.org/project/gcds β†’ locally is not so straight-forward because it does not appear to have any public packaged releases for Composer to get. I guess I could go back to downloading the zip file. Or do you know offhand of other themes which require modules?

  • πŸ‡¨πŸ‡¦Canada Liam Morland Ontario, CA πŸ‡¨πŸ‡¦

    at_theme does, but doesn't declare the dependency in its info file. I don't know of any others.

    You should be able to install the dev version of gcds with Composer.

  • πŸ‡¨πŸ‡¦Canada Liam Morland Ontario, CA πŸ‡¨πŸ‡¦

    On admin/appearance, any theme that depends on modules that are not installed will have this message:

    This theme requires the listed modules to operate correctly. They must first be installed via the Extend page.

    So, Drush is following what Drupal is doing by nor automatically installing the modules.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    It would be ideal if the drush command would have a "enable dependencies" flag or something.

    Not ideal, but we could introduce a new variable just for this job called _UPGRADE_STATUS_MODULES_TO_ENABLE, and then do something like this: vendor/bin/drush --root=$_WEB_ROOT -y en $_UPGRADE_STATUS_MODULES_TO_ENABLE right before trying to enable the module/theme.

  • πŸ‡¨πŸ‡¦Canada Liam Morland Ontario, CA πŸ‡¨πŸ‡¦

    Could it not get those from the info file?

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    We are trying to not have Drupal-specific in the templates.

    We'd need to find the info file (which doesn't need to be on the root of the repo) and extract the dependencies. These dependencies could be in the old format (webform, ban) or new format (webform:webform, drupal:ban), those dependencies could have semver modifiers (linkit:linkit (>= 6)), etc.

    That's a lot of logic that should not live in the CI templates. To me, it should live in the "drush" command where we enable the template (similarly to what the module enable does), or otherwise design an easy workaround that would not unblock common cases (that's why I suggested the variable, which is not ideal, but would unblock things).

  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    We are trying to not have Drupal-specific logic in the templates.

    Devils advocate:

    Upgrade status itself is a Drupal specific job, it has no context outside of Drupal. Additionally the templates have a significant amount of β€œDrupal-specific” logic that is not slated for removal. Every time the templates copy files from core to use as a base (rather than require each contrib maintainer provide their own copy) or tries to set values like chrome-driver targets, etc it is Drupal specific logic being done by the templates not the maintainer.

    A fundamentally overreaching question is should this job be a first class part of the templates? If so then the job really needs to handle the situations it is given. If a maintainer has performed their responsibility to provide a dev-requires and an info.yml, to me the maintainer has done their part as the job was specced during its initial feature request discussion.

    A declared value absolutely works, however does that fit into the spirit of how the job is suppose to fit into the ecosystem?

    Presumably the upgrade status bot does this detection already as part of its process and may have some code to be learned from?

    From a data processing standpoint this sounds like extract the entities and obtain the project names from a known regex (something like [:alnum:\:]?(:alnum:) .* ) We already know what info.yml we are looking for based on the module name value as such we can look for a specific single file (find ./ -name $MODULE_NAME.info.yml). Caveat: if a parent theme then requires another parent theme or other modules not declared in the current info.yml that could not be obtained from just the single info.yml and would require recursion.

    #3459888: Job Upgrade Status: Variable MODULE_NAME should be overridable and the drush command should also work for themes β†’ has some relevant discussion on the shortcuts the job is currently taking regarding (incorrect) assumptions in layout.

    Would Drush doing this make life easier, absolutely, however until Drush supports such a feature it is the caller(aka the template) responsibility to use tools inside their limitations.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    Agree with many of the points but i still think that providing a fix upstream to drush, if accepted, can be more beneficial to the community rather than writing the logic in here.

  • πŸ‡¨πŸ‡¦Canada Liam Morland Ontario, CA πŸ‡¨πŸ‡¦
Production build 0.71.5 2024