MR50 adds a custom composer bin-dir to d10-theme to give us wider test coverage. Also to assist in testing 📌 Testing against main ref Active
First pipeline fails as expected because the overridee CI/CD variables have been deleted now
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Adding back the CI variable, and the jobs pass, which demonstrates that the d10-theme branch currently needs ref: main
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
You should be able to see this:
Yes, that is what I linked to in #4 and talked about in #8 and #10. The settings are not copied over to issue forks.
I have pushed to MR49 to experiment with using rules: in the include. The first run is promising. In composer before_script we see:
_GITLAB_TEMPLATES_REPO=project/gitlab_templates, _REF=default-ref
_CURL_TEMPLATES_REPO=, _REF=main
Then in the body we can see the files curl'd from 'main' which is what we want.
In after_script we get:
_GITLAB_TEMPLATES_REPO=project/gitlab_templates, _REF=default-ref
_CURL_TEMPLATES_REPO=project/gitlab_templates, _REF=main
The log output first looked like we had a problem because _GITLAB_TEMPLATES_REF was default-ref, but then I realised that this is simply because we are not setting that variable. It works because we have hard-coded 'main' in the include. I can add that variable into the new .main-ref-variables.yml file, just so that we don't get confused.
So this approach appears to work in an MR. I now need to test it in pipelines triggered by
- Form UI
- On issue fork branch
- from an upstream MR pipeline
I think we should definitely drop the csslintrc file.
I also think we should do the same for the ESLint config files.
Just wondering if core/.eslintrc.passing.json and core/.eslintrc.jquery.json the files you are refering to? These are used in Contrib testing via Gitlab Templates
Even if we were given Administrator role on that one specific issue fork, to allow the variables to be altered, I think we'd need it for every new issue fork. Currently, as maintainers when you and I run MRs they run in the canonical project space, but a non-maintainer would run their MR in the issue fork, which could be a new one they have just created. The override would nee to be the new default for every new issue fork. I don't suppose there is any overall top-level default for CI/CD variables that can be done at the project level so all new issue forks get the overridden values?
To prove the change to rules: is still correct, here's a pipeline for d10-profile with the job change but not the variable set
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
The other two MRs are also ready, same change in all three.
MR46 for d10-plus is ready for review. The job changed to be manual so that regular downstream pipelines and scheduled pipelines end green, because Upgrade Status ends amber so the pipeline ends amber. That would be a pain for normal downstream testing where we want to see all green.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
I think the changes are probably OK, I did test at the time, but things have moved on. I came here to check, run a re-test and to post some test results. That led to opening ✨ Run Upgrade Status in more test branches Active because we currently only test downstream Upgrade Status on d9-basic. When that is merged I will re-test here.
Regarding the solution in #8 and #10 above, there is a bit of a problem in that the settings are not available (at least not to me) in the issue fork branches, so any pipelines I run here still use 'default-ref'. See attached. When running manually via UI I have to remember to change the ref to 'main' which I can do. But I also have the workflow set on this particular issue 3503337 branch to be able to push from local and the pipeline runs without needing an MR. These now fail due to running 'default-ref'
Do you know who (if anyone) can access the CI/CD settings for an issue fork branch? Or maybe the settings do not exist? Can you see if you have settings on this page ?
If there is no way to set these for anything other than the canonical project then we may need to go back and try your idea in #7 using rules in the include: statement.
Just noted that we still have an open MR421 here, so making this comment to stop the issue going "closed(fixed)"
I wouldn't say its a bug, it's more of feature request to move where the phpunit tests are executed.
There will be a bit of compication due to have to cater for _PHPUNIT_CONCURRENT:1 running run-tests.sh and also _PHPUNIT_CONCURRENT:0 running phpunit binary, but I expect we can work out how to solve that.
I have created a MR to add a create() method and implement ContainerInjectionInterface. All the tests run OK, but I don't know if this will have any impact on fixing the error reported above.
If anyone is able to use this, via a patch from merge_requests/262.diff then please try it.
That's great that your enhancement was merged. Would you like me to see if we can apply the patch in a test branch? Does it need a one-line change to add the argument when calling drush --root=$_WEB_ROOT -y theme:enable $PROJECT_NAME? If so, that changes who and where we might decide to test this. Could we temporarily force Drush 14.x instead?
I made a few suggestions in the MR, just minor wording.
The downstream jobs all ran as expected:
- d11-recipe has no config file
- d10-plus has the original phpstan.neon file
- d10-theme uses a phpstan.neon.dist file
✨ Add PHPStan config filename variations Needs review is merged, so MR430 is now ready for final review. I think it all looks good.
Thank you. Merged. We can run a downstream d10-theme on 🐛 PHPStan GitLab template does not read phpstan.neon.dist in contrib projects Active and that can be merged too.
The SchedulerManager has a __construct() function but does not have a create() function. It's been working OK like this for years, so obviously this is not new, but would adding the create() force the __construct() to get the nwely updated list of 12 arguments, not the stale list of 9?
Does anyone have a solution for this? I'm happy to make a new emergency release if we had a fix. Increasing the number of arguments must be a common thing, so there should be a good solution. I note that Scheduler 2.2.2 apparently has 18,000+ installs → and they can't all be broken, so some devs/maintainers must know a way to fix this.
PHPStan is catered for in ✨ Add PHPStan config filename variations Needs review so this issue is now just for ESLint.
Using BEFORE_SCRIPT_ACTIONS and sed to make the switch, the job fails as required.
The artifacts/junit.xml and artifacts/phpstan-quality-report.json both show that the path has been editted as required to make it relative to the projects own root.
This is ready for review, [done anyway]
Changing back to reportUnmatchedIgnoredErrors: false and the job passes as expected.
I'm thinking we could use this switch to implement BEFORE_SCRIPT_ACTIONS = "phpstan-fail". The "ignore message not found" is not written to the baseline artifact file (which is not ideal) but we do still get the junit.xml and phpstan-quality-report.json which will provide proof of the processing of this sed (which is one of the points about forcing a failure).
Yes the regex would accept phpstan.dist.neon.dist but I thought that if anyone actually created that file then they'd soon find out that it did not work.
Looking back at the if/elseif etc, it's actually OK as is. I was thinking that the regex would mean fewer lines in the job, but maybe it is better to be explicit, so yes forget the regex idea. But the message suggestions in the MR are good and should be used.
I'm going to use this issue to document what was changed to the default CI/CD variables to test against 'main'. I also have quite a few other docs pages changes. The current set-up means that the real documentation site is updated immediately when I run MR pipelines, because we've not finished 📌 Allow testing documentation pages via MRs Active yet.
This is a good enhancement, thanks for opening the MR. We did a similar improvement for phpcs, where any of phpcs.xml phpcs.xml.dist .phpcs.xml or .phpcs.xml.dist can be used. See https://git.drupalcode.org/project/gitlab_templates/-/blob/main/includes...
Can I suggest we make the check by detecting the actual file, then displaying it in the message, for example
# If there is no PHPStan configuration file get the default from /assets/phpstan.neon
- |
PHPSTAN_CONFIG_FILE=($(ls phpstan{.dist,}.neon{.dist,} 2>/dev/null || true))
if [ -z $PHPSTAN_CONFIG_FILE ]; then
echo 'This project has no phpstan{.dist}.neon{.dist}, getting default from Gitlab Templates assets/phpstan.neon'
source $CI_PROJECT_DIR/get-file-via-curl.sh assets/phpstan.neon
# Change the baseline filename placeholder to the runtime value in $_PHPSTAN_BASELINE_FILENAME
sed -i "s#BASELINE_PLACEHOLDER#$_PHPSTAN_BASELINE_FILENAME#g" phpstan.neon
else
echo "Using the project's existing $PHPSTAN_CONFIG_FILE config file"
fi
This way we report the actual file being used, and if there happen to be two then that will also be highlighted in the message, so its clear.
You can test this MR in resource_conflict → either by submitting a pipeline manually via the UI and changing the variables
_GITLAB_TEMPLATES_REPO = issue/gitlab_templates-3557975
_GITLAB_TEMPLATES_REF = 3557975-phpstan-neon-dist
or if you have an open MR then set the following
include:
- project: issue/gitlab_templates-3557975
ref: 3557975-phpstan-neon-dist
We need downstream test coverage for this change. Currently only one branch (d10-plus) has a phpstan.neon file. You can add the following in .gitlab-ci.yml here to temporarily make downstream testing more efficient.
# Temporarily skip the jobs we are not interested in.
SKIP_PAGES: 1
SKIP_COMPOSER_LINT: 1
SKIP_CSPELL: 1
SKIP_ESLINT: 1
SKIP_PHPCS: 1
SKIP_STYLELINT: 1
SKIP_NIGHTWATCH: 1
SKIP_PHPUNIT: 1
SKIP_TEST_ONLY_CHANGES: 1
RUN_JOB_UPGRADE_STATUS: 0
I added drupal/token as the required module, and this is shown in the log.
Also aligned cspell and workflow with other branches.
It works for a scheduled pipeline - I triggered this one early and it has run using 'main'
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Also just rerun from this upstream MR
https://git.drupalcode.org/issue/gitlab_templates-3546508/-/pipelines/66...
All the Composer jobs use the correct upsteram ref 3546508-project-name (branch), see this d9-basic job
So with this approach there will not be anything to commit upstream or downstream. Isn't it great when what started out as a tricky problem end up with such a simple and elegant solution. I will document this in the GTD docs page.
Ah, sorry I had my response typed, then had to do something else, then clicked submit without seeing your #5-#7. That is also a good idea, but it may be simpler, if #8 also works when running via an upstream pipeline.
After adding _GITLAB_TEMPLATES_REF = main in the CI/CD variables the pipeline ran correctly using 'main' and the phpcs job also passed.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
It may be the case that we do not need the new _GTD_TEMPLATES_REF variable if the CI/CD default can be overriden like this. I will now try running this from an upstream Gitlab Templates MR to make sure it can be overriden as per usual.
This is interesting. After checking that it was not the placement of the variables: before the include: (which stil failed) I added an include of a local .gtd.variables files, to see if that would override the predefined defaults - the pipeline ran but we still got 'default-ref'
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Although we can see in the 'show variables' that the new variable from the local file does exist.
I then created a project pipeline variable externally via settings/ci_cd#js-cicd-variables-settings and this did make a difference. The 'main' ref seems to have been used because even though the composer job failed (due to curl getting the symlink file from default-ref) we did get the new "Symlink failed" big error message, which is only in 'main' so far.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/72...
scripts/get-gitlab-templates-version.php> reported that we were running 'default-ref'. Is that because the file was curl'd from 'default-ref' even though the running ref was 'main'?
The initial idea is not quite right. It fails when we create a new variable in .gitlab-ci.yml and use that in an include:. We get Project `project/gitlab_templates` reference `` does not exist! - and the pipeline is not created
The existing variables _GITLAB_TEMPLATES_REPO and _GITLAB_TEMPLATES_REF are defined in the variables file, is that the difference? But the variables file is one of the included files, so they must be pre-defined or "allowed" or "known about" at some earliet stage.
I have created 📌 Testing against main ref Active for the new approach.
I have merged MR42 even though it means the branch will fail until
🐛
Symlink project script needs to be PHP5.6 compatible
Needs work
reaches default-ref or until we implement your idea in #9. I think it will be better to start a fresh issue for that.
I'm going to remove the 'main' ref, as having that would prevent the branch using an upstream Gitlab Templates MR repo and ref. We will find another solution to that problem.
Thanks. I was pleased it turned out to have a simple solution. I've removed the tempoary lines.
d7-basic needing the new processing
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/72...
d7-composer with phpcs standards already installled
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/72...
Ready for review.
Initial commit to demonstrate the problem - d7-basic is set to run on PHP5.6 and fails phpcs
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/72...
The message for CORE_SECURITY is not as helpful as it could be. We get
Logic for CORE_SECURITY.
- [ERROR] CORE_STABLE (11.2.7) and CORE_SECURITY (11.1.5) have different major:minor versions
We have actually determined the last security $latest_security = get_latest($security_releases); which is 11.2.8 so I think that should be shown in the message.
As per this Slack thread, we will use the main ref for all GTD branches. I also had to add _CURL_TEMPLATES_REF: main
This is ready for review, even though the phpcs job fails. That will be resolved in 📌 Cater for PHPCS when phpcodesniffer-composer-installer is not loaded Active but MR42 can still be committed first.
Just for info, the feature/enhancement in ✨ Add recipes path handling Active is in Gitlab Templates release 1.12.0 which became the default version for all Contrib from 6 November.
I tried to find this new page in the documentation site, I browsed looking for documentation but could see nothing. Eventually I resorted to looking at the commit above, to see that it is in docs/php/coding.md so then I went back to the Doc site, opened the PHP section and still have to scroll and search until I found it.
The paragraph is called 'File Names" but it only talks about documentaion pages, so its not really related to PHP at all. Maybe there should be a new top-level "Documentation" menu item for this kind of standard? The MR was only opened after most of the discussion above, and I'm sorry I did not get back to this issue and raise this question quickly. It's not a big problem. Do we have a general issue open for restructuring the menu, etc, or maybe there are other migration tasks still to be done first?
Excellent. They key one was D7 API Composer which passed.
I recall that we used to have to install those standards manually in earlier versions, but in later versions it was done automatically, via the new phpcodesniffer-composer-installer.
I have just tried adding
$CI_PROJECT_DIR/vendor/bin/phpcs --config-set installed_paths vendor/drupal/coder/coder_sniffer in the composer after_script and that has solved it.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Maybe this should go into the composer script on main-d7, conditional on the php version. Or actually it can be done regardless, as it does not mess up if run twice.
Yes, all suggestions done, thank you.
To check the formatting of the updated message, instead of forcing a failure again in this MR, I copied some of the lines to the downstream MR which uses this. The message looks fine
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/72...
RTBC but can you run the downstream jobs a final time.
The existing composer job, using PHP8.1 has
Package operations: 57 installs, 0 updates, 0 removals
...
dealerdirect/phpcodesniffer-composer-installer (v1.2.0)
...
PHP CodeSniffer Config installed_paths set to ../../drupal/coder/coder_sniffer,../../sirbrillig/phpcs-variable-analysis,../../slevomat/coding-standard
But when using PHP5.6 there are fewer packages
Package operations: 46 installs, 0 updates, 0 removals
In particular the phpcodesniffer-composer-installer is not installed, and we do not see the installed_paths output.
Nice addition with the exit code checking, thank you for adding that.
The tests show it works and solves the PHP5.6 problem.
This existing comment could be copied into the header of scripts/get-gitlab-templates-version.php as these are the only two php scripts used in the Drupal 7 pipeline.
I have left a couple of suggestions in the MR, and there is one temporary change to remove, so NW for these things.
Separately from the exit code not being detected (which is anoying) I am also confused by the set of composer jobs for d9
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/72...
Some failed then the re-run of the same job passed. Do you know why that was?
Excellent, I will review.
In your tests in #11 the d7-composer branch is using php8.1, so its good to see that still run. It's the d7-basic pipeline which is now using php5.6. We also need to test it when a positional parameter is passed (to trigger the deprecation).
The downstream d7-basic composer job passes now. I added SCRIPT_VERBOSE:1 to the downstream base job, so we can see the debug output.
I have also thought previously that it would be very helpful if we could detect a return code from symlink_project.php and check this at the end of the job. Now that the error is in a collapsed section it's less obvious. The uncollapsed part could write an error, with $DIVIDER. It would need something like php symlink_project.php $VERBOSE_FLAG $COPY_OPTION || EXIT_CODE=$?
Just tried adding variables: PHP_VERSION: 5.6 to d7-basic and it fails as expected. So we could probably commit that chnage to the downstream branch too. Then anyone who is working on Gitlab Templates MRs and who has maintainer authority on GTD will be able to run those jobs. It shouldn't all fall to you (as maintainer of Api D7) to have to remember to run the jobs :-)
I'm not actively working on a solution to this problem yet, as there are older issues I need to finish. But at least now this MR has coverage for anyone else who wants to pick it up sooner.
Ah, and looking back at MR413 we did not run the API D7 branch in any of the tests. Maybe one of the final checks should be that every downstream branch passes? or is at least run (because there are some known failures).
Both of those ideas are worth considering.
Also we should see if we can change the d7-basic downstream branch to run on PHP5.6 if that is our real minimum, then we'd find these things during testing. If we can't modify that whole branch then maybe add another standalone job in this project's .gitlab-ci.yml, to verify all .php scripts against PHP5.6
I think I noticed the symlink problem and was going to question it, but other things (real life) distracted me and then I forgot to look again.
There is also a typo in the test-only.sh script which caused me some lost time and several re-runs before getting round it. It's already fixed in MR396 on
✨
Allow users to specify custom files that Test-only changes
Active
but that's not merged yet, so I think that will be my next focus.
I also realised that we've not checked test-only changes so I have done that in the d10-plus test branch used earlier. We can see the value of the new variable, that the custom directory is listed and the phpunit is run from there OK.
I have pushed the changes for D7 and for the two places where vendor/bin was mentioned in the other docs pages.
Both D7 downstream branches run green with this change, using the default derived value for COMPOSER_BIN_DIR
https://git.drupalcode.org/issue/gitlab_templates-3508817/-/pipelines/65...
I have also tested on a branch with a customised bin-dir value. Here's the full pipeline, and we see that the phpcs job and phpunit job both run using the custom dir.
Ready for review.
The only remaining questions I have are
- There are a few references to 'vendor/bin' in the documentation files. is it OK to leave these as-is or should we check, and change it to
$COMPOSER_BIN_DIRwhere approriate? - Should we make the main-d7.yml file consistent with this change? Even if the custom
bin-dirproperty can't be used in Drupal 7 we could still create the new variable with a fixed value, and change the few executable calls. We generally have been trying to keep the D7 file in line with changes in D10+ for ease of comparison and future changes.
I've tested the Upgrade Status job on d10-plus and it works fine, just as expected.
I have pushed a draft additional paragraph to the Composer documentation page. I'm sure it can be tweaked, but it's a start.
That's good. I also changed the downstream job to specify a deeper directory "bin-dir": "custom-bin-folder/is/here" and this works fine, as expected.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/71...
Just need to test against against Upgrade Status, as that can't be run on Recipe projects. I will do that manually in a difference downstream test branch.
That's great that the downstream job works. I've left some feedback in the MR, about keeping the useful verbose output you added.
Thanks for the change to prepare-cspell.php, but its still not right when an actual custom value is used. Here is the downstream job with SCRIPT_VERBOSE:1
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/71...
I think the problem is that the paths need to be relative to where cspell is being run from. We have just 'vendor' and 'web', but now you can see we get the absolute path for the composer bin direvctory, which cspell does not match against. Maybe take the last item in the string, split on / , or something like that?
As discussed on Slack #gitlab-templates-development the rsync log produces 2 lines for every single file in a project, which can make the CMS log harder to browse quickly. Here is the log showing the additional wrapped section.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/71...
Add --verbose to prepare-cspell.php in the Check Code job in .gitlab-ci.yml
It's because prepare-cspell.php is also used for the internal checking of this project. But there will be no value for getenv(COMPOSER_BIN_DIR) at this point, because Gitlab Templates does not run it's own main.yml template. You just need to allow for that variable being empty in the php script, not add it if null. Or default to 'vendor'. Also if you add --verbose temporarily to the call to the script you will get lots of debug output.
Upgrade Status is not run on the d11-recipe branch because recipes do not have a *.info.yml files so the job is skipped. We can do a temporary test on another branch to check that one job.
All of the warnings in cspell are for files in the new custom custom-bin-folder so that folder needs to be added to the ignored list, like we did for vendor/bin. @dimitriskr this is done in scripts/prepare-cspell.php, the $non_project_directories has a hard-coded vendor/bin which can now be changed to use getenv(COMPOSER_BIN_DIR)
All of the warnings in cspell are for files in the new custom custom-bin-folder so that folder needs to be added to the ignored list, like we did for vendor/bin
This is looking good. To check the default behavior I've triggered the downstream pipelines for d11-recipe and d10-plus, and all end green.
I also have a change in Downstream MR41 and it all passed except CSpell failed. It passes without MR423 so that definitely needs looking at. Details on ✨ Use overriden composer bin-dir if it exists Active
I picked the d11-recipe branch for this work.
The log shows that COMPOSER_BIN_DIR=/builds/project/gitlab_templates_downstream/custom-bin-folder
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/71...
composer-lint, phpcs, phpstan and phpunit all pass.
cspell fails with several unknown words, which is surprising, and needs some investigation.
The Upgrade Status job was not added to the pipeline, so that also needs looking at.
The jobs that are affected by this change are:
- Drupal CMS
- composer-lint
- phpcs
- phpstan
- phpunit
- upgrade status
d7-basic and d9-basic do not have a composer.json
d10-theme, d10-profile and main do not have phpunit tests.
That leaves a choice of d10-plus or d11-recipe. Neither of these currently has the opt-in to run Upgrade Status but that can be added.
I have created 📌 Add test covrage for custom composer bin-dir Active to test this.
This is a good enhancement, Thanks @dimitriskr for starting it. I will create a MR in one of our downstream test projects to see this change in action. Is it just a case of adding that property into the composer.json? I'll start off with that and then see what happens.
[edit: I have also rebase this MR so that 'check versions' now passes]
jonathan1055 → made their first commit to this issue’s fork.
I guess you did not see my request for a credit.
I've updated the issue summary to show the new variable name and give link to the new documentation page.
I have updated issue summary to show what was actually done here.
I added some screenshots to this thread, but it seems to be closed. So just letting you know they are there.
https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/418...
Everything that I wanted to do here is now complete, so its ready from my point of view. There is one @todo about swapping the url when the new test is in a the next stable release. Were you thinking that could be done in a follow-up so we can merge this now? Or did you want to wait?
Thanks, pleased you like the enhancements. I have pushed four more commits, 89 to 92
All the temporary changes are now removed (so downstream d10-theme will fail again) but the other branches should all pass green as we are no longer forcing OPT_IN_TEST_DRUPAL_CMS: 1 on every branch, so the CMS job won't be added.
https://git.drupalcode.org/issue/gitlab_templates-3552178/-/pipelines/64...
Following discussions on MR and slack I have pushed some changes:
1.composer create-project drupal/cms $_CMS_ROOT $_DRUPAL_CMS_TAG is now in a collapsed section which wraps up 810 lines of log
2. composer require drupal/core-dev is also in a collapsed section which wraps up 300 lines
3. Fixed "The repository ... does not have the correct ownership and git refuses to use it" by adding
git config --global --add safe.directory $CI_PROJECT_DIR
4. Added $DIVIDER around error - before and after
5. Cater for an override value for $_DRUPAL_CMS_TAG. Here's an example setting it to 1.2.5 via UI and using 1.2.7 via variable in .gitlab-ci.yml file (on an earlier version of this MR)
There are still some temporary changes (eg skipping jobs, using issue fork) and also some more to add to the docs, but this is ready for review and feedback.
Simply switching the requirement from drupal/stable to drupal/classy in composer.json solves the Drupal CMS compatibility test. Classy requires stable, so both are now loaded, see the log.
We also want to decide on a module to require, as that was the original purpose of this issue.
The job logs have
$ COMPOSER_MIRROR_PATH_REPOS=1 composer require drupal/$MODULE_NAME -W -n
The repository at "/builds/project/gitlab_templates_downstream/cms/recipes/gitlab_templates_downstream/" does not have the correct ownership and git refuses to use it:
fatal: detected dubious ownership in repository at '/builds/project/gitlab_templates_downstream'
To add an exception for this directory, call:
git config --global --add safe.directory /builds/project/gitlab_templates_downstream
Here's an example for d11-recipe and for module d10-plus--add safe.directory?
Yes, all the main processing is good.
I've left some comments/suggestions in the MR threads. There is also some temporary code to remove, but leave that until last, when everything else is finalised.
I tried running all of the downstream branches.
d11-recipe and d10-plus passed as expected
d10-profile failed due to unmatched "profile" project type in the generic test. If profiles can't be tested on Drupal CMS then we should document that. At least OPT_IN_TEST_DRUPAL_CMS variable is 0 by default so the developer/maintainer would simply not opt in for a profile project.
d10-theme failed with
The repository at "/builds/issue/gitlab_templates_downstream-3521050/cms/web/themes/custom/gitlab_templates_downstream/" does not have the correct ownership and git refuses to use it:
fatal: detected dubious ownership in repository at '/builds/issue/gitlab_templates_downstream-3521050'
I don't know what this means.
d9-basic failed with
Could not find a matching version of package drupal/gitlab_templates_downstream. Check the package spelling, your version constraint and that the package is available in a stability which matches your minimum-stability (dev).
This is likely due to this branch only being compatible up to Drupal 10. This is simply a matter of documentation, and the OPT_IN_TEST_DRUPAL_CMS variable is 0 by default.
This is intriguing. I pushed a chnage to start using
✨
Make theme that depends on a module
Active
for the classy error. I also set OPT_IN... to 0 for all variants except Drupal CMS - but the result was that no downstream pipelines were added. Does this mean we have an unintentional link with OPT_IN_TEST_CURRENT or something else that is preventing the CMS job. Needs work based on that too.
jonathan1055 → made their first commit to this issue’s fork.
jonathan1055 → made their first commit to this issue’s fork.
This is an excellent addition to Contrib testing, and I am very happy to have contributed here, learning about Recipes and expanding the structure and scope of our pipelines. The other Gitlab Templates related work also provides good improvements.
The weekly GTD scheduled pipelines currently show the d11-recipe branch as failing phpunit tests, as they have been and was intentional. (see attached screen-grab). The changes in MR408 showed that this fixed the problem, and in a few days the next d11-recipe pipeline should be green
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
I tested the hypothesis that if the module under test also requires a recipe then this installer path is needed. Using the d10-plus downstream branch without MR408 it should have failed, but the recipe is downloaded correctly and placed in the top-level recipes foler.
Here are the changes in that branch and here's the composer job log. I think it is important to find out why we need those installer-paths in the script. Otherwise we are maintaining code for which we don't understand its purpose.
But also, no reason not to merge MR408.
As per comments in the MR I have reverted the change to add DRUPAL_CMS_PROJECT_FOLDER here.