Inform jobs about the "composer" job exit status and bail out early

There is an error further up the log, before this curl.

scripts-142962-2601438/step_script: line 309: cd: /builds/project/sobki_profile_bootstrap/web/modules/custom/sobki_profile_bootstrap: No such file or directory

https://git.drupalcode.org/project/sobki_profile_bootstrap/-/jobs/260143...
Maybe this needs looking at first as it could be affecting the subsequent processing

And even earlier, in the composer next major job: https://git.drupalcode.org/project/sobki_profile_bootstrap/-/jobs/2601434

I think that’s the culprit

I was just going to post that too. It's very easy to look just at the job, when there is a link to the job not the pipeline.

On another point, this scenario shows the downside of having to have 'composer (next major)' with allow_failure: true. It would be good to find a better way to accomplish that.

This is only temporary though: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/includes...

Yes. In #3463894-37: Update templates so 11.0 is the default/current branch → you said

I think that's the difference between needs and dependencies but I'm not sure. Maybe something to play with in another issue.

Maybe we should look at that. This was precisely the extra support effort that would be good to avoid.

Yup. Maybe we can use this issue to experiment a bit on this.

I made a quick test, based on yours from #3463894-35: Update templates so 11.0 is the default/current branch → but adding

dependencies:
    - "test a"

to test b. It still ran ok.
https://git.drupalcode.org/issue/gitlab_templates-3471235/-/pipelines/26...

https://docs.gitlab.com/ee/ci/yaml/#dependencies implies that it is the artifacts that are needed, so maybe 'test a' has to create an artifact, and when it does not then that would stop 'test b' running.

It might still be the "allow_failure: true" that makes it run.

If you remove it:
- with "needs" it should still run, but only after "test a"
- with "dependencies" it would not run

I'm not 100% sure tho.

In any case, if we find out after the investigation here that having "allow_failure: true" in the "composer (next major)" is not really beneficial, we can remove it, as we still have it in the testing jobs.

if we find out after the investigation here that having "allow_failure: true" in the "composer (next major)" is not really beneficial, we can remove it

I thought that we had to leave that in, otherwise we woud be breaking exising pipelines now that OPT_IN_TEST_NEXT_MAJOR is 1 by default.

True!! I forgot about that part. 🤦‍♂️

Well, again, this "allow_failure: true" is just temporary, so again, I'm not too worried because we happen to be right in the middle of the transition period.

So if the "needs" vs "dependencies" won't make a difference, the action here might be to do nothing and wait a week or two, as the errors are being reported anyway. We might need to give some extra support via here or slack but we know where to look at now after the investigation here in this issue.

I might have a little investigation about setting a composer environment variable to show it has completed OK, then retrieve this variable (in a reusable snippet) at the start of PHPStan and PHPUnit jobs, and if the Composer did not complete properly then halt the job with a message saying so, rather than let the developer waste time looking at the wrong log.

I like that. It a good workaround so we can keep the allow_failure:true setting for next major.

Updated the title and the description.

I've done some work on this. The changes will halt any PHPStan, PHPUnit or Nightwatch job if the corresponding Composer job did not reach the end. Initially I thought the check should be done in all jobs, but it is highly unlikely that anyone would have allow_failure:true in the main "current" composer job. This is really only to cater for the variants, which means just the three jobs mentioned.

I also found that a badly-formed composer.json would not cause expand_composer_json.php to stop, we just get a warning about foreach{} then it continues, so no exit code could be detected. This is improved by adding an error handler which exits on E_WARNING.

Here is a test pipeline https://git.drupalcode.org/project/scheduler/-/pipelines/289124

• The composer Max PHP has a before_script that modifes the composer.json leaving it valid but with packages that cannot be installed.
• Composer Previous Major has a before_script that writes 'force-a-failure' to composer.json making it invalid.
• The subsequent PHPStan and PHPunit variants halt immediately with a clear message:

************************************************************
ERROR: The pre-requisite Composer job did not complete successfully.
Check the log of that job for details. 
************************************************************

This is ready for initial feedback.

Added testing instructions to the issue summary.

Adding 🐛 phpstan (next major) failing on setup Postponed: needs info as a related issue. This MR will solve that problem.

Code looks good, tho i have a question about one part of it where we might not capture the error as intended.

Also, there is a debug line.

Once those two things are sorted i think it’ll be ready.

I've removed the debug line and replied to the MR question.

In followup to to your question, here is a previous test, when I was developing the work. This job has a mal-formed composer.json but the expand_composer ignores the warning, carries on and the job ends OK, thus writing echo "COMPOSER_END_CODE=0" >> build.env
https://git.drupalcode.org/project/scheduler/-/jobs/2817522
This is what led me to use set_error_handler() to detect this and give an exit code.

Thanks for the explanation and for addressing the feedback.
Dowstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3471235/-/pipelines/28...

The code looks good. RTBC unless there is something discovered in the above pipelines.

fjgarlin → credited apotek → .

Bringing credit from duplicate issue.

Merged. Thanks.

Thank you for the great work! This will make module maintenance much clearer!

@apotek you are welcome! and thanks for reporting the discrepancy in the first place on the other issue.

I just added 🐛 breaks if no composer.json is present on the module? Active , which it might be a follow up to this.

Unrelated, I commented on the other issue.

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