Fixed
Status Update
Comments
zi...@gmail.com <zi...@gmail.com> #2
Comment has been deleted.
dm...@google.com <dm...@google.com> #3
Comment has been deleted.
zi...@gmail.com <zi...@gmail.com> #4
Comment has been deleted.
zi...@gmail.com <zi...@gmail.com> #5
Btw, I couldn't reproduce the issue using the steps 1-5 from the Description section of this issue.
For me the step 5 succeeded.
Used Gerrit 3.7 for testing.
EDIT: I missed this part from the step 3 "and then add at least one additional commit on top of 'a'". With that additional commit the issue is reproducible.
For me the step 5 succeeded.
Used Gerrit 3.7 for testing.
EDIT: I missed this part from the step 3 "and then add at least one additional commit on top of 'a'". With that additional commit the issue is reproducible.
zi...@gmail.com <zi...@gmail.com> #6
> The step 5 fails, because the alreadyAccepted list contains a tip of the 'test' branch (https://gerrit-review.git.corp.google.com/c/gerrit/+/366462/5/java/com/google/gerrit/server/submit/SubmitStrategy.java ). As a result, the RebaseSorter marks the tip as uninteresting and the sort method returns nothing.
The `alreadyAccepted` is calculated in the `MergeOp.getAlreadyAccepted` method. From this change [1] the `getAlreadyAccepted` method has a side-effect:
it excludes the commits to be merged (submitted) from the `alreadyAccepted` set.
So the `getAlreadyAccepted` is actually not what it claims to be and the `RebaseSorter` is a victim.
This is why this issue doesn't occur when a commit which is being submitted is contained in a branch by being the tip of the branch and why it does occur when the commit is contained in the branch but is not the branch tip.
[1]https://gerrit-review.googlesource.com/c/gerrit/+/77572/16/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java#575
The `alreadyAccepted` is calculated in the `MergeOp.getAlreadyAccepted` method. From this change [1] the `getAlreadyAccepted` method has a side-effect:
it excludes the commits to be merged (submitted) from the `alreadyAccepted` set.
So the `getAlreadyAccepted` is actually not what it claims to be and the `RebaseSorter` is a victim.
This is why this issue doesn't occur when a commit which is being submitted is contained in a branch by being the tip of the branch and why it does occur when the commit is contained in the branch but is not the branch tip.
[1]
sa...@gmail.com <sa...@gmail.com> #7
Comment has been deleted.
zi...@gmail.com <zi...@gmail.com> #8
ly...@gmail.com <ly...@gmail.com> #9
This is a top issue for us. It is super annoying. The fix would be much appreciated.
zi...@gmail.com <zi...@gmail.com> #10
We found additional use-cases where the fix in the https://gerrit-review.googlesource.com/c/gerrit/+/378543 doesn't work.
For now we propose to revert the original change [1] which caused the issue. The revert change is here [2].
With the revert [2] we loose the performance improvements done in [1].
After my vacation I am going to again look at the original (performance) issue [3] and hopefully provide a fix that doesn't break other scenarios.
As described in [4] we need to provide full test coverage for the RebaseSorter as it currently has no tests and the behaviour is unclear.
[1]https://gerrit-review.googlesource.com/c/gerrit/+/366462
[2]https://gerrit-review.googlesource.com/c/gerrit/+/381714
[3]https://issues.gerritcodereview.com/issues/40015446
[4]https://gerrit-review.googlesource.com/c/gerrit/+/378543
For now we propose to revert the original change [1] which caused the issue. The revert change is here [2].
With the revert [2] we loose the performance improvements done in [1].
After my vacation I am going to again look at the original (performance) issue [3] and hopefully provide a fix that doesn't break other scenarios.
As described in [4] we need to provide full test coverage for the RebaseSorter as it currently has no tests and the behaviour is unclear.
[1]
[2]
[3]
[4]
ap...@google.com <ap...@google.com> #11
Project: gerrit
Branch: stable-3.6
commit c566c0391eef1acb34e78ad272506fbcf2a87980
Author: Thomas Dräbing <thomas.draebing@sap.com>
Date: Thu Aug 03 15:36:44 2023
Revert "Set uninteresting branches based on project configuration"
This reverts commit 4f12dcc3748e195a8bd788bc382fb3deb70c4e71.
Reason for revert:
While this change fixed the performance issues, it didn't take into
account several cases in which the commit that is to be submitted
is already present in another branch, e.g. if the change was uploaded
first and then the commit was directly pushed to a branch.
Further, using the CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET option
was wrong, since it should only affect the behaviour during pushing
a change and not during submitting one.
While reverting this change will again add performance issues to projects with a lot of (outdated) branches that uses Rebase If
Necessary, this performance issue went unnoticed for years and
was only reported by a single project. Further, the real fix will
not be based on this fix. Thus, to enable all use cases again, this
change will be reverted for now. The final fix will follow later.
Release-Notes: Fix for: change can't be submitted if another branch contains exactly the same commit.
Bug: Issue 289505276
Change-Id: I4a4367c930fff14bba01df47aca31162ebc14287
M java/com/google/gerrit/server/submit/RebaseSorter.java
M java/com/google/gerrit/server/submit/SubmitStrategy.java
https://gerrit-review.googlesource.com/381716
Branch: stable-3.6
commit c566c0391eef1acb34e78ad272506fbcf2a87980
Author: Thomas Dräbing <thomas.draebing@sap.com>
Date: Thu Aug 03 15:36:44 2023
Revert "Set uninteresting branches based on project configuration"
This reverts commit 4f12dcc3748e195a8bd788bc382fb3deb70c4e71.
Reason for revert:
While this change fixed the performance issues, it didn't take into
account several cases in which the commit that is to be submitted
is already present in another branch, e.g. if the change was uploaded
first and then the commit was directly pushed to a branch.
Further, using the CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET option
was wrong, since it should only affect the behaviour during pushing
a change and not during submitting one.
While reverting this change will again add performance issues to projects with a lot of (outdated) branches that uses Rebase If
Necessary, this performance issue went unnoticed for years and
was only reported by a single project. Further, the real fix will
not be based on this fix. Thus, to enable all use cases again, this
change will be reverted for now. The final fix will follow later.
Release-Notes: Fix for: change can't be submitted if another branch contains exactly the same commit.
Bug:
Change-Id: I4a4367c930fff14bba01df47aca31162ebc14287
M java/com/google/gerrit/server/submit/RebaseSorter.java
M java/com/google/gerrit/server/submit/SubmitStrategy.java
Description
The
The submit is blocked if a change's commit is already present in any other branch (but not the tip of the branch)
Steps to reproduce (note - steps in tests above are different, but the final state is the same - a change's commit is present in another branch):
1) Ensure that repo has the following settings: CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET = false, Submit Strategy = Rebase Always or Rebase if necessary.
2) Create a change in gerrit. e.g. ChangeId: I123, sha='a', target branch = main.
3) Create a new branch 'test' locally which points to the sha='a' and then add at least one
additional commit on top of 'a'.
4) Make a direct push (i.e. push directly to refs/heads/test, without code review).
5) Try to submit the change from the step 2. It fails.
The step 5 fails, because the alreadyAccepted list contains a tip of the 'test' branch (