Giter Site home page Giter Site logo

dco-check's People

Contributors

aasseman avatar akomakom avatar alpianon avatar christophebedard avatar tumaysem avatar zyga avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar

dco-check's Issues

Python internal error when manually running dco-check pipeline in GitLab

In GitLab, when a pipeline is not triggered by a commit or by a merge or pull request but it is manually launched, the environment variable CI_COMMIT_BEFORE_SHA is set to "000000000000000000000000000000000", thus leading to the following error:

$ echo "CI_COMMIT_BEFORE_SHA is $CI_COMMIT_BEFORE_SHA"
CI_COMMIT_BEFORE_SHA is 0000000000000000000000000000000000000000

$ dco-check
Detected: GitLab
Checking commits: 0000000000000000000000000000000000000000..114574cb574880de5fc0182828076bbb11a0da55
error: fatal: Invalid revision range 0000000000000000000000000000000000000000..114574cb574880de5fc0182828076bbb11a0da55
Traceback (most recent call last):
  File "/usr/local/bin/dco-check", line 10, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.7/dist-packages/dco_check/dco_check.py", line 1095, in main
    check_merge_commits=options.check_merge_commits,
  File "/usr/local/lib/python3.7/dist-packages/dco_check/dco_check.py", line 568, in get_commits
    individual_commits = split_commits_data(commits_data)
  File "/usr/local/lib/python3.7/dist-packages/dco_check/dco_check.py", line 419, in split_commits_data
    commits_data = commits_data.strip('\n')
AttributeError: 'NoneType' object has no attribute 'strip'

More generally, this happens every time there are no new commits to check on the default branch, but the pipeline is launched anyway, for whatever reason.

This case should be expressly handled by dco-check, by skipping the check entirely when there are no new commits

Add tests

Unit tests + system tests (e.g. create a git repo, make some commits, run the check, etc.)

Add more tests for supported CIs

Follow-up to #2

Currently, the supported CIs are tested by actually running dco-check (via a test) in PRs. That only tests a portion of the features: it doesn't test pushing to the default branch.

This would be a bit more complex to test, since we would need to emulate the CI environments (i.e. define the right env var, provide the right data, etc.), create some commits, and run dco-check.

A first step could be to do this with a simple git repo that we set up in a test.

Please provide aarch64 container

I'd like to use your container in CI but some of my infrastructure has moved to aarch64. It would be great to support that architecture without having to maintain additional containers.

Fails when source branch has been merged in the target branch before

(The following applies to the GitRetriever).
In case the source branch has already been merged into the target branch before, dco-check silently fails:

dco-check --verbose
Options:
        check_merge_commits: False
        default_branch: master
        default_branch_from_remote: False
        default_remote: origin
        quiet: False
        verbose: True

Detected: git (default)
        using default branch 'master'
error: 

This is because the git merge-base --forkpoint master command returns nothing (and exits with 1):

dco-check --verbose
Options:
        check_merge_commits: False
        default_branch: master
        default_branch_from_remote: False
        default_remote: origin
        quiet: False
        verbose: True

Detected: git (default)
        using default branch 'master'
['git', 'merge-base', '--fork-point', 'master']
error: 
$ git merge-base --fork-point master
$

It's relatively easy to reproduce with an empty repo:

mkdir -p /tmp/test-repo
cd /tmp/test-repo
git init
touch init
git add init
git commit -m init
git checkout -b topic
touch topic
git add topic
git commit -m topic
git checkout master
git merge --no-ff topic -m "Merge branch topic into stable"
git checkout topic
touch another_file
git add another_file
git commit -m another_file
dco-check --verbose

Note that the no-ff flag is needed, otherwise a fast-forward merge is done and dco-check doesn't fail in that case.

Could it be related to this note in the manual of git-merge-base?

   Also, the remote-tracking branch you use the --fork-point mode with must be the one your topic forked from its tip. If you forked from an older commit than the tip, this mode would not find the fork point (imagine in the
   above sample history B0 did not exist, origin/master started at B1, moved to B2 and then B, and you forked your topic at origin/master^ when origin/master was B1; the shape of the history would be the same as above, without
   B0, and the parent of B1 is what git merge-base origin/master topic correctly finds, but the --fork-point mode will not, because it is not one of the commits that used to be at the tip of origin/master).

I'm note sure what dco-check could do in this case...
Maybe if merge-base --fork-point failed, we should fallback to using git merge-base master topic?

