Thread (71 messages) 71 messages, 6 authors, 2024-07-24

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.

-Justin
So 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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help