Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GitLab pipelines on default branch with no new commits #94

Merged

Conversation

alpianon
Copy link
Contributor

@alpianon alpianon commented Mar 2, 2021

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

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 when it's not needed.

Signed-off-by: Alberto Pianon <alberto@pianon.eu>
Signed-off-by: Alberto Pianon <alberto@pianon.eu>
@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #94 (509f33a) into master (cc6cb61) will decrease coverage by 5.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            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%) ⬇️

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.

@alpianon
Copy link
Contributor Author

alpianon commented Mar 2, 2021

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.

immagine

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:

immagine

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

immagine

@christophebedard christophebedard self-requested a review March 2, 2021 20:37
@christophebedard christophebedard added the bug Something isn't working label Mar 2, 2021
@christophebedard
Copy link
Owner

christophebedard commented Mar 2, 2021

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:

  1. manually-triggered pipelines get $CI_COMMIT_BEFORE_SHA == 0000000...
  2. pipelines for (default?) branches that had no commit beforehand get $CI_COMMIT_BEFORE_SHA == 0000000...

I know the second issue is really a corner case 😆, 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.

@alpianon
Copy link
Contributor Author

alpianon commented Mar 3, 2021

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:

  1. manually-triggered pipelines get $CI_COMMIT_BEFORE_SHA == 0000000...

correct

  1. 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

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.

Copy link
Owner

@christophebedard christophebedard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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