EDIT: it looks like there could be multiple reason why --forkpoint would fail: https://public-inbox.org/git/[email protected]/t/

Improve behaviour for different CIs

Check if some CIs could test new commits when pushing to an existing branch.

Some CIs also offer a way to know when we're building a branch vs. a PR.

Fails on GitLab when triggering a new pipeline on a branch without new commits

Job after a merge to master: https://gitlab.com/micro-ROS/ros_tracing/tracetools_analysis/-/jobs/612272575

Detected: GitLab
on default branch 'master': will check new commits
Checking commits: 38da641ae4ed7472a814e10ac3513b0c3e5e763a..10310dbd8c3ac5e8e505a9292becaede0bfeff1a

Nightly job that was triggered a bit later (no new changes): https://gitlab.com/micro-ROS/ros_tracing/tracetools_analysis/-/jobs/612419164

Detected: GitLab
on default branch 'master': will check new commits
Checking commits: 0000000000000000000000000000000000000000..10310dbd8c3ac5e8e505a9292becaede0bfeff1a
error: fatal: Invalid revision range 0000000000000000000000000000000000000000..10310dbd8c3ac5e8e505a9292becaede0bfeff1a

So it would seem that this line & the CI_COMMIT_BEFORE_SHA env var give an invalid SHA:

commit_hash_base = get_env_var('CI_COMMIT_BEFORE_SHA')

dco-check run as GH action fails after extensive rebase

Kia ora,

I added dco-check to GH, thank you for that, it's awesome :), and it flagged that a bunch of my commits weren't DCO compliant, so I rebased from scratch - e.g., git rebase -i --root. In the process, I not only reworded, but also squashed and dropped some commits.

This has now broken dco-check, as it seems to be trying to retrieve a commit that no longer exists.

The output:

Detected: GitHub CI

