Use _TARGET_CORE consistently to override the core version

Here are the test runs I did with the existing template, when working on another issue, that led to my suggestion.

In the top three rows, setting _TARGET_CORE has no effect and all three run core 11.x-dev
In the bottom three rows, the executing version does vary, but _TARGET_CORE is confusingly saying 10.3.x-dev when we are running D11.

The approach seems good in here.

I've tested this in MR 263 and it works apart from one major problem - the variable DRUPAL_CORE is not now defined until the script section of Composer, so if any contrib project was using that variable in a composer before_script then that would no longer work. I think we have to maintain that BC so this approach is not going to work.

An alternative might be simply to use the DRUPAL_CORE variable directly as the preferred (and documented) way to override the core version. We know that _TARGET_CORE is only used in the current and max php composer variants, and it has absolutely no effect in the other four variants. So maybe we deprecate _TARGET_CORE and just use DRUPAL_CORE for all six variants?

Yeah, I've seen a few modules using it, so it needs to be BC.

We are technically using DRUPAL_CORE in all variants already, and I really think that's a better name for it, but _TARGET_CORE was actually in the pre-variants world, so I bet it's also heavily used.

Is there a way to tell the difference between _TARGET_CORE set globally vs _TARGET_CORE set inside of a job/variant?

For example (totally untested):

.create-environment-variables: &create-environment-variables
    - |
       ...
       if [ "$INTERNAL_TARGET_CORE_CHECK" !== "" && "$INTERNAL_TARGET_CORE_CHECK" != "$DRUPAL_CORE" ]; then
           export DRUPAL_CORE=$INTERNAL_TARGET_CORE_CHECK"
       fi
       ...


composer (max PHP version)
   ...
   variables:
     INTERNAL_TARGET_CORE_CHECK: !reference ["composer (max PHP version)", variables, _TARGET_CORE]

So we are testing if _TARGET_CORE has been set inside the variant and putting it in INTERNAL_TARGET_CORE_CHECK, and then when we are creating the env variables, we just use that value.

We'll need to document that, if _TARGET_CORE is used inside a job, it will overwrite whatever is in DRUPAL_CORE.

---

The most simple fix would be to fix the documentation for _TARGET_CORE and say that it is ONLY used for the current variants.
We have a variants page https://project.pages.drupalcode.org/gitlab_templates/info/variants/ where we explain how to add them or modify them.

I think I actually prefer this, but I'm open to opinions.

_TARGET_CORE was actually in the pre-variants world,

ah, that explains quite a bit why it is used in two variants but not the other four.

Your totally untested idea is interesting, but like you I think it is over-complicating in this particular case. I would really like to deprecate _TARGET_CORE and just use DRUPAL_CORE for all four variants. I have pushed a revised MR and tested it here. Backwards-compatibility is maintained for jobs that have _TARGET_CORE and I think this overall makes it simpler to explain and understand.

It may be particularly important to have a simpler way to override the core version in some variants when we make the shift in 📌 Update templates so 11.0 is the default/current branch RTBC

I really like this solution, it's clean and BC compatible.
Downstream pipelines: https://git.drupalcode.org/project/gitlab_templates/-/pipelines/300676

Tested with _TARGET_CORE: 10.2.0 for current composer and both _TARGET_CORE: "10.0.x" and DRUPAL_CORE: "10.1" in Max PHP variant
https://git.drupalcode.org/project/scheduler/-/pipelines/300662

Also tested with global _TARGET_CORE: 10.1 incorrecty defined at the very top level, in addition to the above. The BC only affects the current and Max
https://git.drupalcode.org/project/scheduler/-/pipelines/300644

Merged! I will tag 1.5.10 and make that the default ref in preparation for Monday's changes.

Thanks, this is great to have it committed before 1.6.0. Updated the issue summary to refelect the second approach.

Just a couple of minor things to fix

• The changelog url has 2024-10-04 which is correct, but the date shown is 2024-10-03, I think this should also be today 10-04
• The url in include.drupalci.hidden-variables.yml#L286 still has the xxx- placeholder

I can open a MR for these if you want, or you can commit it directly.

I have created MR264 to correct the changelog urls.

In doing so, and checking the link worked, I noticed that actually none of the urls scroll you the the desired place in the file because the anchor needs three hyphens to match the text in the title ## 1.5.10 - 2024-10-04. The spaces either side of the first hyphen are also translated into hyphens, so we need three in the url.

fjgarlin → closed merge request !264

That must have been a change at some point we didn't realize of. All of them fixed via commits (I saw the MR late).

Thanks. In the MR you closed, there was also one other fix, to give useful info in the changelog now that _TARGET_CORE has gone, replace it with DRUPAL_CORE. That would have been correct even back when that change was done, and it is useful to retain the info. See attached.

I saw it, but because it was an old entry, I thought it'd also be ok to leave it as is, but if DRUPAL_CORE was already available at the time, then it's all good as it is applicable. I changed it. Sorry I didn't see the MR in time and I'm messing around with direct commits but thanks for reporting these things.

No need to apologise about the MR. Yes that message would have been valid with DRUPAL_CORE back when the change was made. In fact it is more correct, because you had to use DRUPAL_CORE for the other four variants, back then.

Note: this is a visual review only, I have not converted a project in GitlabCi to use the latest commit to 100% validate the below.

