#2485 closed defect (fixed)
The New Git Source Step Does Not Handle Changes From GerritChangeSource Correctly
Reported by: | jroo | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | 0.8.+ |
Version: | 0.8.7p1 | Keywords: | gerrit git master-side-source-steps |
Cc: |
Description
The implementation from buildbot.steps.sources.Git handles review changes from GerritChangeSource? correctly but the new implementation from buildbot.steps.sources.git.Git does not.
The following bits should be in master.cfg for reproducing the issue:
from buildbot.changes.gerritchangesource import GerritChangeSource c[ 'change_source' ] = [ GerritChangeSource( gerritserver = 'gerrit.example.com', gerritport = 29418, username = 'gerrit-user', ), ] # .... c['schedulers'] = [ AnyBranchScheduler( name = 'all', change_filter = filter.ChangeFilter( project = 'MyProject' ), treeStableTimer = None, builderNames = [ 'Some Builder', ], ), ] # ... factory = BuildFactory() factory.addStep( Git( repourl = GitRepoUrl, mode = 'update', submodules = True, ) ) c[ 'builders' ] = [ BuilderConfig( name = 'Some Builder', slavenames = [ 'Slave', ], mergeRequests = False, factory = factory, ), ]
Change History (9)
comment:1 Changed 8 years ago by dustin
- Keywords master-side-source-steps added
- Milestone changed from undecided to 0.8.+
comment:2 Changed 8 years ago by jroo
In my use case it does not matter if Git and Gerrit source steps are implemented in different classes. For some it might; someone could have GitPoller? and GerritChangeSource? objects as change source and if the source fetch steps would be in different classes the build steps would be required to be different depending on the source of the change.
comment:3 Changed 8 years ago by dustin
Right, my point is just that you wouldn't expect a GitPoller and an SVN step to work together, so I'm not sure why GerritChangeSource and Git should work together, rather than GerritChangeSource and Repo, or a new Gerrit class (that may be a subclass of Git). I'm really just looking for the most appropriate design.
comment:4 Changed 8 years ago by jroo
But Gerrit implements Git repository so you might expect Git and Gerrit to work together.
In my opinion it would be good solution to just inherit the gerrit step from git step if not implement both of those in single class; after all Gerrit is a Git repository + something else.
comment:5 Changed 8 years ago by dustin
Fair enough. I'd prefer to see this as a subclass, when it gets implemented.
comment:6 Changed 7 years ago by srinup
- Resolution set to fixed
- Status changed from new to closed
comment:7 Changed 7 years ago by cbessette
Using https://raw.github.com/buildbot/buildbot/5a2b8ee0b559712d6413100aeee49a42ec5cd0cf/master/buildbot/steps/source/gerrit.py with Buildbot 0.8.7p1 worked for me.
comment:8 Changed 7 years ago by jroo
I tested the new gerrit step and it seems to work fine for updates. But if the build slave clones (i.e. when the build slave runs for the first time) and the change is due to new review the slave does not get the correct commit for building.
The following git clone is run: git clone --branch refs/changes/60/10760/2 <my-git-repo-url> .
And the following warning is issued by git: warning: Remote branch refs/changes/60/10760/2 not found in upstream origin, using HEAD instead
This is because the refs/changes/xx/yyy is not a branch and thus does not work for clone command.
comment:9 Changed 7 years ago by dustin
That sounds like #2548
There's some code to handle gerrit-specific properties in oldsource.py
I'm not sure what the advantage is of supporting this in the Git step rather than the Gerrit step.