Re: [PATCH 7/8] ci: run style check on GitHub and GitLab
From: Karthik Nayak <hidden>
Date: 2024-07-10 13:38:37
Justin Tobler [off-list ref] writes: [snip]
quoted
You can see the output $ ./ci/run-style-check.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" fatal: ambiguous argument '': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' This only happens if "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" is empty.Ya I noticed this failure, but was wondering if it was maybe due to something else. I have been unable to reproduce it and in all the jobs I was running resulted in a merge pipeline with the variable defined. But maybe sometimes a regular pipeline gets run for some reason and consequently $CI_MERGE_REQUEST_TARGET_BRANCH_SHA is not defined? Was the pipeline triggered directly from the source branch?
Just a regular push. Not sure at all why this happened. I was testing different types of style issues on the CI and this happened once.
quoted
quoted
https://gitlab.com/gitlab-org/git/-/jobs/7291792329#L2470 Do you have an example of the check-whitespace job failing in GitLab CI? Maybe I'm missing something, but I don't see a problem. -JustinSo I think I get the issue, GitLab has two kinds of pipelines it runs: 1. merge pipeline: Here the pipeline runs on the source branch (the feature branch which has to be merged). 2. merged pipeline: Here the pipeline creates a merge commit using the source and target branch and then runs the pipeline on the merged commit.Correct, this is my understanding.quoted
And "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" is only defined in the 'merged pipeline'. If you see the pipelines for my branch on GitLab [1]. You'll see only one of them is marked as 'merge results' and the others being marked as 'merged results'. The former includes the job I mentioned above, where "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" is not defined. I'm still not sure why it marked only one of the pipelines as such, but this means there is chance that it could happen.Huh, I'm guessing the CI job must have been triggered from the source branch directly. Did you manually run the CI job? I wonder if that could have caused it.
Not that I remember.
quoted
So I guess the best outcome is to use "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA", but fallback to "$CI_MERGE_REQUEST_DIFF_BASE_SHA", if the former is not defined.This is exactly what I think we should do too. For merge pipelines we will want to use $CI_MERGE_REQUEST_TARGET_BRANCH_SHA so that only the commits included in the MR are scanned in CI. If that variable is not defined, it makes sense to fallback to $CI_MERGE_REQUEST_DIFF_BASE_SHA. Since it's not a merge pipeline it will only scan commits included from the MR and therefore work as expected. This should handle both cases correctly. :) -Justin
Yeah seems like the best solution at this point, let me implement this.
Attachments
- signature.asc [application/pgp-signature] 690 bytes