False positives from (PG) git cl upload about uploading from old patchsets |
||||
Issue descriptionVersion: git --version : 2.9.2.windows.1 git cl --version : 2.0 OS: Windows 10 What steps will reproduce the problem? I can't say for certain, but it's happening regularly. This CL is an example (https://skia-review.googlesource.com/c/4438/). I've done all that work on one machine. Last night, I made some edits to the branch, and did a git cl upload of PS 16 ("Remove unused stuff"). This morning I made an additional edit (no rebase or squash or any other git commands in between), then uploaded with that one additional commit (PS 17, "Fix switch case"). The script warned me: > The last upload made from this repository was patchset #15 but the most recent > patchset on the server is #16. Uploading will still work, but if you've > uploaded to this issue from another machine or branch the patch you're > uploading now might not include those changes. > About to upload; enter to confirm.
,
Nov 11 2016
I think SetPatchset is only called in _GerritChangelistImpl from CMDPatchWithParsedIssue. Brian, did you run 'git cl patch' at some point on this branch?
,
Nov 11 2016
Most likely you've edited the description online, haven't you? If so, I think I've already filed a bug about it, but I can't find atm, so let's keep this one. The solution i see is for git cl to check revision history of the CL and see the missing patchsets were all made using UI's edit description.
,
Nov 11 2016
i take it back - you've not modified description using UI. And rmistry@ analysis is correct. I think I forgot set latest patchset on upload from Gerrit.
,
Nov 11 2016
Oh, in fact, on upload it's not clear what patchset number was created. So, maybe we shouldn't store patchset at all even if one patches the issue?
,
Nov 11 2016
I believe the problem is that the patchset number is stored in git config only during 'git cl patch'. If you do not use 'git cl patch' and only use 'git cl upload' then 'git config branch.${branch_name}.gerritpatchset' value is always None because it is never set. The patchset comparison is skipped if it is None which is why everyone does not see this.
The solution is to SetPatchset from CMDUpload for Gerrit.
,
Nov 11 2016
Can we just do self.SetPatchset(self.GetMostRecentPatchset()) from CMDUPload?
,
Nov 11 2016
> self.SetPatchset(self.GetMostRecentPatchset()) should be a good enough fix, but it adds extra latency on upload
,
Nov 11 2016
This all sounds reasonable. I have done git cl patch (specifically, 'git cl patch --reapply' to squash things).
,
Nov 11 2016
This was pretty easy, so I just did it. PTAL: https://chromium-review.googlesource.com/410270
,
Nov 11 2016
See discussion in https://chromium-review.googlesource.com/c/399118/ It might be best for now to disable setting patchset during 'git cl patch' instead to avoid unnecessary prompts for users when just the description changes. Once there is logic to check that only the description changed we can re-enable it.
,
Nov 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/fda50ca02dfd8edea5abd0b9019a87793b80db3e commit fda50ca02dfd8edea5abd0b9019a87793b80db3e Author: Ravi Mistry <rmistry@google.com> Date: Mon Nov 14 15:19:18 2016 Skip checking local vs remote patchset for Gerrit BUG= chromium:664520 Change-Id: Idd5855e14f6237577b351ff51582335996522417 Reviewed-on: https://chromium-review.googlesource.com/399118 Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org> [modify] https://crrev.com/fda50ca02dfd8edea5abd0b9019a87793b80db3e/git_cl.py [modify] https://crrev.com/fda50ca02dfd8edea5abd0b9019a87793b80db3e/tests/git_cl_test.py
,
Nov 14 2016
Disabled checking local vs remote patchsets for Gerrit CLs in https://chromium-review.googlesource.com/399118. Marking this as fixed. Please reopen if you run into problems after syncing depot_tools. |
||||
►
Sign in to add a comment |
||||
Comment 1 by bsalo...@google.com
, Nov 11 2016