UI variables take precedence over build.env

Now that we are using build.env to hold the generated variables, maybe we should only write to that file variables that are distinct from the UI form inputs. Then we know that anything written to that that file will never get overriden in a subsequent job.

Changed the issue description with two proposed resolutions and a link to the GitLab documentation.

You have a good point in #2, but if maybe using the defaults fixes things, we might want to keep things that way. What I think is that we should put somewhere in the documentation about the variable precedence.

Also, removing the _CURL* variables from the UI form (aka move them to the hidden file) would probably solve this.

I'm keen on exploring different options and opinions in any case.

_CURL_TEMPLATES_REPO: $_GITLAB_TEMPLATES_REPO
_CURL_TEMPLATES_REF: $_GITLAB_TEMPLATES_REF

I'm sure we tried this already, right at the start, when manipulating those two variables so they work in UI and in a customised .gitlab-ci.yml.

removing the _CURL* variables from the UI form (aka move them to the hidden file) would probably solve this.

I would like to try that first, as it seems like a potential solution. Is there a way to test the UI for submitting a pipeline in a Gitlab Templates MR, before committing the change to 'main'?

Or maybe they do not need to be in the hidden file either?

Didn't think of that. Worth a try for sure (we'd need to add a deprecation warning that they are no longer needed in that case).

Is there a way to test the UI for submitting a pipeline in a Gitlab Templates MR, before committing the change to 'main'?

I'm not sure I fully understand what you mean here. If you go to the pipelines section of the fork, you can trigger the pipeline for the fork and it will show the form via the UI for the variables. Not sure if that's what you mean.

we'd need to add a deprecation warning that they are no longer needed in that case

They would still be needed when testing a MR via a customized .gitliab-ci.yml so there is no change there. It's just that they are not needed in the form becuase we should be able to use the _GITLAB_TEMPLATES... values to set them. I don't think any deprecation notice is needed, because we are not deprecating anything.

I'm not sure I fully understand what you mean here.

What I meant was how to test gitlab_templates MR299 on a contrib project, when running the contrib pipeline via UI.

You're right. No need for deprecation.

--

You'd need to create an issue/fork (no need for an MR unless you want to test the MR itself), reference MR299 in the `.gitlab-ci.yml` and then go to the pipelines section of the fork and trigger a new one. That should make the GitLab UI form appear.

Thanks for the hint. I have replicated the error in this pipeline
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/359286

cspell fails in the same way.
phpcs failed in the curl, but the job ran OK. We are only testing for phpcs.xml.dist which the project does not have, but it does have phpcs.xml, so we are doing the curl unnecessarily (but that's a separate issue)
phpstan passed because the project has its own phpstan.neon so does not use curl. I will remove the projects own file on the next test.

Here is a re-test using MR299 and submitting via the UI. The two variables are no longer shown in the form (see attached)
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/360126

cspell - passes green
phpcs - runs properly (but ends red due to the projects customised phpcs.xml being renamed)
phpstan - runs properly (but highlights warnings due to project's own phpstan.neon being renamed)
phpunit - runs OK

I have checked the documentation https://project.pages.drupalcode.org/gitlab_templates/info/testing-mrs/ and nothing needs to be changed.

This is ready for review.

Downstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3491556/-/pipelines/36...

And it'd probably be great to have a test against an MR, to make sure we've covered:
- Direct project (via downstream pipelines)
- Direct project via UI form (via the fork test you did)
- MR (no test run yet)

The code looks good, so I'm marking RTBC, but will wait for a MR test before merging.

Here is a MR pipeline containing the same scheduler changes as in the test in #11, except I also set the two _curl variables in the .gitlab-ci.yml
https://git.drupalcode.org/project/scheduler/-/pipelines/360182
All the curl calls are run correctly.

You mentioned in #4 about documenting the variable precedence. I could not see an obvious place where we'd put this, but I guess 'custimizations' is the closest relevent page?

Good point, I forgot about that one. I think here is the best place: https://project.pages.drupalcode.org/gitlab_templates/info/customization...

Back to NW just based on the documentation update.

Meanwhile, Date Point's build is green again. Has something been already changed?
https://git.drupalcode.org/project/date_point/-/pipelines/359856

Hello Chi,
There were some other changes in the last few days, if you are using '1.6.x' latest you would get them, but the 'default' ref has not been chnaged since 21 November.

But this fault has not been fixed yet. Regarding your successful pipeline, it depends how it was submitted, as this fault only affected pipeline started from the UI form. If it was a scheduled pipeline or from a merge request then those are not affected. This pipeline did not have _SHOW_ENVIRONMENT_VARIABLES: 1, which is fine, you don't need it on all the time. But that is why I am asking how you ran that pipeline.

I think it would be good for the composer jobs to write out some basic info about the pipeline, project, etc. I have this on my testing jobs, but it should be standard in all pipelines. But that is for a separate issue.

Regarding #14 I have added a paragraph here
https://git.drupalcode.org/issue/gitlab_templates-3491556/-/blob/3491556...
Ready for review

Thanks for the related issue and for the documentation update. RTBC.

Merged. Thanks!