It appears this commit has removed _TARGET_CORE from the UI. We will no longer have a pre-populated field for users to select the target version in manual pipeline runs. An average user will now need to know the name of this internal variable when running manual pipeline runs. This is a negative change to DX.

With DRUPAL_CORE as the replacement for _TARGET_CORE it appears we will be regressing 🐛 All variants run the same Drupal version when the pipeline is triggered via web Fixed as making a change to DRUPAL_CORE via the UI will impact ALL test runs instead of just the base job.

Side note:
We keep calling it the 'current' job however its main purpose has been to be a 'base job' used both for real time interaction via the UI and for maintainers to tweak for their deployment needs of what they define as their 'normal' day to day test.

Hi cmlara,
Thanks for raising this. We did have a discussion about this point (maybe it was on slack). It comes down to a balance between setting variables in the UI vs. developer setting the versions they want in the .gitlab-ci.yml. Also using _target_core in the UI form could never change the version in the other variants, which is more likely to be needed, and was confusing.

making a change to DRUPAL_CORE via the UI will impact ALL test runs instead of just the base job.

A better way would be for the user to enter a value for the specific variable that is used in the variant they are interested in. These are documented on this page.

Maybe we could clarify this somewhere. I don't think we have a specific documentation page for running pipelines via the UI. That was written in the old doc pages before we had our MkDocs.

Thinking about this a bit more, I don't know of any good reason why these five variables defining the core versions have to be hidden. We did do a bulk clearance of variables out of the form and into .hidden-variables.yml but maybe these could move back.

CORE_SUPPORTED
CORE_SECURITY_PREVIOUS_MINOR
CORE_PREVIOUS_STABLE
CORE_NEXT_MINOR
CORE_MAJOR_DEVELOPMENT

They could be shown in the UI form, with their defaults filled in, and added descriptions. That would make it easy for a user to override specific ones if wanted.

It comes down to a balance between setting variables in the UI vs. developer setting the versions they want in the .gitlab-ci.yml

Interesting, I recall this being a required deliverable from the GitLab initiative that we were suppose to allow as easy as possible method for users to make changes and manual runs (similar to DrupalCi). If it is agreed that is no longer a required deliverable the changes mere here make sense.

Also using _target_core in the UI form could never change the version in the other variants

The other variants really had no need to be changed via the UI, they were designed to be preconfigured setups that tested explicit configurations. The ones they did have a need to be with variants did use $_TARGET_CORE. This goes back to comment #6.

They could be shown in the UI form, with their defaults filled in, and added descriptions

I would suggest you would want new variables scoped to each job in that case to avoid overriding variables thag are suppose to be “static”. Variables that are suppose to tell us the state of the ecosystem really should not be modified by runs to change jobs.

This change (and the proposed adjustment) have moved us to a point where the contributor needs to know the internals of the testing setup for each individual project where previously this was abstracted.

Human time being valued over automation time just throwing DRUPAL_CORE at it and letting everything else break / provide invalid results will have a likely chance of being the used choice by contributors once they know it exists (though that creates issues with projects that do not allow variants to fail)map the values via the UI may or may not provide value.

Either way if it was an intentional decision with the repercussions already discussed than so be it. I didn’t see the Slack thread and wondered if this was a possible oversight vs intentional change of the deliverable.

I was just doing my best and thought this was an improvement. I was not here at the projects conception, and so don't have those guiding principles in my head, nor know where to look to read them. But removing _TARGET_CORE (from the UI form and in customised .gitlab-ci templates) was intended to reduce wasted user effort when it does not do what it seems it should. I would say if someone wants to use the UI form to test other core versions 'on-the-fly' then the best thing a new user could do was simply set one of the OPT_IN flags. Then you get a fully configured job for that version. Trying to use _target_core would likely also need _target_php to be changed, and also the webdriver and associated variables. Those are unlikely to be things a new user would know about, so the job would fail.

We want to reduce the number of variables in the form that are difficult to use, and the OPT_IN variants provide coverage for all the usual "other" core versions. At least, they should do. For more obscure version combinations the developer would likely have more knowledge and would be using a cuctom .gitlab-ci.yml. At least, that's the way I see it. But if this change does have to be reverted, I won't stand in the way, as we want to move on 📌 Update templates so 11.0 is the default/current branch RTBC today too.

I was not here at the projects conception, and so don't have those guiding principles in my head

I can say the same myself indeed. Thanks for bringing it up.

We were trying to fix the possible confusion of a variable only used in one variant, but I totally understand the points raised.

I see two simple options that might help bring back the lost functionality:
1. Remove the deprecation warning for _TARGET_CORE and alter the description to mention that it only affects the "current" variant.
2. As suggested in #27, promote the CORE_SUPPORTED, CORE_SECURITY_PREVIOUS_MINOR, CORE_PREVIOUS_STABLE, CORE_NEXT_MINOR, CORE_MAJOR_DEVELOPMENT variables to the non-hidden file and give them descriptions of where they are used.

My preferred way forwards is 2. Note that this is already documented here https://project.pages.drupalcode.org/gitlab_templates/info/variants/#use... and it's a really easy to do change. It would also allow altering the core version for any of the variants, which I think it'd be an improvement to only being able to do it in one of the variants.

We can easily do this in a follow-up.

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