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

Gerrit source step doesn't work well with multiple codebases #3460

Open
willlovett-arm opened this Issue Jul 21, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@willlovett-arm
Contributor

willlovett-arm commented Jul 21, 2017

I have a project that consists of multiple codebases, usually with the same branch names, within a gerrit instance.

My GerritChangeSource monitors them and performs pre-commit testing on the project (on patchset-created events) by checking out each of these codebases using a Gerrit step.

However, Gerrit.build.getProperty("event.patchSet.ref") is not currently indexed by codebase, so every Gerrit step attempts to checkout the change ref, which fails on all but the modified codebase.

I currently work around this in my buildbot config by subclassing Gerrit:

from buildbot.steps.source.gerrit import Gerrit

class MultiCodebaseGerrit(Gerrit):
    def __init__(self, **kwargs):
        Gerrit.__init__(self, **kwargs)

    def startVC(self, branch, revision, patch):
        if self.sourcestamp.codebase == self.codebase:
            super(Gerrit, self).startVC(branch, revision, patch)
        else:
            super(Git, self).startVC(branch, revision, patch)
  1. Am I missing some easier way of doing this?, and if not
  2. Is the patch below roughly the right way of fixing it? If so, I'll create a pull request
diff --git a/master/buildbot/steps/source/gerrit.py b/master/buildbot/steps/source/gerrit.py
index 979f77ce0..4c70ab184 100644
--- a/master/buildbot/steps/source/gerrit.py
+++ b/master/buildbot/steps/source/gerrit.py
@@ -26,7 +26,11 @@ class Gerrit(Git):

     def startVC(self, branch, revision, patch):
         gerrit_branch = None
-        if self.build.hasProperty("event.patchSet.ref"):
+        if self.sourcestamp.codebase != self.codebase:
+            # The change is from a different codebase, so just check out the
+            # head of the given branch
+            pass
+        elif self.build.hasProperty("event.patchSet.ref"):
             gerrit_branch = self.build.getProperty("event.patchSet.ref")
             self.updateSourceProperty("gerrit_branch", gerrit_branch)
         else:

Thanks!

Will Lovett

@tardyp

This comment has been minimized.

Show comment
Hide comment
@tardyp

tardyp Jul 22, 2017

Member

The patch looks good. Please send a PR, and don't forget to update the unit tests so that we don't forget about this use case.

docs.buildbot.net/latest/developer/tests.html

Member

tardyp commented Jul 22, 2017

The patch looks good. Please send a PR, and don't forget to update the unit tests so that we don't forget about this use case.

docs.buildbot.net/latest/developer/tests.html

willlovett-arm added a commit to willlovett-arm/buildbot that referenced this issue Sep 22, 2017

Fix to support multiple gerrit codebases (issue buildbot#3460)
- Adds a check to the gerrit source step that this codebase is the one that
  has been changed
- Allows gerrit reporter to use the gerrit event.patchset.revision as its
  revision.  This allows for the possibility that the gerrit startVC has not
  yet been executed when a failure occurs, and so got_revision will not be
  set

willlovett-arm added a commit to willlovett-arm/buildbot that referenced this issue Sep 22, 2017

Fix to support multiple gerrit codebases (issue buildbot#3460)
- Adds a check to the gerrit source step that this codebase is the one that
  has been changed
- Allows gerrit reporter to use the gerrit event.patchset.revision as its
  revision.  This allows for the possibility that the gerrit startVC has not
  yet been executed when a failure occurs, and so got_revision will not be
  set

tardyp added a commit that referenced this issue Sep 28, 2017

Merge pull request #3645 from willlovett-arm/willlovett-arm-3460
Fix to support multiple gerrit codebases (issue #3460)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment