Found it - see ✨ Appended timezone to publish_on and unpublished_on field Needs work which stalled. There are some good discussions there, so we need to work through those.
Hi amanire. Thanks for starting this, and in principle I like the idea. I thought this may have been discussed before now as it is useful. We do have the 'timecheck' report, see attached, but that is only available to admins, via admin/reports/status
You have pushed to the 2.x branch, but if you push to the 3566488-state-timezone-used branch you can create a MR then we will get the tests running.
The tests fail with
Error: Class "Doctrine\Common\Annotations\AnnotationRegistry" not found in Drupal\rules\Context\AttributeDiscoveryWithAnnotations->getDefinitions() (line 77 of modules/contrib/rules/src/Context/AttributeDiscoveryWithAnnotations.php).
This is because doctrine/annotations (2.0.2) which was loaded as a requirement in Core 11.2 is not present in 11.3. The required code has been moved into Core for 11.3 and the
change record →
says:
Code that relied on the Doctrine\Common\Annotations\AnnotationException exception should switch to using Drupal\Component\Annotation\Doctrine\AnnotationException instead;
However, that needs to be done in Rules not here. So until that is done we may need to add a dependency on doctrine/annotations
1. Previously node_is_page($node)=1. This can be replaced with $view_mode=full
2. The change record for node_get_type_label() is
https://www.drupal.org/node/3533301 →
It says
If on 11.3 or above use
$node->getBundleEntity()->label()instead.
If on 11.2 or below use$node->get('type')->entity->label()instead.
unning rulesaction tests on 11.2 confirms this, and we get
Error: Call to undefined method Drupal\node\Entity\Node::getBundleEntity() in Drupal\scheduler_rules_integration\Plugin\RulesAction\SetPublishingDate->doExecute()
See https://git.drupalcode.org/project/scheduler/-/jobs/7944137 and the attached.
It can be resolved using method_exists() and is_callable()
3. revisionIds() can be replaced with an entity query, and we can also use ->latestRevision() for efficiency. See
https://www.drupal.org/node/2918184 →
The failure is because the default for "promote" has been changed from TRUE to FALSE starting with Core 11.3.
Change record
https://www.drupal.org/node/3517642 →
The metadata test uses the front page node list and checks that the node is present but the metadata is not added.
For total peace of mind, I just re-checked what happens when the project has a .nmvrc file and all works as expected.
The environment variable takes precedence in 'current' and 'previous major' and we get version 20.
For 'max PHP' the environment variable is blank, as expected, and we get vervsion 22, because that is what is specified in the .nvmrc file
Pipeline via branch workflow
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
and exactly the same result via UI (with phpunit and nightwatch skipped)
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
Yes, it was useful to list out the differences in the four scenarios. The fix not only makes the 'current' composer run, but also eliminates the diffence we saw in 'previous major', so its a double win. Good to have a new default release too. Thanks.
The following testing uses the d9-basic branch. The repo has _NODE_VERSION: 'v20' set just for the 'current' composer job in .gitlab-ci.yml, because this is Drupal 10.
In a scheduled pipeline to show the current situation - all runs OK
'current' D10.6 PHP8.1 has _NODE_VERSION=v20 and loads nvm v20
'Max PHP' Drupal 10.6, PHP8.4 blank _NODE_VERSION but it also loads v20
'previous major' Drupal 9.5 PHP8.1 has _NODE_VERSION=v20 (due to the general setting in main.yml) and loads v20 and runs OK
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Here is the same repo started via UI (not using this MR)
The composer job value of _NODE_VERSION: 'v20' is ignored. All jobs have a blank value for the environment variable.
'current' D10.6 PHP8.1 fails with The engine "node" is incompatible with this module. Expected version ">=20". Got "18.20.8"
'Max PHP' loads nvm v20 and runs OK.
'previous major' now loads v18 and runs OK. The difference is because _NODE_VERSION is blank here.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Now with a d9-basic test branch using this MR via committed hardcoded repo and ref.
Pipeline via branch (not UI). All jobs pass as before.
Composer current has environment variable _NODE_VERSION=v20, and we get nvm v20
Max PHP has blank _NODE_VERSION but it also loads v20
Previous major has _NODE_VERSION=v20 and runs OK
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
Pipeline started via UI. Everything runs correctly
Composer has _NODE_VERSION=v20, and we get nvm v20
Max PHP has blank _NODE_VERSION and it loads v20
Previous major has _NODE_VERSION=v20 and loads v20
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
So I think this is RTBC.
OK. I will test this MR via a manual form pipeline in a GTD branch.
The variable can always be added manually in the UI if someone needs to do one-off testing. But in general it will not be needed in the form as it's not a thing that is often needed to be chnaged, unlike SKIP and OPT_IN etc
Not straightforward ...
Yes I agree. Doing these custom manipulations is not very nice.
the variable is shown in the web UI form and the default is empty.
I've just had another idea. If the variable was not in the form, then I think the correct override in 'previous major' would work the same as it does in MR and scheduled pipelines. If the variable was moved to the include.drupalci.hidden-variables.yml file that might solve it.
[overlapped - the above is what I had typed, simultaneously with you]
The word "incase" used to be in the en_us dictionary, but it has been removed in a recent update. Before we had:
Drupal version : 11.2.8
GITLAB_TEMPLATES_VERSION = 1.12.1
CSpell version 8.19.4
Word F Dictionary Dictionary Location
incase * cpp node_modules/@cspell/dict-cpp/dict/cpp.txt
incase * en_us* node_modules/@cspell/dict-en_us/en_US.trie.gz
https://git.drupalcode.org/issue/scheduler-3565951/-/jobs/7908130
Latest - the word is only in the cpp dictionary, which is not used in our configuration:
Drupal version : 11.3.1
GITLAB_TEMPLATES_VERSION = 1.13.0, 1.13.x-latest, 1.x-latest, default-ref
CSpell version 9.2.2
Word F Dictionary Dictionary Location
incase * cpp node_modules/@cspell/dict-cpp/dict/cpp.txt.gz
As this is not urgent, I suggest we postpone it, and work on 📌 Allow testing documentation pages via MRs Active . When that is done, we will be able to test the changes in this MR and not have a surprise like we did in #3473000-59: Documentation pages →
For ref, the follow-up issue is 📌 Adjust markdown so that CHANGELOG.md can be used in the documentation site Active
Thanks. I have just rebased this MR following the other merge.
The downstream jobs all run OK. Here are the logs for the downstream d10-plus CMS, d10-theme CMS and d11-recipe CMS
I have also made the curl changes, and we see that the file is retrieved from the 1.2.9 repo as required.
MR443 is ready for review
@codebymikey the justinrainbow/json-schema dependency is actually solved in this MR. It allows the older version of Composer to be used if needed and that automatically resolves to the earlier version of json-schema.
The chnages from https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/441... will become the 'default-ref` later today. There's been a delay due to the holidays, that's all.
Temporary code removed, as per #11 and the downstream pipelines run. @fjgarlin please could you trigger the other pipelines that I don't have access to, just to be safe.
Also, as pointed out by @fjgarlin we have this todo which can be done here. The file did not exist in 1.2.8
https://git.drupalcode.org/project/drupal_cms/-/raw/1.2.8/DrupalCmsCompa...
but it does exist now in 1.2.9
https://git.drupalcode.org/project/drupal_cms/-/raw/1.2.9/DrupalCmsCompa...
In the pipeline, in cspell version 9.2.2 the word is also in the cpp dictionary, but this dictionary is not used here either. So I have no idea how the job used to pass previously.
Two of the words (colour and sirbrillig) where in the Core 11.2.10 dictionary.txt but not in the corresponding 11.3.1 file
The word incase is not in either of the two core dictionaries, even at 11.2. Locally, in CSpell version 8.19, npx cspell trace --only-found incase shows that it is included in the node_modules/@cspell/dict-cpp/dict/cpp.txt.gz file.
I have added the advisories to cater for Drupal 11.1 and this is ready for review. There are temporary changes to assets/internal/downstream-variables.yml which I will remove if there is no further downstream testing needed.
I have improved the links and text explaining what commits are in which tag, so that we can more easily see the difference between 'main', '1.x-latest' and 'default-ref'.
Also I added the changelog file into the documentation site, it is built using a symlink to the original, so that it can be viewed properly within the site. Here's the log showing that it is being checked https://git.drupalcode.org/issue/gitlab_templates-3473000/-/jobs/7861537...
As we don't yet have documentation MR testing yet, I can't run the "pages" job here. But I worked out how to do the link using Scehduler, adding the README.md file into the /docs/ site. Here is the pages job log and here's the reuslt. So this should work just the same for the Changelog in this project.
MR406 is ready for review. It would be good to get this in first, so that everyone can use the updated links to check what commits are where, before we release the next 'default-ref'. We currently have commits in three separate stages of progression, as can be seen from the updated doc links.
jonathan1055 → changed the visibility of the branch main to hidden.
Thanks @dennisdk for opening the issue and for creating the MR. I am going to replicate this locally then test your fix.
📌 Composer 2.9 security advisories in Drupal 7 branches Active is merged.
As intended, the first pipeline fails with
Your requirements could not be resolved to an installable set of packages.
Problem 1
- Root composer.json requires drush/drush ^8.3.3 -> satisfiable by drush/drush[8.3.3, ..., 8.5.0].
- drush/drush[8.3.3, ..., 8.5.0] require symfony/process ~2.7|^3|^4.4 -> found symfony/process[v2.7.0, ..., v2.8.52, v3.0.0, ..., v3.4.47, v4.4.0, ..., v4.4.44] but these were not loaded, because they are affected by security advisories. To ignore the advisories, add ("PKSA-wws7-mr54-jsny") to the audit "ignore" config. To turn the feature off entirely, you can set "block-insecure" to false in your "audit" config.
After adding the audit: ignore: then the pipeline passes
My current best suggestion is:
- The solution for modern Drupal is as per the changes made in expand_composer_json.php, adding the three specific exceptions. This will ensure that we keep sight of any new security advisories, and if more appear we likelwise add them to the script
- For Drupal 7 we add
COMPOSER_NO_SECURITY_BLOCKING: 1into.composer-basein main-d7.yml as in the MR. This will solve the problem for all D7 pipelines, whether the project has a composer.json or not. - For our own GTD d7-composer branch, we override the above using
COMPOSER_NO_SECURITY_BLOCKING: 0but we add the single ignore value in the projects own composer.json. This will ensure that we also keep sight of any new security advisories for the Drupal 7 dependencies
All GTD pipelines are green and MR442 is ready for review.
Independently I have opened 📌 Composer 2.9 security advisories in Drupal 7 branches Active and I'd like to test that change before MR442 is merged.
For Contrib testing, I will try to use this MR as a patch in a Gitlab Templates CI pipeline. This does not replace the suggestion in #44, it's an extra. I am interested in getting this done, to unblock ✨ Allow --directory and @group to work together in run-tests.sh Active
I tried to add the advisories using the command
composer config audit.ignore.PKSA-yhcn-xrg3-68b1 "D9 twig"
but got the message
Setting audit.ignore.PKSA-yhcn-xrg3-68b1 does not exist or is not supported by this command
It seems you can read the values of audit.ignore but not set them.
Then I found the options --no-audit and --no-security-blocking which can be added to the composer require. These can be set using an environment variables so I added COMPOSER_NO_SECURITY_BLOCKING: 1 and this works great. Now we just need to decide if this is an acceptable solution for the D7 pipelines. It is needed in the d7-basic 'max php' variant and both variants in d7-composer. We can stick with the specific ignores for D9 or we could use the environment variable there but also only set it when running a Drupal 9 pipeline.
The first commit, Adding the advisory exceptions works fine in 'modern' Drupal and the D9 job now passes. But it does not fix the Drupal 7 jobs because expand_composer_json.php is not used in the main-d7.yml pipeline template. There are several ways we could fix this for D7. Here are the first three I have thought of, and there are probably more:
- Add the advisory exceptions into the transitory composer.json directly in the Composer job step
- Create a cut-down version of expand_composer_json.php specifically for D7
- Restrict the Composer version to 2.8 (which does not do the security check) just in the specific legacy jobs that need it
1 is easy if the project has no composer.json because the job creates an empty file. But if the project does have it's own, it becomes more tricky
2 is probably straight-forward but cumbersome to have two scripts. Maybe utilise the existing script but pass in a "running D7" flag, to skip most of the processing.
3 could be simple but I don't know how to do it.
All of the downstream "current" variants run OK now, except d7-composer, which is due to the problem in 📌 Security blocks on Twig versions for Drupal 9 Active
I have removed the temporary change to downstream-variables.yml and MR386 is ready for review. There are four downstream jobs that fail (d9-basic "previous major", d7-basic "max PHP" and both variants in d7-composer) but this is due to the above issue, unrelated to the changes here.
Ah, yes, of course, we have to cater for the project having no composer.json file. I was too strict in that validation, and didn't run all the downstream pipelines. That just shows the benefit in always running everything as part of the review, as we do cover lots of scenarios. I will remember to do that before setting to "needs review" ;-)
I think we should still verify that the input file exists, in case the --input argument is used, but the file is allowed to be empty.
Yes is due to new feature in the lastest composer version. Composer 2.9 automatically blocks dependencies with security advisories.
The Drupal 9 job which ran OK on 17 December used Composer version 2.8.10 2025-07-10 19:08:33. The job which failed on 24 December used Composer version 2.9.2 2025-11-19 21:57:25. The twig version v2.15.6 is the same in both.
We could take the view that we no longer need to provide test coverage at Drupal 9, but that is a bigger decision, not simply for this issue. However, the same problem happens in both of the variants of the d7-composer branch, and we do not want to drop Drupal 7 test coverage at this time. A simple solution would be to add the specific config: audit: ignore: exceptions to the generated composer.json. This can be done in the default settings in expand_composer.php and solve both the D7 and D9 problems.
As this has to be fixed in the upstream Gitlab Templates, I have moved this issue there.
Hi @er.garg.karan
As I said in #5 and #8 I need to replicate this problem starting with a clean Drupal install. Otherwise we have no idea what bug we are fixing. Just stating you settings is not enough to demonstrate the problem. It could be a conflict with another module, or caused by some other combination of things on your site.
You have re-opened the issue, which is fine, but also you closed the MR222 - did you mean to close it?
A similar problem happens in Drupal 7 on the d7-basic branch composer 'Max PHP' variant
Your requirements could not be resolved to an installable set of packages.
Problem 1
- Root composer.json requires drush/drush ^8.3.3 -> satisfiable by drush/drush[8.3.3, ..., 8.5.0].
- drush/drush[8.3.3, ..., 8.5.0] require symfony/process ~2.7|^3|^4.4 -> found symfony/process[v2.7.0, ..., v2.8.52, v3.0.0, ..., v3.4.47, v4.4.0, ..., v4.4.44] but these were not loaded, because they are affected by security advisories.
To ignore the advisories, add ("PKSA-wws7-mr54-jsny") to the audit "ignore" config. To turn the feature off entirely, you can set "block-insecure" to false in your "audit" config.
You can also try re-running composer require with an explicit version constraint, e.g. "composer require phpunit/phpunit:*" to figure out if any version is installable, or "composer require phpunit/phpunit:^2.1" if you know which you need.
Installation failed, reverting ./composer.json to its original content.
See https://git.drupalcode.org/issue/gitlab_templates_downstream-3562055/-/j...
No worries, thanks for merging that one. Now on this MR we can see that check temporary changes fails as expected.
After removing those temporary changes the job ends green. The pipeline fails due to Check Versions, but MR386 is ready for review, or RTBC given your comment in #22
Good question. Looking back the scheduled weekly d10-plus pipelines it has been failing since 3 Dec. The last green run was 29 Nov. The new failure is caused by a deprecation in search_api_requirements which we are not ignoring. The version of search_api has not changed. It is due to the new deprecation added to 11.3.x on 3rd Dec - see https://www.drupal.org/node/3549685 →
It is simple to fix as we already have the mechanism to do that, using the projects .deprecation-ignore.txt file - I have checked this in a test pipeline and the job passed green so have now committed it to the d10-plus branch.
The pipelines are green except d9-basic - previous major due to a newly add problem with security blocks in Drupal 9.5
Problem 1
- Root composer.json requires drupal/core-recommended ^9.5 -> satisfiable by drupal/core-recommended[9.5.x-dev].
- drupal/core-recommended 9.5.x-dev requires twig/twig ~v2.15.4 -> found twig/twig[v2.15.4, v2.15.5, v2.15.6] but these were not loaded, because they are affected by security advisories. To ignore the advisories, add ("PKSA-yhcn-xrg3-68b1", "PKSA-2wrf-1xmk-1pky", "PKSA-6319-ffpf-gx66") to the audit "ignore" config. To turn the feature off entirely, you can set "block-insecure" to false in your "audit" config.
However, this is unrelated to the changes here.
I have also added a specific example to the doc page, so MR441 is ready for review.
Added search_api_requirements without a #[LegacyRequirementsHook] attribute is deprecated in this commit on d10-plus
I have pushed the change here to set _NODE_VERSION: v20 in Composer (previous major) and the downstream d10-plus branch works right out of the box now - https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/77...
I also pushed a permanent change to the d9-basic branch and that now works correctly.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/77...
It may be a good idea to update the new doc page with that specific example of a customised job. But all the techical work is done here, I think.
OK, I have removed those now. I was just trying it out in case you were not around to do any more merges for a while.
Thank you. This unblocks ✨ Prioritise project installer paths as first takes precedence Active
This issue is postponed on 📌 Add supporting internal file to test MRs. Active
I have also opened GTD issue 📌 Custom installer-paths Active to allow testing with a custom installer-paths. Ultimately one of the downstream branches may get a permanent commit with this, to provide ongoing test coverage.
The versions have moved on, we now have 11.3.1 and 10.6.1
https://git.drupalcode.org/issue/gitlab_templates-3563698/-/jobs/7730134
I have pushed an update and it's green again
https://git.drupalcode.org/issue/gitlab_templates-3563698/-/jobs/7730297
Thanks. Moving the echo worked as required. Here is the same downstream d10-profile as before, and we do not get any empty collapsed section. Node version v20 is installed later, as expected.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/77...
Details of the initial testing are on this slack thread, but in summary -
1. In the d9-basic test branch with a .nvmrc file containing v16 the Drupal 9 composer (previous major) runs perfectly. For Drupal 10.5 (current) it fails because of Yarn incompatibility
Drupal@: The engine "node" is incompatible with this module. Expected version ">= 18.0". Got "16.20.2"
This is what we'd expect.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
2. Using v18 in the .nvmrc file it works for both D9.5 and D10.5
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
3. With an override variable _NODE_VERSION: 'v16' for just the D9 variant it all worked as expected. We get v18 for current and v16 for 'previous major'
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
4. On a different branch, with no .nvmrc file, but entering _NODE_VERSION = v18 via the UI, get get version 18 loaded as expcetd
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/77...
I think this covers all of the scenarios we need to test, so I'm marking it RTBC, but @flgarlin will have the final say.
Thank you @ross-zch for the patch, and thank you @dennisdk @lily.yan @kpaxman and @dmezquia for testing and confirming the solution.
Merged and fixed.
The patch from #9 was a re-roll of the patch from the inital issue summary. I used it to create MR263
There are some things still to do:
- Check what permission should be needed to allow this. Is it different from the existing ones?
- We need test coverage for this new feature
- Need to check that it works correctly when SCMI is also installed
Ideally it would be nice to give the user a warning/ask for confimation before proceeding, but we don't currently have anything like that in this module. Maybe an extra checkbox to tick, with explanatory text so that we can make sure the user knows they are altering the workflow that had been set.
Thank you @mglaman for opening the issue and creating the MR and thank you @hydra for testing and confirming. This is merged and fixed, and I've given you both credit too.
jonathan1055 → made their first commit to this issue’s fork.
Here's an example of the point I made in the MR regarding the "NVM Install" collapsed section being empty if neither the file nor the variable exist. It looks confusing, as it implies that something went wrong because there is nothing to read.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/77...
So I suggest the echos for 'start' and 'end' of section are moved to inside the conditional block.
NW for that, and for the suggestions in the MR about the docs wording.
As per #16-18 this issue is postponed on ✨ [PP-4] Change run-tests.sh to use Symfony Console classes for command line parsing Postponed
But the error in run-tests.sh test discovery that is being solved here is now causing actual test failures in contrib, now that some 3rd-party modules have dropped using @group mymodule in favor of #[Group('mymodule')].
Do we have a plan for which run-tests.sh issues are being worked on, and in what order?
The first problem found is that the yarn install fails in 'previous major' variant, which is now running core 10.6.0 when it had been 10.5.8
error qified@0.5.2: The engine "node" is incompatible with this module. Expected version ">=20". Got "18.20.8"
error Found incompatible module.https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/76...
MR441 is ready for review. This change will affect 'Current', 'previous minor', 'previous major' and 'next minor' variants, so maybe will need more than the cursory checking we do when just bumping a patch-fix release.
https://git.drupalcode.org/issue/gitlab_templates-3563698/-/jobs/7697782
Removing the changes from downstream-variables.yml and we get EXIT_CODE=2
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/jobs/7695359
Removing the GTD issue forrk, we get EXIT_CODE=1
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/jobs/7695861
Removing all but one debug line, to check it works when only one file is affected. EXIT_CODE=1 as expected
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/jobs/7696006
With that final line removed, the job ends green with no EXIT_CODE
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/jobs/7696043
The pipeline failed due to unrelated 'Check Versions'
[ERROR] Drupal 11.3.0 is out!
[ERROR] The value of NEXT_MINOR_STABLE is currently 11.3.0 - this needs to be updated.
I have also pushed a first draft at documentation - which can be previewed here so marking NR for this.
Yes I agree with you, I wouldn't want to fuss with remember no space, or using a pipe. I was really only checking those for background info, as I might post on those issues. I am guessing that YAML syntax is so flexible and part of it is making less of a distinction between what is in a string and what is not. So theoretically it might not be a bug as per how yaml is designed to work, but it is certainly a very unhelpful and unexpected consequence.
I have pushed the change to use @temp, @temporary and @debug, and also to check files in the assets/ folder. Not the internal ones, but those used in the actual jobs. So MR439 is ready for review of the code and the text of the messages. Here's the log
Then I'll remove the temporay code in stages, so we see that it works correctly when only 2 then only 1 of the checks fail.
I got it working, even though the automated MR completely re-formated the .gitlab-ci.yml to make the pipeline fail. I undid most of those changes and now we have a pipeline running with a new job "secret_detection".
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Those changes can be brought back upstream now.
The new job detects one leak, but does not end amber or red, so there is no indication that anything is wrong. We'd need that job to end amber at least. But now we have something working we can start to refine it.
I have added the example on that page to the main test branch on issue/gitlab_templates_downstream-3562055 and changed the downstream job definition to use that branch. But no extra job was run, the pipeline looks normal (except for cspell not recognising glpat)
In that documentation page when creating the test project, step 3.4 says 'Select the Enable Secret Detection checkbox'. I'm not sure where to find this setting on an existing project. It could be in gitlab_templates_downstream/-/security/configuration. I clicked the ' ' to see what changes would be made, and it automatically created MR57. Now we can analyse the differences.
There is no colouring when the same string is inside a pipe
- |
echo "DEBUG: this is fine"
and the pipeline runs OK
Also this thread mentions that if you follow the colon with a word not a space, it works. So I have just pushed that change and the pipeline is OK.
Yes @debug is also a good option.
@dmezquia and @mrweiner thank you for the feedback. Yes this does seem an odd problem, as this must be quite a normal thing to do, when extra parameters are required. Could it possibly be the interaction from another module? It is perfectly technically possible for any module to define or redefine the arguments in services.yml for another project. Given that only 10 people are following this issue (which inlcudes me an gitlab) so only 8 users, if there was a serious error in the codebase it would affect far more people. It must be some combination which does not occur that often.
I'm happy to commit MR262 and make a new release if that helps some people, and we can keep this issue open to continue the investigation. Any thoughts?
@prudloff We are ready to try this POC. Please could you give me, or link to, some examples of the secrets that should not be committed in repos? I know I could do my own research, but I'm starting from zero and I'm not sure what I am looking for. But you already have experience of this and opened the issue, so it would be much quicker for you. Thanks.
Ah, that is not good. If you have a word followed by a colon inside an echo string it is still interpreted at a real yml keyword and the pipeline fails to get created.
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/pipelines/68...
It seem's like a bug in gitlab pipeline creation.
When the colon is removed from the echo string the pipeline runs
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/pipelines/68...
You are right, yes "debug: this is the info" does feel more natural to write and read. The reason I didn't do that is I had trained myself out of using colons in real code because any word in a quoted string followed by a colon is interpreted as a yaml keyword (incorrectly) and coloured differently using the editors colouring rules (see attached from my local editor, and both the gitlab online editors). So I've been avoiding using that in permanent code as it looks odd. But maybe for temporary and debug lines it is perfect to have a visual difference too. Good call! I will make that change.
I've just rebased so we can make use of assets/internal/downstream-variables.yml and try some downstream pipelines.
Where does the template: Jobs/Secret-Detection.gitlab-ci.yml come from? Is there a separate job that should be added to the pipeline or is the secert detection added to an existing job?
jonathan1055 → made their first commit to this issue’s fork.
MR439 is ready for review and feedback. In addition to the check explained in #19 above, the final check detects the use of the special strings *debug*, *temp* and *temporary* in active code or in comments. I chose to have a '*' each side of the string so that usage of this would be distinct and easily findable. If we remember to use those specific strings in temporary lines then we wont have the problem of missed debug, like we did recently.
This MR contains examples of all three checks
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/jobs/7670922
When you have done an initial review and I've addressed any feeback I can remove the tempoary changes in stages, to show the varying log output, and then finally when none remain the job will be green.
Thank you. I am working on expanding the checks for temporary code and debug lines in a second merge request MR439 on 📌 Add supporting internal file to test MRs. Active and that should be quite quick. So I will leave this one as NW for now, and I'd like to ask that MR439 is merged first, then we can use it here to demonstrate what is can achieve.
As requested, to test a custom installer-paths value I have used the dowmstream issue fork 3562055-d10-plus with this addition. Here is the trimmed downstream pipeline. The composer job runs fine, and the log shows that web/modules/custom does not exist (as required) and that the 3rd-party module has been added to the custom folder web/my/path-to-modules
However, the PHPunit job fails with 'Unable to install modules: module 'gitlab_templates_downstream' is missing its dependency module search_api'. It appears that phpunit does not know where to find the other modules.
Similarly Upgrade Status fails with 'Unable to install modules upgrade_status due to missing modules upgrade_status.' We can see that upgrade_status module has been installed in the correct place in the custom directory
The same phpunit problem can be seen in 3562055-d11-recipe where there are more 3rd-party modules needed. Here is the full pipeline
The 3562055-d10-theme Upgrade Status job fails in the same way. Here is the d10-theme pipeline
However, these problems in PHPunit and Upgrade Status are NOT due to the changes in MR386, they all happen exactly the same when running those test branches against ref:main
3562055 -d10-plus https://git.drupalcode.org/issue/gitlab_templates_downstream-3562055/-/p...
3562055-d10-theme https://git.drupalcode.org/issue/gitlab_templates_downstream-3562055/-/p...
3562055-d11-recipe https://git.drupalcode.org/issue/gitlab_templates_downstream-3562055/-/p...
If we want to investigate how to make PHPUnit and Upgtrade Status run with a customised installer-paths directory for 3rd-party modules, then that would be better done in a separate issue, as it is not related to the changes here.
The only other change since our comments in #18-#20, apart from using downstream issue branches and the downstream-variables.yml file is this commit to use PROJECT_NAME ahead of CI_PROJECT_NAME. I have explained and tested this in a comment thread on the MR.
So MR386 is ready for review.
I have opened MR439 to extend the checking of temporary changes so that we have a better way to see them, and prevent unintended commits. The job now checks for any ocurrence of project: issue/gitlab_templates_downstream which is used to temporarily test a specific downstream issue fork branch instead of the canonical project. The lines will be shown in the log. I will push a temporary chnage to show this.
Unfortunately this does not work we well as I had expected. I had previously thought that "The customized variables are copied at the point when the issue fork is created". This may be true, and the custom variables are certainly used when the pipeline is from a MR on the issue fork, as shown in the 3rd paragraph of comment #22 on #3557880-22: Testing against main ref → . But when the pipeline is just triggered by a push to the issue fork when there is no MR, then it runs with the default values, and the GT version is 'default-ref' not 'main' - as we can see in this branch pipeline. The composer job uses default-ref and therefore the cspell job fails.
According to this gitlab doc
By default, pipelines from forked projects can’t access the CI/CD variables available to the parent project. If you run a merge request pipeline in the parent project for a merge request from a fork, all variables become available to the pipeline.
It says 'by default' so is there a way to override that default?
If not, then all it means is I need to enter ref: main and _CURL_TEMPLATES_REF: main in each of these test branches.
Thanks for putting this together so quickly, it looks great!!
Well, it was your idea in the first place, I just picked up an ran with it, as I know it will help us both, and anyone else who works on MRs here.
Good to see that the jobs was run in the Merge Train and the Push. This means that if anyone did just "merge immediately" or push to main from local, with variables still in the file, then the pipeline failure should alert that it needs to be fixed. But also, even if variables were left in that file it would only affect downstream jobs when run from MRs here, there is no wider impact.
That's great. I'm pleased we do not need that DUMMY_VAR
I have pushed the change to comment out those variables.
Thanks for the feedback. I have added the comment, and made the rules: if: condition more strict, so we don't run the job in scheduled pipelines.
I discovered that we do not need the dummy variable, the file syntax is accepted with all variables commented out. I've updated the comment in the file.
There are two variables still uncommented in the file, so when you have checked everything else, if you try to commit via Merge Train we should see that the job halts the train.
Instead of using tail to skip the allowed line, I have expanded the grep pattern to allow the required lines.
When the file has a variable the job fails as requirred
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/jobs/7627511
When the file only has comments, the job passes
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/jobs/7627609
The checking job first draft - I set it to manual if not on commit, so that it can be tested. This might be how we do it anyway.
The job fails as required when the file has anything other than comments
But the file fails the yml syntax checker when it only comments. As this is an internal file, under the control of this project, we can simply skip the start of the file, and check from a known point onwards.
Good start. The variables in the included file have been used, and we get just one variant and only two jobs in the d10-plus downstream pipeline.
https://git.drupalcode.org/issue/gitlab_templates-3562253/-/pipelines/68...
Next, to check which takes precedence - the file or the hard-coded variables in .gitlab-ci.yml .downstream-base. I think it will be the specific variables defined that override those in the file. We won't be doing it this way much, but need to know in case.
Thanks for that steer. I'm going to work on this now, as it will assist all other MRs here. I have added some tasks to the issue summary.
There was a delay, but 11.2.10 is also out now. MR437
No 'bad', it was not a criticism. I was just noting that the default of using the issue title is sometimes less specific than the MR title. I don't know how those defaults are set, though.
Ah, because I re-used this issue, the commit message default is the same as before. The MR title is often better to use than the issue title. I wonder if that is a setting or option that could be changed?
Re-using this issue for the next update, D10 is now at 10.5.8 - MR436
jonathan1055 → made their first commit to this issue’s fork.
I was just deleting my local branch when I noticed/realised that some of the debug lines were left in. I have opened MR434 to remove them. This has reminded me that I should always open a MR comment thread on the debug lines with a reminder to remove them :-) Sorry to make extra work for you.
Thanks. Yes it was an old issue, and 2 of the 5 problems had since been fixed elsewhere. But it's good to get this finished. In working on this issue I did have a few other ideas about cspell, but decided to stick to the things being fixed here, not creep the scope any more. I will open a follow-up issue later, but want to work on some other long-standing open issues first :-)