christophebedard / dco-check

Simple DCO check script to be used in any CI
Apache License 2.0
12 stars 6 forks source link

GitLabRetriever does not work with pipelines for merge requests #114

Open rmelotte opened 2 years ago

rmelotte commented 2 years ago

When the pipelines for merge requests feature is used, dco-check fails with the following error:

Options:
    check_merge_commits: False
    default_branch: master
    default_branch_from_remote: False
    default_remote: origin
    quiet: False
    verbose: True
Detected: GitLab
could not get environment variable: 'CI_COMMIT_BRANCH'
    on merge request branch 'None': will check new commits off of target branch 'main'

I reproduced it in a test repository here: https://gitlab.com/raphael.melotte/dco-check-test/-/jobs/1655135885 . And it's also possible to reproduce it manually outside of Gitlab:

export GITLAB_CI=y
export CI_DEFAULT_BRANCH=master
export CI_COMMIT_SHA="$(git rev-parse HEAD)"
export CI_PIPELINE_SOURCE=merge_request
export CI_MERGE_REQUEST_ID="my-mr"
export CI_MERGE_REQUEST_TARGET_BRANCH_NAME="master"
export CI_MERGE_REQUEST_TARGET_BRANCH_SHA=""
dco-check --verbose

The problem is that CI_COMMIT_BRANCH is unset in pipelines for merge requests. From https://docs.gitlab.com/ee/ci/variables/predefined_variables.html:

The commit branch name. Available in branch pipelines, including pipelines for the default branch. Not available in merge request pipelines or tag pipelines.

A possible workaround is to manually set the variable. For example:

dco:
  stage: build
  variables:
    CI_COMMIT_BRANCH: $CI_MERGE_REQUEST_SOURCE_BRANCH_NAME
  needs: []
  image: christophebedard/dco-check:latest
  rules:
    - if: $CI_MERGE_REQUEST_ID
    - if: $CI_EXTERNAL_PULL_REQUEST_IID
  script:
    - dco-check --verbose

With that, dco-check fails, but without giving any reason. See this job, or run the following in a local repository:

export GITLAB_CI=y
export CI_DEFAULT_BRANCH=master
export CI_COMMIT_SHA="$(git rev-parse HEAD)"
export CI_PIPELINE_SOURCE=merge_request
export CI_MERGE_REQUEST_ID="my-mr"
export CI_MERGE_REQUEST_TARGET_BRANCH_NAME="master"
export CI_MERGE_REQUEST_TARGET_BRANCH_SHA=""
export CI_COMMIT_BRANCH=test
dco-check --verbose

This time the problem is with the CI_MERGE_REQUEST_TARGET_BRANCH_SHA variable: it's set, but empty in merge requests pipelines:

The HEAD SHA of the source branch of the merge request. The variable is empty in merge request pipelines. The SHA is present only in merged results pipelines.

As a workaround for the current version, one can set it manually. For example:

dco:
  stage: build
  variables:
    CI_COMMIT_BRANCH: $CI_MERGE_REQUEST_SOURCE_BRANCH_NAME
  needs: []
  image: christophebedard/dco-check:latest
  rules:
    - if: $CI_MERGE_REQUEST_ID
    - if: $CI_EXTERNAL_PULL_REQUEST_IID
  script:
    - CI_MERGE_REQUEST_TARGET_BRANCH_SHA=$(git merge-base --fork-point origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME) dco-check --verbose -b origin/master

or:

export GITLAB_CI=y
export CI_DEFAULT_BRANCH=master
export CI_COMMIT_SHA="$(git rev-parse HEAD)"
export CI_PIPELINE_SOURCE=merge_request
export CI_MERGE_REQUEST_ID="my-mr"
export CI_MERGE_REQUEST_TARGET_BRANCH_NAME="master"
export CI_MERGE_REQUEST_TARGET_BRANCH_SHA=""
export CI_COMMIT_BRANCH=test
export CI_MERGE_REQUEST_TARGET_BRANCH_SHA=$(git merge-base --fork-point origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME)
dco-check --verbose

While this works, it has the same problem as in https://github.com/christophebedard/dco-check/issues/113 (i.e. git merge-base --fork-point can fail in some scenarios).

christophebedard commented 2 years ago

Hi! Thanks for reporting this.

The problem is that CI_COMMIT_BRANCH is unset in pipelines for merge requests.

I think this is the same issue as #104. I just pushed a branch I worked on a while ago with a potential fix for this: gitlab-detacted-pipelines-fix. I can't test it because I don't have access to this feature (from what I remember). If you could test it, it'd be great!

ydirson commented 9 months ago

Just tested this branch, see https://gitlab.com/xen-project/xen-guest-agent/-/merge_requests/30

There is a command error that does not get caught, and the DCO is reported as good without even being checked.

christophebedard commented 9 months ago

I'll fix this so that the error is properly reported. As for why the error happens, can you try doing a git fetch in CI before running dco-check? I'm guessing it doesn't know anything about the main branch (target) since it's a "detached" pipeline.

ydirson commented 8 months ago

See https://gitlab.com/xen-project/xen-guest-agent/-/jobs/5796509345

As you said it would help to know about main branch, but then it would be origin/main.

If I create the local main branch OTOH it starts to work:

https://gitlab.com/xen-project/xen-guest-agent/-/jobs/5796533504