Bugzilla – Full Text Bug Listing
| Summary: | [Gerrit] Configure Gerrit to "Pick" or "Rebase" if necessary | ||
|---|---|---|---|
| Product: | Community | Reporter: | Camille Letavernier <cletavernier> |
| Component: | Cross-Project | Assignee: | Eclipse Webmaster <webmaster> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | david_williams, marc.khouzam, matthias.sohn |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
Camille Letavernier
I too think "rebase if necessary" is best ... from what I've heard on cross-project list. Webmasters, is this something you can do for us? Or ... something "we" should know how to do ourselves? Thanks, (In reply to Camille Letavernier from comment #0) > When following the Gerrit review process, due to the "Fast-forward only" > policy of the Simrel repository, it may happen that we need to rebase & > reverify several times a commit before it can be submitted to the > repository. Currently, the only way around this is to bypass the Gerrit > review > > Any commit pushed to the repository before the Gerrit validation is complete > will force a contributor to rebase its commits & revalidate it. And since > the validation takes 15 minutes, the chances are quite high that a new > commit will be pushed in the meantime, forcing yet another rebase & > revalidate. > > Such issues have been discussed on the mailing list, e.g.: > https://dev.eclipse.org/mhonarc/lists/cross-project-issues-dev/msg11942.html, > https://dev.eclipse.org/mhonarc/lists/cross-project-issues-dev/msg12022.html > > This seems especially confusing for people that are not used to Git/Gerrit. > Others might be less reluctant to manually validate the commit or simply > bypass Gerrit, but then it might encourage a bad practice. > > Several suggestions arised during these discussions: > > - Cherry pick / Cherry pick if necessary > - Rebase if necessary (Similar to pick, but maintains dependencies between > commits when needed) > - Merge if necessary (But that breaks the "fast-forward only" policy of > Simrel) "fast-forward only" means that Gerrit will fail submit if the change to be submitted isn't a successor of the current branch tip. This means all the 3 options cherry-pick if necessary, rebase if necessary, merge if necessary will break this policy. > 'Rebase if necessary' seems to be the best option (Since that's what a > contributor would manually do anyway) (In reply to Matthias Sohn from comment #2) > "fast-forward only" means that Gerrit will fail submit if the change to be > submitted > isn't a successor of the current branch tip. This means all the 3 options > cherry-pick if necessary, rebase if necessary, merge if necessary will break > this > policy. Not sure what you are saying, Matthias. Are you complaining of making any change? Or, saying there is something better? Or, saying there is no difference? For clarity, I'll see if I can state the problem we are trying to solve -- all comes down to making things easier for committers. But, there may be others that can state the case better than me. The problem is that several commits go into Gerrit based on that same 'top' of master, each take a while to validate. As they do, they are committed to master, non-automatically (of course :) ... so, eventually, one of the commits that came in later rather than earlier, is no longer based based on the current tip of master. It's desired to "make things right" automatically, because 99% of the time (for our "sim. rel. use-case") people are working with different files. (The one percent is when someone has to make a change to their file, and the 'simrel.b3aggr' file at the same time. I hope I've captured the essence of the problem, and will look forward to Matthias' recommendation ... and anyone else, who has one. Thanks, I may have misused the "fast-forward" term My understanding is that the Simrel repository's convention is to used Rebase rather than Merge, and that Gerrit is bound to that convention too, so when Gerrit tries to push a merge commit, it is rejected (While a rebase commit would be allowed). But it might actually be that it really accepts "fast-forward" only, i.e. neither merge nor rebase; only 'exact match'. Then using 'Rebase/Pick if necessary' instead of 'fast-forward only' would work (While 'merge if necessary' may also work, it would break the simrel repo convention) Now, I'm only a user of Git/Gerrit, so I don't know where all these strategies/constraints are configured/enforced, so please excuse my lack of accuracy in the terms I use :) I hope I could clarify what I meant The advantage of "fast-forward only" is that it ensures that only the exact commit which was validated by Hudson can become the new tip of the branch. The downside is frequent races when multiple concurrent changes for different projects compete to be submitted. Hence I think we should change policy and use one out of (1) cherry-pick if necessary (2) rebase if necessary (3) merge if necessary All these options no longer can guarantee that the exact commit which was validated will become the new tip of the branch. So in rare cases this may lead to a new tip of the branch breaking the build. Though to my experience this happens only very rarely in practice. In the case of the simrel repository this is even less likely since typically most changes only affect a single file/project. (1) has the disadvantage that it breaks dependency chains, so if there are 2 commits in review depending on each other A <-- change-2 | B <-- change-1 | C <-- master | ... and you submit change-2 it will be cherry-picked on top of master which breaks the earlier dependency on change-1 which might be surprising (2) has the advantage that it ensures that the resulting commit graph will be linearized, the downside is that it has to rewrite multiple commits in case of dependency chains as the one described above. (3) will introduce a merge commit if necessary, hence the history of a single branch isn't linearized anymore (reflecting what really happened). So I'd say if you want linear history choose (2) if you want to see what really happened choose (3). (In reply to Matthias Sohn from comment #5) > Hence I think we should change policy and use one out of > > (1) cherry-pick if necessary > (2) rebase if necessary > (3) merge if necessary > > [...] > So I'd say if you want linear history choose (2) if you want to see what > really happened choose (3). I'm convinced. "merge if necessary" sounds best. We had the convention of "fast forward only" way back in the good 'ol days that people would commit directly to Git ... and, there, I think it makes a little more sense? .. but, honestly, we mostly set that policy "because that's the way we always do it". I do not think there is any particular reason to stick with that. And in this case, where "someone else" does the actual/final commit, it seems the 'merge' would show a more accurate history, and in the rare case of having to revert something, might make it easier to find the correct place to revert to. +1 Ok I've updated simrel/org.eclipse.simrel.build simrel/org.eclipse.simrel.tests simrel/org.eclipse.simrel.tools to be 'merge if necessary' . -M. |