Try to remove most of the hardcoded "/custom" paths and use DRUPAL_PROJECT_FOLDER to calculate the right ones

Added some suggestions to the summary.

Yes this is a good idea, and I'll work on it next. It is clear from https://drupal.slack.com/archives/CGKLP028K/p1740436107889029 for example that this will be a useful feature.

Regarding the new variable DRUPAL_PROJECTS_PATH: modules/custom (D8+) or sites/all/modules/custom (D7) we could achieve this in various ways -

(a) we have two new variables, so that the default can be set for each, and viewed in the UI form.

(b) we could have a single variable that holds for example modules/custom, or whatever needs to be over-ridden, and in the D7 jobs we always append "sites/all" in front of this variable. That still allows it to be entered in the UI.

(c) this is not something that will change adhoc, or needed to vary for random testing, so it could be an internal variable, that is derived at the start of the pipeline like we do for PROJECT_NAME and DRUPAL_PROJECT_FOLDER, and this can be overridden in a project's .gitlab-ci.yml.

I am thiking that (c) is my preference, but wanted to get some feedback before starting on this.

Happy with option (c). It's solid. We can assign the default value, document it, and then module maintainers can override if needed.

I have pushed the initial changes. Not tested yet. A few things to note

1. No changes made to the phpcs job (in d10 or d7). When ✨ Add basepath parameter to PHP Code Sniffer Active is merged then we can update this MR
2. I have not yet changed this sed in upgrade status. It may be better to run the job in the project's own folder - see 🐛 Make all jobs report artifacts use source relative paths. Active
3. In phpunit, the D10 run-tests.sh had --directory modules/custom/$CI_PROJECT_NAME so that has been replaced by --directory $DRUPAL_PROJECTS_PATH/$CI_PROJECT_NAME. But the D7 version previously had --directory sites/all/modules/custom , without the final $CI_PROJECT_NAME. I think this was an oversight/error/omission, so I have added that here, making it the same --directory $DRUPAL_PROJECTS_PATH/$CI_PROJECT_NAME

I will test these changes, before setting to NR.

It's looking good so far.

Tested here for D11 with the default values. All OK as expected.
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/435539

To see what happens when we have a non-default value I added

  variables:
    DRUPAL_PROJECTS_PATH: modules/contrib

to the downstream trigger for d10-plus. The composer and eslint jobs ran fine. phpcs failed as expected, because that has not been changed. There is no phpunit job yet for any of the GTD branches (see 📌 Add a phpunit test Active )
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

I triggered the other project's downstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3506040/-/pipelines/43...

Those ran OK. I have now added an override DRUPAL_PROJECTS_PATH: modules/contrib to KeyCDN and API 10+ as they both have phpunit tests.

After the last commit: https://git.drupalcode.org/issue/gitlab_templates-3506040/-/pipelines/43...

I have now completed 📌 Add a phpunit test Active so we can test the phpunit changes downstream too.

Added links to some test results - 'default' for when no values is specified, 'modified' for when a different value is used.

Added more test result links.

I will work on the Upgrade Status sed next.

Waiting on ✨ Add basepath parameter to PHP Code Sniffer Active before making the phpcs changes.

I have made the changes for the upgrade status 'sed'. I tried various things to avoid needing to do it in the first place, such as changing directory to the projects own top-level and running it there, which made no difference. I also tried changing the parameter --root=$_WEB_ROOT to --root=$DRUPAL_PROJECT_FOLDER but the result was still then same, with the long path writen to the output, backslashed, to give modules\/contrib\/mymodule-nnnnn\/mymodule.info.yml exactly as is now. I also tried to create a variable from $DRUPAL_PROJECTS_PATH adding in the backslashes, for use in the 'sed'but that was getting very cumbersome. It worked but needed six backslahes in the raw string, to allow the sed to work. Then I had the simple idea of doing two sed commands, the first just replaces all \/ with a single / . Then the second sed can use the straight value of $DRUPAL_PROJECTS_PATH and replace it with blank. This worked perfectly, and as a bonus it also fixes the backslashed url in the message See https:\/\/drupal.org\/node\/3070687

I've tested this with the modified d9-basic downstream branch, to get some output as required. The composer job nicely shows that this GTD branch is being used, and the upgrade status job has the required debug showing the two changes to the output file.

The above test is with the default value 'modules/custom' so next I will test it with a modified value for $DRUPAL_PROJECTS_PATH

Using DRUPAL_PROJECTS_PATH: modules/anything/goes the downstream job passes

Ignore the red phpcs failure, I will make those changes when ✨ Add basepath parameter to PHP Code Sniffer Active is merged, to save having to re-do it after a conflict.

I've updated the summary with active links to the remainder of the test cases. This is all done except the phpcs job, so is ready for review, or you can wait until the phpcs job is done.

I gave another pass at the code and it looks great so far. Also, great use of the "sed" commands.

This search shows that the only other place where /custom is used is, as you say, in the phpcs job, so we are really close to getting this. Back to NW just based on that, otherwise it'd already be RTBC.

in the phpcs job, so we are really close to getting this.

Yes, that change is trivial to do, I was just leaving it until ✨ Add basepath parameter to PHP Code Sniffer Active is merged, to avoid a conflict. Given that it is RTBC I thought it would not be too long. Also I know there are penty of debug lines to remove.

I actually forgot to merge it! So thanks for the reminder. Doing it now.

✨ Add basepath parameter to PHP Code Sniffer Active is merged now.