Checking commits: a33bba34234dcc2e98aaa9b5ad7b58211bba17b2..f0a6afac8d98e1c29a472fd929bf8721b5c81da2

  File "/opt/hostedtoolcache/Python/3.9.6/x64/bin/dco-check", line 8, in <module>
    sys.exit(main())
  File "/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/site-packages/dco_check/dco_check.py", line 1128, in main
    commits = commit_retriever.get_commits(
  File "/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/site-packages/dco_check/dco_check.py", line 939, in get_commits
    response = request.urlopen(req)
  File "/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/urllib/request.py", line 214, in urlopen
    return opener.open(url, data, timeout)
  File "/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/urllib/request.py", line 523, in open
    response = meth(req, response)
  File "/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/urllib/request.py", line 632, in http_response
    response = self.parent.error(
  File "/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/urllib/request.py", line 561, in error
    return self._call_chain(*args)
  File "/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/urllib/request.py", line 494, in _call_chain
    result = func(*args)
  File "/opt/hostedtoolcache/Python/3.9.6/x64/lib/python3.9/urllib/request.py", line 641, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 404: Not Found

The actual commits, post rebase:

git log --oneline
f0a6afa (HEAD -> main, origin/main, origin/HEAD) Update README.md
4eb1370 Add DCO check
d928469 Automate assignment to GH project
1107a80 Added skeleton of first client app, and tried to codify project structure
02bd9d8 Initial commit

I'm assuming that state was stored by GH when dco-check cloned the repo, and this confused it. I took the following actions, one of which fixed it, unfortunately I didn't do it in a very scientific manner, my apologies.

  1. Renamed workflow file from dco.yml to DCO-check.yml
  2. Renamed action from "dco" to "DCO check" in the YAML
  3. Renamed the check step from "check DCO" to "DCO check"

Fails to check commits for a brand new branch on GitLab

See here: #94 (comment)

For example, in the comment linked above, I created a new project and pushed a first commit. This created the default branch (master) at the same time. $CI_COMMIT_BEFORE_SHA is then 0000000....

With #94, dco-check disregards situations like that as if there were no new commits. This should be fixed.

Mypy detects several possible type errors

Since the projects has several type annotations I decided to run mypy on it to see what it reports.

zyga@superfx:~/git/dco-check$ mypy .
setup.py:15: error: No library stub file for module 'setuptools'
setup.py:15: note: (Stub files are from https://github.com/python/typeshed)
dco_check/dco_check.py:288: error: Incompatible return value type (got "Optional[Match[str]]", expected "bool")
dco_check/dco_check.py:568: error: Argument 1 to "split_commits_data" has incompatible type "Optional[str]"; expected "str"
dco_check/dco_check.py:587: error: Argument 4 to "CommitInfo" has incompatible type "Optional[str]"; expected "str"
dco_check/dco_check.py:588: error: Argument 5 to "CommitInfo" has incompatible type "Optional[str]"; expected "str"
dco_check/dco_check.py:661: error: Argument 1 to "fetch_branch" has incompatible type "Optional[str]"; expected "str"
dco_check/dco_check.py:906: error: Unsupported operand types for + ("str" and "None")
dco_check/dco_check.py:906: note: Right operand is of type "Optional[str]"
dco_check/dco_check.py:913: error: "pprint" does not return a value
dco_check/dco_check.py:990: error: 'None' object is not iterable
dco_check/dco_check.py:1065: error: Item "None" of "Optional[CommitDataRetriever]" has no attribute "name"
dco_check/dco_check.py:1077: error: Item "None" of "Optional[CommitDataRetriever]" has no attribute "get_commit_range"
dco_check/dco_check.py:1092: error: Item "None" of "Optional[CommitDataRetriever]" has no attribute "get_commits"
Found 12 errors in 2 files (checked 3 source files)

It seems most of those can be reduced to missing None checks.

GitLab integration fails with detached pipelines for mere requests

Our project is using pipelines for merge requests. This feature is documented at https://docs.gitlab.com/ee/ci/merge_request_pipelines/

In the default recommended setup, dco-check 0.0.11 fails as follows:

dco-check
Detected: GitLab
could not get environment variable: 'CI_COMMIT_BRANCH'

Some extra debugging shows more information, interestingly CI_COMMIT_BRANCH is obviously missing and CI_MERGE_REQUEST_EVENT_TYPE=detached indicates that this is a detached merge request.

I think that in this mode, it's sufficient to use CI_COMMIT_REF_NAME instead.

env | grep CI_ | sort
CI_API_V4_URL=https://git.ostc-eu.org/api/v4
CI_BUILDS_DIR=/builds
CI_BUILD_BEFORE_SHA=0000000000000000000000000000000000000000
CI_BUILD_ID=3543
CI_BUILD_NAME=dco
CI_BUILD_REF=d08a62cd72976257694e1598ad1c5bf6520f2034
CI_BUILD_REF_NAME=feature/dco
CI_BUILD_REF_SLUG=feature-dco
CI_BUILD_STAGE=compliance
CI_BUILD_TOKEN=[MASKED]
CI_COMMIT_BEFORE_SHA=0000000000000000000000000000000000000000
CI_COMMIT_DESCRIPTION=
CI_COMMIT_MESSAGE=Add CI job checking DCO
CI_COMMIT_REF_NAME=feature/dco
CI_COMMIT_REF_PROTECTED=false
CI_COMMIT_REF_SLUG=feature-dco
CI_COMMIT_SHA=d08a62cd72976257694e1598ad1c5bf6520f2034
CI_COMMIT_SHORT_SHA=d08a62cd
CI_COMMIT_TIMESTAMP=2021-03-04T15:54:38+01:00
CI_COMMIT_TITLE=Add CI job checking DCO
CI_CONCURRENT_ID=0
CI_CONCURRENT_PROJECT_ID=0
CI_CONFIG_PATH=.ostc-ci/gitlab-ci.yml
CI_DEFAULT_BRANCH=develop
CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX=git.ostc-eu.org:443/OSTC/dependency_proxy/containers
CI_DEPENDENCY_PROXY_PASSWORD=[MASKED]
CI_DEPENDENCY_PROXY_SERVER=git.ostc-eu.org:443
CI_DEPENDENCY_PROXY_USER=gitlab-ci-token
CI_DISPOSABLE_ENVIRONMENT=true
CI_JOB_ID=3543
CI_JOB_IMAGE=registry.ostc-eu.org/ostc/containers/dco-check:latest
CI_JOB_JWT=[MASKED]
CI_JOB_NAME=dco
CI_JOB_STAGE=compliance
CI_JOB_STATUS=running
CI_JOB_TOKEN=[MASKED]
CI_JOB_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos/-/jobs/3543
CI_KUBERNETES_ACTIVE=true
CI_MERGE_REQUEST_DIFF_BASE_SHA=efc6c221c0984c42dc840c54a4554d95c1c68ece
CI_MERGE_REQUEST_DIFF_ID=1381
CI_MERGE_REQUEST_EVENT_TYPE=detached
CI_MERGE_REQUEST_ID=437
CI_MERGE_REQUEST_IID=46
CI_MERGE_REQUEST_PROJECT_ID=92
CI_MERGE_REQUEST_PROJECT_PATH=OSTC/OHOS/meta-ohos
CI_MERGE_REQUEST_PROJECT_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos
CI_MERGE_REQUEST_REF_PATH=refs/merge-requests/46/head
CI_MERGE_REQUEST_SOURCE_BRANCH_NAME=feature/dco
CI_MERGE_REQUEST_SOURCE_BRANCH_SHA=
CI_MERGE_REQUEST_SOURCE_PROJECT_ID=92
CI_MERGE_REQUEST_SOURCE_PROJECT_PATH=OSTC/OHOS/meta-ohos
CI_MERGE_REQUEST_SOURCE_PROJECT_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos
CI_MERGE_REQUEST_TARGET_BRANCH_NAME=develop
CI_MERGE_REQUEST_TARGET_BRANCH_SHA=
CI_MERGE_REQUEST_TITLE=Draft: Add CI job checking DCO
CI_NODE_TOTAL=1
CI_OPEN_MERGE_REQUESTS=OSTC/OHOS/meta-ohos!46
CI_PAGES_DOMAIN=example.com
CI_PAGES_URL=http://ostc.example.com/OHOS/meta-ohos
CI_PIPELINE_ID=1418
CI_PIPELINE_IID=113
CI_PIPELINE_SOURCE=merge_request_event
CI_PIPELINE_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos/-/pipelines/1418
CI_PROJECT_CONFIG_PATH=.ostc-ci/gitlab-ci.yml
CI_PROJECT_DIR=/builds/OSTC/OHOS/meta-ohos
CI_PROJECT_ID=92
CI_PROJECT_NAME=meta-ohos
CI_PROJECT_NAMESPACE=OSTC/OHOS
CI_PROJECT_PATH=OSTC/OHOS/meta-ohos
CI_PROJECT_PATH_SLUG=ostc-ohos-meta-ohos
CI_PROJECT_REPOSITORY_LANGUAGES=bitbake,shell,c++
CI_PROJECT_ROOT_NAMESPACE=OSTC
CI_PROJECT_TITLE=meta-ohos
CI_PROJECT_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos
CI_PROJECT_VISIBILITY=public
CI_REGISTRY=registry.ostc-eu.org
CI_REGISTRY_IMAGE=registry.ostc-eu.org/ostc/ohos/meta-ohos
CI_REGISTRY_PASSWORD=[MASKED]
CI_REGISTRY_USER=gitlab-ci-token
CI_REPOSITORY_URL=https://gitlab-ci-token:[MASKED]@git.ostc-eu.org/OSTC/OHOS/meta-ohos.git
CI_RUNNER_DESCRIPTION=gitlab-runner-gitlab-runner-66758ff899-w9vsm
CI_RUNNER_EXECUTABLE_ARCH=linux/amd64
CI_RUNNER_ID=9
CI_RUNNER_REVISION=8fa89735
CI_RUNNER_SHORT_TOKEN=uKNjDYdc
CI_RUNNER_TAGS=
CI_RUNNER_VERSION=13.6.0
CI_SERVER=yes
CI_SERVER_HOST=git.ostc-eu.org
CI_SERVER_NAME=GitLab
CI_SERVER_PORT=443
CI_SERVER_PROTOCOL=https
CI_SERVER_REVISION=a1a60e9f753
CI_SERVER_TLS_CA_FILE=/builds/OSTC/OHOS/meta-ohos.tmp/CI_SERVER_TLS_CA_FILE
CI_SERVER_URL=https://git.ostc-eu.org
CI_SERVER_VERSION=13.9.0-ee
CI_SERVER_VERSION_MAJOR=13
CI_SERVER_VERSION_MINOR=9
CI_SERVER_VERSION_PATCH=0

GitLabRetriever does not work with pipelines for merge requests

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 #113 (i.e. git merge-base --fork-point can fail in some scenarios).

Ignoring specific commits

Hi,

is there a way to ignore specific commits or only run the dco-check starting from a certain date?

Unfortunately, my dco-check integration was not fail-safe and now I have an unsigned commit. I don't want to alter the git history here since there is already a release, which cannot be changed anymore.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.