Closed alpianon closed 3 years ago
Merging #94 (509f33a) into master (cc6cb61) will decrease coverage by
5.00%
. The diff coverage is0.00%
.
@@ Coverage Diff @@
## master #94 +/- ##
==========================================
- Coverage 73.12% 68.11% -5.01%
==========================================
Files 2 2
Lines 480 483 +3
Branches 77 78 +1
==========================================
- Hits 351 329 -22
- Misses 98 130 +32
+ Partials 31 24 -7
Impacted Files | Coverage Δ | |
---|---|---|
dco_check/dco_check.py | 67.98% <0.00%> (-5.03%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update cc6cb61...509f33a. Read the comment docs.
I set up a dummy repo on GitLab to show when the problems happens and how the proposed fix solves it.
https://gitlab.com/alpianon/test-dco-check
This is the first pipeline log with dco-check 0.0.11 (without the fix) run on default (master) branch: it fails because it gets 0000000000000000000000000000000000000000
as CI_COMMIT_BEFORE_SHA
from GitLab.
Then I changed .gitlab-ci.yml
to use the pacthed dco-check, with the proposed fix. The first pipeline after this change actually has a new commit to detect, so it works normally; see this log:
Finally I re-launched the pipeline from GL webUI, CI_COMMIT_BEFORE_SHA
is 0000000000000000000000000000000000000000
because there are no new commits, but now dco-check works correctly, see the log
Hi, thanks for reporting this and for proposing a fix!
I think something like this happened for scheduled pipelines, since there's never any new commits for those, which is why dco-check
simply ignores scheduled pipelines.
Your explanation looks good, but, just to make sure, how was the first pipeline triggered? Was it manual or automatic?
Because it looks like there are actually two issues here:
$CI_COMMIT_BEFORE_SHA == 0000000...
$CI_COMMIT_BEFORE_SHA == 0000000...
.gitlab-ci.yml
(resulting from your 2 commits) as the only commit, and we get $CI_COMMIT_BEFORE_SHA == 0000000...
in an automatic pipeline (GitLab default perhaps) when we do have a new commit: https://gitlab.com/christophebedard/dco-check-test/-/jobs/1067920206I know the second issue is really a corner case :laughing:, so I wouldn't mind merging this and opening a separate issue to figure out how to fix issue 2 above.
Let me know what you think/if you're okay with that.
I think something like this happened for scheduled pipelines, since there's never any new commits for those, which is why
dco-check
simply ignores scheduled pipelines.Your explanation looks good, but, just to make sure, how was the first pipeline triggered? Was it manual or automatic?
It was manual (via webUI). At first thought, I was thinking of adding another exception for the case where CI_PIPELINE_SOURCE is 'web' (currently there is only an exception for 'schedule'), but since there may be other cases in which a pipeline is triggered without new commits, I thought it was best to handle the general case.
Because it looks like there are actually two issues here:
manually-triggered pipelines get
$CI_COMMIT_BEFORE_SHA == 0000000...
- your last pipeline shows this, and I tried it too with my own project: https://gitlab.com/ros-tracing/ros2_tracing/-/jobs/1067806356
correct
pipelines for (default?) branches that had no commit beforehand get
$CI_COMMIT_BEFORE_SHA == 0000000...
- perhaps this is what happened with your first pipeline too
yes, it is
- but to make sure, I created a new repo and pushed your
.gitlab-ci.yml
(resulting from your 2 commits) as the only commit, and we get$CI_COMMIT_BEFORE_SHA == 0000000...
in an automatic pipeline (GitLab default perhaps) when we do have a new commit: https://gitlab.com/christophebedard/dco-check-test/-/jobs/1067920206- in this case, your fix would fail to catch a missing sign-off. See this pipeline run (automatically again) on the very first commit (that is not signed-off) on another test repo: https://gitlab.com/christophebedard/dco-check-test2/-/jobs/1067943798
yes, you're right.
I know the second issue is really a corner case laughing, so I wouldn't mind merging this and opening a separate issue to figure out how to fix issue 2 above.
I agree. A possible solution would be to check git log and see if that there is only one initial commit, and check that.
Fix #92
In GitLab, when a pipeline is launched but there are no new commits (eg. when a pipeline is manually launched via GL webUI),
CI_COMMIT_BEFORE_SHA
is set to a string of 40 zeros. This fix expressly handles this case, to avoid internal python errors and to correctly skip DCO check because it's not needed.I added an example in the comment below, with GitLab pipeline logs from a dummy repo