Excellent, thank you. In fact, the changes in that PHPCS mr just merged make use of the recently added $DRUPAL_PROJECT_FOLDER variable, which is now derived using $DRUPAL_PROJECTS_PATH. So there is nothing to do in the phpcs jobs at all :-)

The rebased mr downstream jobs for the modified path for phpcs in the downstream pipelines now end green instead of red as before.

I will remove the debug now.

All debug and downstream overrides removed. Ready for final review.

Downstream pipelines after the last commits: https://git.drupalcode.org/issue/gitlab_templates-3506040/-/pipelines/44...

This looks great! RTBC and merging shortly.

I've just realised that this needs documenting. I'll do that tomorrow.

I hit submit on this and the form complained because you just typed the above.

--
Overlapping message:

Actually, before merging it, should we add a couple of lines or a small section in the documentation about this?

Something like:

By default, the current module being tested will be symlinked in the "modules/custom" folder (or "sites/all/modules/custom" for D7) within the Drupal installation, but you can change the location by changing the DRUPAL_PROJECTS_PATH variable. eg: DRUPAL_PROJECTS_PATH: "modules/contrib".

Timing :-)

I have made the doc change - you can preview it here
https://git.drupalcode.org/issue/gitlab_templates-3506040/-/blob/3506040...

On ✨ Consider using contrib instead of custom for module path Needs work I had no idea that the existing MR238 was so similar to this new one. That original is quite old, has multiple conflicts, so was worth me starting afresh, but I had not actually studied that one before.

There are two specific things that are not done in MR334
(a) adding 'contrib' into the path for themes and profiles in expand_composer
(b) changing the php variable name from pathToModules to pathToProject

Do you want either of those changes incorporated here?

I knew that the code touched a lot of common parts but also knew that the MR was quite old, so that's why I marked the issue as related.

I think it's worth getting those two changes in, so yeah, let's do it.

I have pushed those two changes, but have asked some questions in the MR.

All questions resolved, but it would be good to test the change in expand_composer_json.php? Do we know any projects that would be affected by that theme and profile change?

Boostrap theme seems to have the default template https://git.drupalcode.org/project/bootstrap/-/blob/5.0.x/.gitlab-ci.yml... so that'd be a good one to try.

Thanks. I have asked on the Bootstrap slack channel. In the meantime I have been using our new GTD d10-theme branch to test this, and specifically MR9 which adds some requirements of 3rd-party themes and a module. It all works well so far, with the projects being loaded in the correct directories.

I also realised that we have the $PROJECT_TYPE variable, which will be 'module', 'theme' or 'profile'. So instead of hard-coding "modules/custom" as the default if no override is given, we can now use DRUPAL_PROJECTS_PATH="${PROJECT_TYPE}s/custom" so that themes and profiles automatically get the correct default.

Here's a downstream branch where I have removed the override variable for "themes/custom".
https://git.drupalcode.org/issue/gitlab_templates_downstream-3511568/-/p...
The correct default is determined - as shown when browsing the artifacts

I've made the same change to main-d7 even though there is no project type to be read from the .info files. We now cater for an override variables: PROJECT_TYPE which we did not before, and default it to 'module' if none. This keeps things in line and easier to maintain.

If you are OK with these changes then I will update the doc to show the new more relevant defaults.

I have also tested these changes on a new d10-profile branch in GTD - see MR10 in 📌 Add a branch to test a 'profile' project type Active

When those two GTD merge requests are complete, I can reverted the downstream branches in this MR.

I have updated the doc page with the new derived defaults.

This covers everything now, so is ready for final review.

100% ok with the changes in #37, in fact, I love this: DRUPAL_PROJECTS_PATH="${PROJECT_TYPE}s/custom" as it makes it fully automatic. And the D7 workaround is good as well.

I'm reviewing the code now.

I think the changes look great. I have no further feedback. You did some testing on #37 and #38. I'm not sure if you want to cover any more.

Downstream pipelines (all of them) ran here: https://git.drupalcode.org/issue/gitlab_templates-3506040/-/pipelines/44...

There are a couple of new lines as debug, but I think those are meant to stay there permanently, so I'm marking this RTBC.

I cannot run the new "profile" and "theme" downstream pipelines, I'm getting this:

First time I see it.

Oh see why, it's what you mentioned above in #38. They are forks and not merged yet. Once those are merged and the changes are made here too, then RTBC, but for now, NW just for that.

I was typing the below, but you got there first.

This MR currently uses two downstream branches, to test the proposed additions to 'd10-theme' and 'd10-profile' so I will revert those back to running the default downstream branches before you commit this.

Before we merge the downstream MRs and I remove those branch overrides, for our own information and knowledge, can you request write access on one of those issue branches, then re-try the downstream job. I presume that is all it needs.

Good suggestion. Yeah, that was it. I requested access to the "profile" fork and I can now run the downstream pipeline.

I assumed that when creating a fork, all project members were copied over to the fork, but as it happens, it's not the case, it's just the person creating the fork. This is ok, I just assumed otherwise.

Good to have that confirmed. I've now merged those two downstream MRs and reset the downstream branches here. Also as per this slack thread I've moved OPT_IN_TEST_MAX_PHP: 1 out of .downstream-base and into just the projects where we need it.

Downstream pipelines for GTD all work as expected. The d10-profile and d10-theme branches no longer run Max PHP. Setting to NR.

Thanks for the last changes. I think this is ready. Downstream pipelines run well after the last two commits.
RTBC.

Merged. Thanks for this one, it's a great feature adding flexibility in the set up.

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