Fix CI_SERVER_URL usage as it changes on external instances

I introduced a new variable for this altogether instead of relying on CI_SERVER_URL.

The MR is ready for review. The downstream pipelines are running: https://git.drupalcode.org/project/gitlab_templates/-/pipelines/495616

Note that we don't have any external integration to run this on, but given that we are hardcoding the URL to the correct value, it should just work.

fjgarlin → credited heddn → .

This all looks good. Updated the IS with the actual new variable.

This was merged and a new tag was made, which is as well the new default-ref for all contrib.

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

@fjgarlin Please could you re-open this issue. Now that we have changed the get-file-via-curl.sh script to use the new $DRUPALORG_CI_SERVER_URL this variable should really be displayed in the log when there is a curl failure. I just tried to trigger manually a Composer job from a pipeline first run a week ago, and the new curl script failed. I think it was because the variable $DRUPALORG_CI_SERVER_URL did not exist in Gitlab Templates then, but when downloading the gitlab_templates/scripts/get-file-via-curl.sh it gets the latest version, which is using the new variable. See this job log

I know it's an edge-case, and probably will not happen often, but it would be better to display all of the variables values used, then it is clearer to see what happened.

Good call. Re opened.

jonathan1055 → changed the visibility of the branch main to hidden.

I've made the change, and doing some testing in GTD.

In the d7-composer branch the I set _CURL_TEMPLATES_REPO: '' which did not cause a problem in the composer job, because we re-calculate if it is blank. The build.env file has

_CURL_TEMPLATES_REPO=issue/gitlab_templates-3524073
_CURL_TEMPLATES_REF=3524073-drupalorg-ci-server-url
GITLAB_TEMPLATES_VERSION=3524073-drupalorg-ci-server-url (branch)

which is what we expect. But the subsequent phpcs job fails, the value of _CURL_TEMPLATES_REPO seems to have been lost when the script executes

# REPO and REF need be non-blank to ensure that the curl will give an easily detectable return code.
_CURL_TEMPLATES_REPO=${_CURL_TEMPLATES_REPO:-repo-is-blank}
_CURL_TEMPLATES_REF=${_CURL_TEMPLATES_REF:-ref-is-blank}

and we get the url with https://git.drupalcode.org/repo-is-blank/ at the start. This is problem 1, but more serious is the fact that the curl command still does not give a failed return code even when it fails. See this d7-composer phpcs log. The command is deemed to be a success and we don't get anything in $EXIT_CODE so there is no message when we'd like there to be one.

Any ideas?

In d10-theme the intended failure works, and we get the message with the updated output.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/54...

For error 1, where $_CURL_TEMPLATES_REPO being empty in the phpcs job even though the value is written to build.env, maybe this is due to the variable hierarchy? It could be that if the variable is defined in the custom .gitlab-ci.yml (or in this case the downstream 'trigger' job) then this takes precedence ov the value in build.env. If that is it, then it's fine.

For error 2, I'm goint to try again without making the variable non-blank, to see if that effects the exit code.

Well, for problem 1, I'd say that we change the logic from

# REPO and REF need be non-blank to ensure that the curl will give an easily detectable return code.
_CURL_TEMPLATES_REPO=${_CURL_TEMPLATES_REPO:-repo-is-blank}
_CURL_TEMPLATES_REF=${_CURL_TEMPLATES_REF:-ref-is-blank}

to

if [[ $_CURL_TEMPLATES_REPO == "" || $_CURL_TEMPLATES_REF == "" ]]; then
   echo "Error message goes here..."
   exit 1;
fi

And yeah, according to https://docs.gitlab.com/ci/variables/#cicd-variable-precedence, pipelines variables (3) have precedence over "dotenv" reports (7).

Thanks for that suggestion, yes I think it is better to detect empty variables and fail directly, rather than try to let the curl fail. I've pushed a change, and set EXIT_CODE and then the same exit block displays all the values. I used -1 so that we do not clash with any actual curl exit codes which are all positive

Here's the test in d7-composer which has the blank value
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/54...

Ready for review.

The solution looks good.

I think it's ready for final clean up and RTBC. NW for now until the clean up happens.

Debug and test lines removed, ready for final review.
This is actually a good improvement to check the variables for blank first, it makes it more reliable to fail if anything is empty.

Agree. Easier to read and no need to make assumptions, just fail if not all inputs needed are present.

Merged. Thanks for the improvement to the script and log.

Thanks. To verify, I have just re-run the 'next minor' job that I linked to in #10 and it now does exactly as intended. The output is clearer, and we can see that the problem was caused by DRUPALORG_CI_SERVER_URL being empty.

https://git.drupalcode.org/project/scheduler/-/jobs/5459742#L38