Add _TARGET_PHP_IMAGE_VARIANT to cater for newer sqlite versions for Drupal 11

Created on 17 May 2024, 4 months ago
Updated 3 September 2024, 5 days ago

Problem/Motivation

From #3447105: DB requirements for next major were changed , where we needed to raise the MySQL version for Drupal 11, the following question was asked by @cmlara:

Do we need to make any changes for SQLITE?

D11 now requires SQLITE 3.45 which I believe now requires the drupalci/sqlite-3 php (Unless I missed a change the other PHP images have 3.26)

Proposed resolution

Investigate if we need to. Core uses these lines for the testing: https://git.drupalcode.org/project/drupal/-/blob/11.x/.gitlab-ci.yml?ref...

    _TARGET_PHP: "8.3-ubuntu"
    _TARGET_DB: "sqlite-3"

Remaining tasks

Investigate and possible MR to fix.

📌 Task
Status

Fixed

Component

gitlab-ci

Created by

🇪🇸Spain fjgarlin

Live updates comments and jobs are added and updated live.
  • Contributed project blocker

    It denotes an issue that prevents porting of a contributed project to the stable version of Drupal due to missing APIs, regressions, and so on.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @fjgarlin
  • Status changed to Needs review 3 months ago
  • 🇪🇸Spain fjgarlin

    In order to keep PHP_VERSION like a valid PHP version number, I've introduced a new variable for the image variant, and then use that variable to choose the right one in the "next major" testing.

    MR: https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/213...

    Ready to review.

  • Status changed to Needs work 3 months ago
  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    Why can't we just always use ubuntu-apache? 🤔 That'd be much simpler. It's very non-obvious when you would have to use which one 🫣

  • Status changed to Needs review 3 months ago
  • 🇪🇸Spain fjgarlin

    I'm afraid that the only "ubuntu" variant created to date is 8.3-ubuntu. See the available PHP images: https://git.drupalcode.org/project/drupalci_environments/-/tree/dev/php?...

    So adding ubuntu to any other PHP version will fail.

  • 🇪🇸Spain fjgarlin

    Once D11.0.0 is released we will need to move some of these variables around. We might do 8.3-ubuntu-apache the default and then change to just "apache" versions for previous variants, but for now this is the best workaround.

  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    Can't we instead pick the appropriate image based on which SQLite version is being requested? 🤔 That'd simplify the DX.

    As #6 indicates: the new variable is a temporary stopgap — it won't (shouldn't) be necessary eventually. But introducing that new variable is equivalent to introducing a new public API.

    The better solution would IMHO to expand

      ################
      # SQLite
      ################
    
      # The miminum supported version of SQLite on modern Drupal.
      CORE_SQLITE_MIN: "3"
    
      # The maximum supported version of SQLite on modern Drupal.
      CORE_SQLITE_MAX: "3"
    
      # The next/prerelease targeted version of SQLite for modern Drupal.
      CORE_SQLITE_NEXT: "n/a"
    
      # The miminum supported version of SQLite on legacy Drupal (Drupal 7).
      CORE_LEG_SQLITE_MIN: "3"
    
      # The maximum supported version of SQLite on legacy Drupal (Drupal 7).
      CORE_LEG_SQLITE_MAX: "3"
    
      # The next/prerelease targeted version of SQLite for legacy Drupal (Drupal 7).
      CORE_LEG_SQLITE_NEXT: "n/a"
    

    👆 That's currently listing versions (plus 7 vs "modern" Druapl). That's fine. But what we should have also is SQLite versions from the "current vs next" Drupal perspective.

    So IMHO it'd be better to add:

      # The miminum supported version of SQLite on NEXT modern Drupal.
      CORE_NEXT_SQLITE_MIN: "3.45"
    

    (and similar for MySQL + MariaDB + PostgreSQL) per https://www.drupal.org/node/3444548

  • 🇪🇸Spain fjgarlin

    I 100% agree with that, the only issue is that we don't have the supporting images for it.

    There is nothing stopping us from using other images, but up until now, and especially because GitLabCI was/is a replacement for DrupalCI, we are only using "drupalci_environments" images (https://git.drupalcode.org/project/drupalci_environments/-/tree/dev/php?...).

    Note that the sqlite error we are getting is not even when we try to use sqlite as DB engine. This is due to "run-tests.sh" needing sqlite to store tests results, IDs, etc.

    php $_WEB_ROOT/core/scripts/run-tests.sh ... --sqlite "sites/default/files/.sqlite" ... $_PHPUNIT_EXTRA
    

    I wonder if we even need that --sqlite part? I don't know enough about the run-tests.sh script to know what'd be the difference.

  • 🇫🇷France andypost

    Technically libsqlite has backward compatible ABI so it can be replaced after PHP was build with different version.

    OTOH it we move out 8.1-8.3 images to Ubuntu Noble from debian then all can use latest version

  • Status changed to Needs work 3 months ago
  • 🇪🇸Spain fjgarlin

    Having the right version would be in all the PHP images would be amazing, but I don't know how much work all that can be.

    What would be the command to get a specific sqlite version that we could run in the scripts section?
    I'd be happy to bring the right sqlite version inside the script section too.

    Either solution would work fine for me.

  • 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺

    This is becoming more and more of an issue IMHO, and I'm surprised so few others are reporting it. 😅

    Ran into this again, now for the Experience Builder module, which must be tested against all different databases (see https://www.drupal.org/node/3444548 ), because it will specifically be using JSON functionality in each of those databases.

    I've been flailing to get it to work, and in https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... I eventually attempted to port the essence of what's in this MR … and that worked. 👍

  • 🇪🇸Spain fjgarlin

    With what we have now, the workaround suggested in #3 is our best bet. It's far from ideal but it could unblock things.

    Alternatively, if I can get a snippet to install "any" sqlite version, I can put things together to use that approach (which is more robust that only having one image with the correct sqlite version...). I'd love to implement this but I'd need some help with that part.

  • 🇫🇷France andypost

    I bet EB can set _TARGET_PHP to 8.3-ubuntu so default to required sqlite version until debian can allow install newer sqlite-dev

  • Status changed to Needs review about 1 month ago
  • 🇪🇸Spain fjgarlin

    Yeah, setting "8.3-ubuntu" fixes the issue for _TARGET_PHP. The MR does the same fix, but in a "cleaner" way, keeping _TARGET_PHP to just a PHP version, instead of having a weird suffix.

    I think I'm going to set it again as "Needs review" as this is the best fix with what we have so far.

  • 🇪🇸Spain fjgarlin

    I've refactored the MR slightly so that we offer the choice, but we don't set it as not all PHP images have the "-ubuntu" variant. Maybe we can also add information to the docs.

  • 🇪🇸Spain fjgarlin

    Done #15,

    The current MR as it stands now changes nothing but allows setting up a variable to get a different image with a newer sqlite version.

  • 🇬🇧United Kingdom jonathan1055

    Changing the title to reflect the final solution. Hope that's the correct interpretation.

    All looks good. Are there any test runs? I will try a plain one with no customisations if there is not one already.

  • 🇪🇸Spain fjgarlin

    Good call on changing the title.
    Just triggered all downstream pipelines: https://git.drupalcode.org/project/gitlab_templates/-/pipelines/248048

  • 🇫🇷France andypost

    So -ubuntu image will be default?

    Then related can be closed 📌 Use Ubuntu images in all CI environments for core Postponed

  • 🇪🇸Spain fjgarlin

    The code in core is completely different from contrib now, as in separately managed, so that issue is still valid.

    And no, the "apache-ubuntu" variant is not made the default, we are just documenting it how to use it with a variable. The default is still "apache". The reason for this is that not all PHP images have the the "-ubuntu" suffix, so it'd break other cases if we make it the default.

  • 🇫🇷France andypost

    I mean for contrib tests running default on 11.x the _TARGET_PHP_IMAGE_VARIANT should be apache-ubuntu

  • 🇪🇸Spain fjgarlin

    This issue is only to add the possibility and document it.

    #3463894: Update templates so 11.0 is the default/current branch will do the necessary tweaks for D11.

    Even tho running the tests requires SQLite itself, it's only when the target database for Drupal is SQLite that we need this higher version (that's why many people are unaffected by this). If all new PHP images will have "-ubuntu" then we will update that value for all contrib, but in the other issue.

    This issue should change nothing, just add the possibility of using the "apache-ubuntu" variant with just a variable change.

  • 🇬🇧United Kingdom jonathan1055

    Added a suggestion to the description text.

    From #18

    Just triggered all downstream pipelines: https://git.drupalcode.org/project/gitlab_templates/-/pipelines/248048

    Do any of them use the alternative value for the new variable? Or maybe we take it that the variable is being used as expected, and this can be RTBC?

  • 🇪🇸Spain fjgarlin

    Yeah, the downstream pipelines that were run show the new variable in use already.

    For example: https://git.drupalcode.org/project/api/-/jobs/2383238

    We see:
    Using Kubernetes executor with image drupalci/php-5.6-apache:production ...

    And the variables below:

    _TARGET_PHP=5.6
    _TARGET_PHP_IMAGE_TAG=production
    _TARGET_PHP_IMAGE_VARIANT=apache
    

    We can make, if we want, an extra test testing the "apache-ubuntu", just to be 100% sure of it, but I'm pretty confident based on the above, as otherwise it would have broken when trying to get the correct image.

  • Status changed to RTBC 24 days ago
  • 🇫🇷France andypost

    I find it ready

  • Pipeline finished with Skipped
    19 days ago
    #259072
    • fjgarlin committed 1efd0384 on main
      Issue #3447792 by fjgarlin, andypost, Wim Leers, jonathan1055: Add...
  • Status changed to Fixed 19 days ago
  • 🇪🇸Spain fjgarlin

    Thanks all. Merged.

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024