diff --git a/README.md b/README.md index b5b443d..c099cdd 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ effectively makes everybody else rebase again. `gerrit-queue` is meant to remove these races to master. Developers can set the `Autosubmit` customized label to `+1` on all changesets -in a series, and if all preconditions on are met ("submittable" in gerrit +in a chain, and if all preconditions on are met ("submittable" in gerrit speech, this usually means passing CI and passing Code Review), `gerrit-queue` takes care of rebasing and submitting it to master. @@ -44,58 +44,59 @@ but usually developers think in terms of multiple changesets. ### Fetching changesets `gerrit-queue` fetches all changesets from gerrit, and tries to identify these -chains of changesets. We call them `Series`. All changesets need to have strict -parent/child relationships to be detected (so if only half of the stack gets -rebased by the Gerrit Web interface, these are considered individual series. +chains of changesets. +All changesets need to have strict parent/child relationships to be detected. +(This means, if a user manually rebases half of a chain through the Gerrit Web +Interface, these will be considered as two independent chains!) -Series are sorted by the number of changesets in them. This ensures longer -series are merged faster, and less rebases are triggered. In the future, this -might be extended to other metrics. +Chains are rebased by the number of changesets in them. This ensures longer +chains are merged faster, and less rebases are triggered. In the future, this +might be extended to other strategies. ### Submitting changesets The submitqueue has a Trigger() function, which gets periodically executed. -It can keep a reference to one single serie across multiple runs. This is -necessary if it previously rebased one serie to current HEAD and needs to wait +It can keep a reference to one single chain across multiple runs. This is +necessary if it previously rebased one chain to current HEAD and needs to wait some time until CI feedback is there. If it wouldn't keep that state, it would -pick another series (with +1 from CI) and trigger a rebase on that one, so +pick another chain (with +1 from CI) and trigger a rebase on that one, so depending on CI run times and trigger intervals, if not keepig this information it'd end up rebasing all unrebased changesets on the same HEAD, and then just pick one, instead of waiting for the one to finish. The Trigger() function first instructs the gerrit client to fetch changesets -and assemble series. -If there is a `wipSerie` from a previous run, we check if it can still be found -in the newly assembled list of series (it still needs to contain the same -number of series. Commit IDs may differ, because the code doesn't reassemble a -`wipSerie` after scheduling a rebase. -If the `wipSerie` could be refreshed, we update the pointer with the newly -assembled series. If we couldn't find it, we drop it. +and assemble chains. +If there is a `wipChain` from a previous run, we check if it can still be found +in the newly assembled list of chains (it still needs to contain the same +number of changesets. Commit IDs may differ, because the code doesn't reassemble +a `wipChain` after scheduling a rebase. +If the `wipChain` could be refreshed, we update the pointer with the newly +assembled chain. If we couldn't find it, we drop it. Now, we enter the main for loop. The first half of the loop checks various -conditions of the current `wipSerie`, and if successful, does the submit -("Submit phase"), the second half will pick a suitable new `wipSerie`, and +conditions of the current `wipChain`, and if successful, does the submit +("Submit phase"), the second half will pick a suitable new `wipChain`, and potentially do a rebase ("Pick phase"). #### Submit phase -We check if there is an existing `wipSerie`. If there isn't, we immediately go to +We check if there is an existing `wipChain`. If there isn't, we immediately go to the "pick" phase. -The `wipSerie` still needs to be rebased on `HEAD` (otherwise, the submit queue +The `wipChain` still needs to be rebased on `HEAD` (otherwise, the submit queue advanced outside of gerrit), and should not fail CI (logical merge conflict) - otherwise we discard it, and continue with the picking phase. -If the `wipSerie` still contains a changeset awaiting CI feedback, we `return` +If the `wipChain` still contains a changeset awaiting CI feedback, we `return` from the `Trigger()` function (and go back to sleep). If the changeset is "submittable" in gerrit speech, and has the necessary submit queue tag set, we submit it. #### Pick phase -The pick phase finds a new `wipSerie`. It'll first try to find one that already +The pick phase finds a new `wipChain`. It'll first try to find one that already is rebased on the current `HEAD` (so the loop can just continue, and the next submit phase simply submit), and otherwise fall back to a not-yet-rebased -serie. Because the rebase mandates waiting for CI, the code `return`s the +chain. Because the rebase mandates waiting for CI, the code `return`s the `Trigger()` function, so it'll be called again after waiting some time. ## Compile and Run diff --git a/frontend/frontend.go b/frontend/frontend.go index 77145e3..9a5e021 100644 --- a/frontend/frontend.go +++ b/frontend/frontend.go @@ -19,8 +19,8 @@ import ( //go:embed templates var templates embed.FS -//loadTemplate loads a list of templates, relative to the templates root, and a -//FuncMap, and returns a template object +// loadTemplate loads a list of templates, relative to the templates root, and a +// FuncMap, and returns a template object func loadTemplate(templateNames []string, funcMap template.FuncMap) (*template.Template, error) { if len(templateNames) == 0 { return nil, fmt.Errorf("templateNames can't be empty") @@ -53,13 +53,13 @@ func MakeFrontend(rotatingLogHandler *misc.RotatingLogHandler, gerritClient *ger mux := http.NewServeMux() mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) { - var wipSerie *gerrit.Serie = nil + var wipChain *gerrit.Chain = nil HEAD := "" currentlyRunning := runner.IsCurrentlyRunning() // don't trigger operations requiring a lock if !currentlyRunning { - wipSerie = runner.GetWIPSerie() + wipChain = runner.GetWIPChain() HEAD = gerritClient.GetHEAD() } @@ -91,7 +91,7 @@ func MakeFrontend(rotatingLogHandler *misc.RotatingLogHandler, gerritClient *ger tmpl := template.Must(loadTemplate([]string{ "index.tmpl.html", - "serie.tmpl.html", + "chain.tmpl.html", "changeset.tmpl.html", }, funcMap)) @@ -102,7 +102,7 @@ func MakeFrontend(rotatingLogHandler *misc.RotatingLogHandler, gerritClient *ger // State "currentlyRunning": currentlyRunning, - "wipSerie": wipSerie, + "wipChain": wipChain, "HEAD": HEAD, // History diff --git a/frontend/templates/serie.tmpl.html b/frontend/templates/chain.tmpl.html similarity index 81% rename from frontend/templates/serie.tmpl.html rename to frontend/templates/chain.tmpl.html index 60f0c18..5d1248e 100644 --- a/frontend/templates/serie.tmpl.html +++ b/frontend/templates/chain.tmpl.html @@ -1,4 +1,4 @@ -{{ define "serie" }} +{{ define "chain" }} @@ -9,7 +9,7 @@ - + {{ range $changeset := .ChangeSets }} {{ block "changeset" $changeset }}{{ end }} diff --git a/frontend/templates/index.tmpl.html b/frontend/templates/index.tmpl.html index e04c0a3..f4fee1a 100644 --- a/frontend/templates/index.tmpl.html +++ b/frontend/templates/index.tmpl.html @@ -19,7 +19,7 @@ Info
  • - WIP Serie + WIP Chain
  • Log @@ -55,9 +55,9 @@

    Info

    •
    Serie with {{ len .ChangeSets }} changesChain with {{ len .ChangeSets }} changes
    -

    WIP Serie

    - {{ if .wipSerie }} - {{ block "serie" .wipSerie }}{{ end }} +

    WIP Chain

    + {{ if .wipChain }} + {{ block "chain" .wipChain }}{{ end }} {{ else }} - {{ end }} diff --git a/gerrit/serie.go b/gerrit/chain.go similarity index 63% rename from gerrit/serie.go rename to gerrit/chain.go index 788cf46..57abaea 100644 --- a/gerrit/serie.go +++ b/gerrit/chain.go @@ -7,34 +7,34 @@ import ( "github.com/apex/log" ) -// Serie represents a list of successive changesets with an unbroken parent -> child relation, +// Chain represents a list of successive changesets with an unbroken parent -> child relation, // starting from the parent. -type Serie struct { +type Chain struct { ChangeSets []*Changeset } // GetParentCommitIDs returns the parent commit IDs -func (s *Serie) GetParentCommitIDs() ([]string, error) { +func (s *Chain) GetParentCommitIDs() ([]string, error) { if len(s.ChangeSets) == 0 { - return nil, fmt.Errorf("Can't return parent on a serie with zero ChangeSets") + return nil, fmt.Errorf("can't return parent on a chain with zero ChangeSets") } return s.ChangeSets[0].ParentCommitIDs, nil } // GetLeafCommitID returns the commit id of the last commit in ChangeSets -func (s *Serie) GetLeafCommitID() (string, error) { +func (s *Chain) GetLeafCommitID() (string, error) { if len(s.ChangeSets) == 0 { - return "", fmt.Errorf("Can't return leaf on a serie with zero ChangeSets") + return "", fmt.Errorf("can't return leaf on a chain with zero ChangeSets") } return s.ChangeSets[len(s.ChangeSets)-1].CommitID, nil } -// CheckIntegrity checks that the series contains a properly ordered and connected chain of commits -func (s *Serie) CheckIntegrity() error { - logger := log.WithField("serie", s) - // an empty serie is invalid +// Validate checks that the chain contains a properly ordered and connected chain of commits +func (s *Chain) Validate() error { + logger := log.WithField("chain", s) + // an empty chain is invalid if len(s.ChangeSets) == 0 { - return fmt.Errorf("An empty serie is invalid") + return fmt.Errorf("an empty chain is invalid") } previousCommitID := "" @@ -48,12 +48,12 @@ func (s *Serie) CheckIntegrity() error { parentCommitIDs := changeset.ParentCommitIDs if len(parentCommitIDs) == 0 { - return fmt.Errorf("Changesets without any parent are not supported") + return fmt.Errorf("changesets without any parent are not supported") } - // we don't check parents of the first changeset in a series + // we don't check parents of the first changeset in a chain if i != 0 { if len(parentCommitIDs) != 1 { - return fmt.Errorf("Merge commits in the middle of a series are not supported (only at the beginning)") + return fmt.Errorf("merge commits in the middle of a chain are not supported (only at the beginning)") } if parentCommitIDs[0] != previousCommitID { return fmt.Errorf("changesets parent commit id doesn't match previous commit id") @@ -65,20 +65,20 @@ func (s *Serie) CheckIntegrity() error { return nil } -// FilterAllChangesets applies a filter function on all of the changesets in the series. +// AllChangesets applies a filter function on all of the changesets in the chain. // returns true if it returns true for all changesets, false otherwise -func (s *Serie) FilterAllChangesets(f func(c *Changeset) bool) bool { +func (s *Chain) AllChangesets(f func(c *Changeset) bool) bool { for _, changeset := range s.ChangeSets { - if f(changeset) == false { + if !f(changeset) { return false } } return true } -func (s *Serie) String() string { +func (s *Chain) String() string { var sb strings.Builder - sb.WriteString(fmt.Sprintf("Serie[%d]", len(s.ChangeSets))) + sb.WriteString(fmt.Sprintf("Chain[%d]", len(s.ChangeSets))) if len(s.ChangeSets) == 0 { sb.WriteString("()\n") return sb.String() @@ -106,7 +106,3 @@ func (s *Serie) String() string { s.ChangeSets[len(s.ChangeSets)-1].CommitID)) return sb.String() } - -func shortCommitID(commitID string) string { - return commitID[:6] -} diff --git a/gerrit/chains.go b/gerrit/chains.go new file mode 100644 index 0000000..d60d974 --- /dev/null +++ b/gerrit/chains.go @@ -0,0 +1,125 @@ +package gerrit + +import ( + "sort" + + "github.com/apex/log" +) + +// AssembleChain consumes a list of changesets, and groups them together to chains. +// +// Initially, every changeset is put in its own individual chain. +// +// we maintain a lookup table, mapLeafToChain, +// which allows to lookup a chain by its leaf commit id +// We concat chains in a fixpoint approach +// because both appending and prepending is much more complex. +// Concatenation moves changesets of the later changeset in the previous one +// in a cleanup phase, we remove orphaned chains (those without any changesets inside) +// afterwards, we do an integrity check, just to be on the safe side. +func AssembleChain(changesets []*Changeset, logger *log.Logger) ([]*Chain, error) { + chains := make([]*Chain, 0) + mapLeafToChain := make(map[string]*Chain, 0) + + for _, changeset := range changesets { + l := logger.WithField("changeset", changeset.String()) + + l.Debug("creating initial chain") + chain := &Chain{ + ChangeSets: []*Changeset{changeset}, + } + chains = append(chains, chain) + mapLeafToChain[changeset.CommitID] = chain + } + + // Combine chain using a fixpoint approach, with a max iteration count. + logger.Debug("glueing together phase") + for i := 1; i < 100; i++ { + didUpdate := false + logger.Debugf("at iteration %d", i) + for j, chain := range chains { + l := logger.WithFields(log.Fields{ + "i": i, + "j": j, + "chain": chain.String(), + }) + parentCommitIDs, err := chain.GetParentCommitIDs() + if err != nil { + return chains, err + } + if len(parentCommitIDs) != 1 { + // We can't append merge commits to other chains + l.Infof("No single parent, skipping.") + continue + } + parentCommitID := parentCommitIDs[0] + l.Debug("Looking for a predecessor.") + // if there's another chain that has this parent as a leaf, glue together + if otherChain, ok := mapLeafToChain[parentCommitID]; ok { + if otherChain == chain { + continue + } + l = l.WithField("otherChain", otherChain) + + myLeafCommitID, err := chain.GetLeafCommitID() + if err != nil { + return chains, err + } + + // append our changesets to the other chain + l.Debug("Splicing together.") + otherChain.ChangeSets = append(otherChain.ChangeSets, chain.ChangeSets...) + + delete(mapLeafToChain, parentCommitID) + mapLeafToChain[myLeafCommitID] = otherChain + + // orphan our chain + chain.ChangeSets = []*Changeset{} + // remove the orphaned chain from the lookup table + delete(mapLeafToChain, myLeafCommitID) + + didUpdate = true + } else { + l.Debug("Not found.") + } + } + chains = removeOrphanedChains(chains) + if !didUpdate { + logger.Infof("converged after %d iterations", i) + break + } + } + + // Check integrity, just to be on the safe side. + for _, chain := range chains { + l := logger.WithField("chain", chain.String()) + l.Debugf("checking integrity") + err := chain.Validate() + if err != nil { + l.Errorf("checking integrity failed: %s", err) + } + } + return chains, nil +} + +// removeOrphanedChains removes all empty chains (that contain zero changesets) +func removeOrphanedChains(chains []*Chain) []*Chain { + newChains := []*Chain{} + for _, chain := range chains { + if len(chain.ChangeSets) != 0 { + newChains = append(newChains, chain) + } + } + return newChains +} + +// SortChains sorts a list of chains by the number of changesets in each chain, descending +func SortChains(chains []*Chain) []*Chain { + newChains := make([]*Chain, len(chains)) + copy(newChains, chains) + sort.Slice(newChains, func(i, j int) bool { + // the weight depends on the amount of changesets in the chain + return len(chains[i].ChangeSets) > len(chains[j].ChangeSets) + }) + return newChains +} diff --git a/gerrit/changeset.go b/gerrit/changeset.go index 6ec0f41..3e30156 100644 --- a/gerrit/changeset.go +++ b/gerrit/changeset.go @@ -9,7 +9,6 @@ import ( ) // Changeset represents a single changeset -// Relationships between different changesets are described in Series type Changeset struct { changeInfo *goGerrit.ChangeInfo ChangeID string diff --git a/gerrit/client.go b/gerrit/client.go index 314f972..9bfd6f2 100644 --- a/gerrit/client.go +++ b/gerrit/client.go @@ -27,9 +27,9 @@ type IClient interface { SubmitChangeset(changeset *Changeset) (*Changeset, error) RebaseChangeset(changeset *Changeset, ref string) (*Changeset, error) ChangesetIsRebasedOnHEAD(changeset *Changeset) bool - SerieIsRebasedOnHEAD(serie *Serie) bool - FilterSeries(filter func(s *Serie) bool) []*Serie - FindSerie(filter func(s *Serie) bool) *Serie + ChainIsRebasedOnHEAD(chain *Chain) bool + FilterChains(filter func(s *Chain) bool) []*Chain + FindFirstChain(filter func(s *Chain) bool) *Chain } var _ IClient = &Client{} @@ -41,7 +41,7 @@ type Client struct { baseURL string projectName string branchName string - series []*Serie + chains []*Chain head string } @@ -96,13 +96,13 @@ func (c *Client) Refresh() error { return err } - c.logger.Infof("assembling series…") - series, err := AssembleSeries(changesets, c.logger) + c.logger.Infof("assembling chains") + chains, err := AssembleChain(changesets, c.logger) if err != nil { return err } - series = SortSeries(series) - c.series = series + chains = SortChains(chains) + c.chains = chains return nil } @@ -188,32 +188,32 @@ func (c *Client) ChangesetIsRebasedOnHEAD(changeset *Changeset) bool { return changeset.ParentCommitIDs[0] == c.head } -// SerieIsRebasedOnHEAD returns true if the whole series is rebased on the current HEAD -// this is already the case if the first changeset in the series is rebased on the current HEAD -func (c *Client) SerieIsRebasedOnHEAD(serie *Serie) bool { - // an empty serie should not exist - if len(serie.ChangeSets) == 0 { +// ChainIsRebasedOnHEAD returns true if the whole chain is rebased on the current HEAD +// this is already the case if the first changeset in the chain is rebased on the current HEAD +func (c *Client) ChainIsRebasedOnHEAD(chain *Chain) bool { + // an empty chain should not exist + if len(chain.ChangeSets) == 0 { return false } - return c.ChangesetIsRebasedOnHEAD(serie.ChangeSets[0]) + return c.ChangesetIsRebasedOnHEAD(chain.ChangeSets[0]) } -// FilterSeries returns a subset of all Series, passing the given filter function -func (c *Client) FilterSeries(filter func(s *Serie) bool) []*Serie { - matchedSeries := []*Serie{} - for _, serie := range c.series { - if filter(serie) { - matchedSeries = append(matchedSeries, serie) +// FilterChains returns a subset of all chains, passing the given filter function +func (c *Client) FilterChains(filter func(s *Chain) bool) []*Chain { + matchedChains := []*Chain{} + for _, chain := range c.chains { + if filter(chain) { + matchedChains = append(matchedChains, chain) } } - return matchedSeries + return matchedChains } -// FindSerie returns the first serie that matches the filter, or nil if none was found -func (c *Client) FindSerie(filter func(s *Serie) bool) *Serie { - for _, serie := range c.series { - if filter(serie) { - return serie +// FindFirstChain returns the first chain that matches the filter, or nil if none was found +func (c *Client) FindFirstChain(filter func(s *Chain) bool) *Chain { + for _, chain := range c.chains { + if filter(chain) { + return chain } } return nil diff --git a/gerrit/series.go b/gerrit/series.go deleted file mode 100644 index 295193e..0000000 --- a/gerrit/series.go +++ /dev/null @@ -1,126 +0,0 @@ -package gerrit - -import ( - "sort" - - "github.com/apex/log" -) - -// AssembleSeries consumes a list of `Changeset`, and groups them together to series -// -// We initially put every Changeset in its own Serie -// -// As we have no control over the order of the passed changesets, -// we maintain a lookup table, mapLeafToSerie, -// which allows to lookup a serie by its leaf commit id -// We concat series in a fixpoint approach -// because both appending and prepending is much more complex. -// Concatenation moves changesets of the later changeset in the previous one -// in a cleanup phase, we remove orphaned series (those without any changesets inside) -// afterwards, we do an integrity check, just to be on the safe side. -func AssembleSeries(changesets []*Changeset, logger *log.Logger) ([]*Serie, error) { - series := make([]*Serie, 0) - mapLeafToSerie := make(map[string]*Serie, 0) - - for _, changeset := range changesets { - l := logger.WithField("changeset", changeset.String()) - - l.Debug("creating initial serie") - serie := &Serie{ - ChangeSets: []*Changeset{changeset}, - } - series = append(series, serie) - mapLeafToSerie[changeset.CommitID] = serie - } - - // Combine series using a fixpoint approach, with a max iteration count. - logger.Debug("glueing together phase") - for i := 1; i < 100; i++ { - didUpdate := false - logger.Debugf("at iteration %d", i) - for j, serie := range series { - l := logger.WithFields(log.Fields{ - "i": i, - "j": j, - "serie": serie.String(), - }) - parentCommitIDs, err := serie.GetParentCommitIDs() - if err != nil { - return series, err - } - if len(parentCommitIDs) != 1 { - // We can't append merge commits to other series - l.Infof("No single parent, skipping.") - continue - } - parentCommitID := parentCommitIDs[0] - l.Debug("Looking for a predecessor.") - // if there's another serie that has this parent as a leaf, glue together - if otherSerie, ok := mapLeafToSerie[parentCommitID]; ok { - if otherSerie == serie { - continue - } - l = l.WithField("otherSerie", otherSerie) - - myLeafCommitID, err := serie.GetLeafCommitID() - if err != nil { - return series, err - } - - // append our changesets to the other serie - l.Debug("Splicing together.") - otherSerie.ChangeSets = append(otherSerie.ChangeSets, serie.ChangeSets...) - - delete(mapLeafToSerie, parentCommitID) - mapLeafToSerie[myLeafCommitID] = otherSerie - - // orphan our serie - serie.ChangeSets = []*Changeset{} - // remove the orphaned serie from the lookup table - delete(mapLeafToSerie, myLeafCommitID) - - didUpdate = true - } else { - l.Debug("Not found.") - } - } - series = removeOrphanedSeries(series) - if !didUpdate { - logger.Infof("converged after %d iterations", i) - break - } - } - - // Check integrity, just to be on the safe side. - for _, serie := range series { - l := logger.WithField("serie", serie.String()) - l.Debugf("checking integrity") - err := serie.CheckIntegrity() - if err != nil { - l.Errorf("checking integrity failed: %s", err) - } - } - return series, nil -} - -// removeOrphanedSeries removes all empty series (that contain zero changesets) -func removeOrphanedSeries(series []*Serie) []*Serie { - newSeries := []*Serie{} - for _, serie := range series { - if len(serie.ChangeSets) != 0 { - newSeries = append(newSeries, serie) - } - } - return newSeries -} - -// SortSeries sorts a list of series by the number of changesets in each serie, descending -func SortSeries(series []*Serie) []*Serie { - newSeries := make([]*Serie, len(series)) - copy(newSeries, series) - sort.Slice(newSeries, func(i, j int) bool { - // the weight depends on the amount of changesets series changeset size - return len(series[i].ChangeSets) > len(series[j].ChangeSets) - }) - return newSeries -} diff --git a/submitqueue/runner.go b/submitqueue/runner.go index 9fe234f..0cc6451 100644 --- a/submitqueue/runner.go +++ b/submitqueue/runner.go @@ -13,11 +13,11 @@ import ( // it contains a mutex to avoid being run multiple times. // In fact, it even cancels runs while another one is still in progress. // It contains a Gerrit object facilitating access, a log object, the configured submit queue tag -// and a `wipSerie` (only populated if waiting for a rebase) +// and a `wipChain` (only populated if waiting for a rebase) type Runner struct { mut sync.Mutex currentlyRunning bool - wipSerie *gerrit.Serie + wipChain *gerrit.Chain logger *log.Logger gerrit *gerrit.Client } @@ -32,12 +32,13 @@ func NewRunner(logger *log.Logger, gerrit *gerrit.Client) *Runner { // isAutoSubmittable determines if something could be autosubmitted, potentially requiring a rebase // for this, it needs to: -// * have the "Autosubmit" label set to +1 -// * have gerrit's 'submittable' field set to true -// it doesn't check if the series is rebased on HEAD -func (r *Runner) isAutoSubmittable(s *gerrit.Serie) bool { +// - have the "Autosubmit" label set to +1 +// - have gerrit's 'submittable' field set to true +// +// it doesn't check if the chain is rebased on HEAD +func (r *Runner) isAutoSubmittable(s *gerrit.Chain) bool { for _, c := range s.ChangeSets { - if c.Submittable != true || !c.IsAutosubmit() { + if !c.Submittable || !c.IsAutosubmit() { return false } } @@ -49,14 +50,14 @@ func (r *Runner) IsCurrentlyRunning() bool { return r.currentlyRunning } -// GetWIPSerie returns the current wipSerie, if any, nil otherwiese +// GetWIPChain returns the current wipChain, if any, nil otherwiese // Acquires a lock, so check with IsCurrentlyRunning first -func (r *Runner) GetWIPSerie() *gerrit.Serie { +func (r *Runner) GetWIPChain() *gerrit.Chain { r.mut.Lock() defer func() { r.mut.Unlock() }() - return r.wipSerie + return r.wipChain } // Trigger gets triggered periodically @@ -65,7 +66,7 @@ func (r *Runner) Trigger(fetchOnly bool) error { // Only one trigger can run at the same time r.mut.Lock() if r.currentlyRunning { - return fmt.Errorf("Already running, skipping") + return fmt.Errorf("already running, skipping") } r.currentlyRunning = true r.mut.Unlock() @@ -86,122 +87,122 @@ func (r *Runner) Trigger(fetchOnly bool) error { return nil } - if r.wipSerie != nil { - // refresh wipSerie with how it looks like in gerrit now - wipSerie := r.gerrit.FindSerie(func(s *gerrit.Serie) bool { - // the new wipSerie needs to have the same number of changesets - if len(r.wipSerie.ChangeSets) != len(s.ChangeSets) { + if r.wipChain != nil { + // refresh wipChain with how it looks like in gerrit now + wipChain := r.gerrit.FindFirstChain(func(s *gerrit.Chain) bool { + // the new wipChain needs to have the same number of changesets + if len(r.wipChain.ChangeSets) != len(s.ChangeSets) { return false } // … and the same ChangeIDs. for idx, c := range s.ChangeSets { - if r.wipSerie.ChangeSets[idx].ChangeID != c.ChangeID { + if r.wipChain.ChangeSets[idx].ChangeID != c.ChangeID { return false } } return true }) - if wipSerie == nil { - r.logger.WithField("wipSerie", r.wipSerie).Warn("wipSerie has disappeared") - r.wipSerie = nil + if wipChain == nil { + r.logger.WithField("wipChain", r.wipChain).Warn("wipChain has disappeared") + r.wipChain = nil } else { - r.wipSerie = wipSerie + r.wipChain = wipChain } } for { // initialize logger r.logger.Info("Running") - if r.wipSerie != nil { - // if we have a wipSerie - l := r.logger.WithField("wipSerie", r.wipSerie) - l.Info("Checking wipSerie") + if r.wipChain != nil { + // if we have a wipChain + l := r.logger.WithField("wipChain", r.wipChain) + l.Info("Checking wipChain") - // discard wipSerie not rebased on HEAD + // discard wipChain not rebased on HEAD // we rebase them at the end of the loop, so this means master advanced without going through the submit queue - if !r.gerrit.SerieIsRebasedOnHEAD(r.wipSerie) { - l.Warnf("HEAD has moved to %v while still waiting for wipSerie, discarding it", r.gerrit.GetHEAD()) - r.wipSerie = nil + if !r.gerrit.ChainIsRebasedOnHEAD(r.wipChain) { + l.Warnf("HEAD has moved to %v while still waiting for wipChain, discarding it", r.gerrit.GetHEAD()) + r.wipChain = nil continue } // we now need to check CI feedback: - // wipSerie might have failed CI in the meantime - for _, c := range r.wipSerie.ChangeSets { + // wipChain might have failed CI in the meantime + for _, c := range r.wipChain.ChangeSets { if c == nil { l.Error("BUG: changeset is nil") continue } if c.Verified < 0 { - l.WithField("failingChangeset", c).Warnf("wipSerie failed CI in the meantime, discarding.") - r.wipSerie = nil + l.WithField("failingChangeset", c).Warnf("wipChain failed CI in the meantime, discarding.") + r.wipChain = nil continue } } // it might still be waiting for CI - for _, c := range r.wipSerie.ChangeSets { + for _, c := range r.wipChain.ChangeSets { if c == nil { l.Error("BUG: changeset is nil") continue } if c.Verified == 0 { - l.WithField("pendingChangeset", c).Warnf("still waiting for CI feedback in wipSerie, going back to sleep.") + l.WithField("pendingChangeset", c).Warnf("still waiting for CI feedback in wipChain, going back to sleep.") // break the loop, take a look at it at the next trigger. return nil } } // it might be autosubmittable - if r.isAutoSubmittable(r.wipSerie) { - l.Infof("submitting wipSerie") + if r.isAutoSubmittable(r.wipChain) { + l.Infof("submitting wipChain") // if the WIP changeset is ready (auto submittable and rebased on HEAD), submit - for _, changeset := range r.wipSerie.ChangeSets { + for _, changeset := range r.wipChain.ChangeSets { _, err := r.gerrit.SubmitChangeset(changeset) if err != nil { l.WithField("changeset", changeset).Error("error submitting changeset") - r.wipSerie = nil + r.wipChain = nil return err } } - r.wipSerie = nil + r.wipChain = nil } else { - l.Error("BUG: wipSerie is not autosubmittable") - r.wipSerie = nil + l.Error("BUG: wipChain is not autosubmittable") + r.wipChain = nil } } - r.logger.Info("Looking for series ready to submit") - // Find serie, that: + r.logger.Info("Looking for chains ready to submit") + // Find chain, that: // * has the auto-submit label // * has +2 review // * has +1 CI // * is rebased on master - serie := r.gerrit.FindSerie(func(s *gerrit.Serie) bool { + chain := r.gerrit.FindFirstChain(func(s *gerrit.Chain) bool { return r.isAutoSubmittable(s) && s.ChangeSets[0].ParentCommitIDs[0] == r.gerrit.GetHEAD() }) - if serie != nil { - r.logger.WithField("serie", serie).Info("Found serie to submit without necessary rebase") - r.wipSerie = serie + if chain != nil { + r.logger.WithField("chain", chain).Info("Found chain to submit without necessary rebase") + r.wipChain = chain continue } - // Find serie, that: + // Find chain, that: // * has the auto-submit label // * has +2 review // * has +1 CI // * is NOT rebased on master - serie = r.gerrit.FindSerie(r.isAutoSubmittable) - if serie == nil { - r.logger.Info("no more submittable series found, going back to sleep.") + chain = r.gerrit.FindFirstChain(r.isAutoSubmittable) + if chain == nil { + r.logger.Info("no more submittable chain found, going back to sleep.") break } - l := r.logger.WithField("serie", serie) - l.Info("found serie, which needs a rebase") - // TODO: move into Client.RebaseSeries function + l := r.logger.WithField("chain", chain) + l.Info("found chain, which needs a rebase") + // TODO: move into Client.RebaseChangeset function head := r.gerrit.GetHEAD() - for _, changeset := range serie.ChangeSets { + for _, changeset := range chain.ChangeSets { changeset, err := r.gerrit.RebaseChangeset(changeset, head) if err != nil { l.Error(err.Error()) @@ -211,7 +212,7 @@ func (r *Runner) Trigger(fetchOnly bool) error { } // we don't need to care about updating the rebased changesets or getting the updated HEAD, // as we'll refetch it on the beginning of the next trigger anyways - r.wipSerie = serie + r.wipChain = chain break }