New issue
Advanced search Search tips

Issue 649846 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

git cl upload ignores args to `git diff`

Project Member Reported by lgar...@chromium.org, Sep 23 2016

Issue description

Version: depot_tools@15ab08227b961ddce83db7e41fb83b9288ff9ccc
OS: OSX 10.11.6

What steps will reproduce the problem?
(1) Create `branch1` with one commit.
(2) Branch `branch2` from `branch1` and add one more commit.
(3) Run `git cl upload HEAD~` or `git cl upload branch2`.

What is the expected output?
`git cl upload` only uploads the latest commit.

What do you see instead?
`git cl upload` ignores the arg and uploads both commits.

It took me a long time to settle on a workflow that doesn't make me want to tear my hair out, and my workflow involves explicitly specifying the base commit every time, so I would *really* like this behaviour, at least before we switch all of Chromium to Gerrit. :-)

At the very least, it would be nice to throw an error (or an "are you sure" prompt) for now if the behaviour of an upload will diverge from `git cl upload`. Otherwise, I'm almost guaranteed to collapse dependent CLs unintentionally, and the same will likely happen to some others.

tandrii@, agable@ suggested assigning to you.
 
Components: -Tools Infra>Codereview>Gerrit Infra>SDK
Status: Started (was: Assigned)
Labels: Milestone-Fishfood Proj-Gerrit-Migration
Status: Assigned (was: Started)
I think Gerrit git cl doesn't set properly parent commit. I never thought of this case.
clarification: parent commit means the parent in a synthesized squashed commit that is actually pushed to Gerrit. Also, likely the squashed commit itself is also from the wrong range, but the cause is the same.

Comment 4 by aga...@chromium.org, Oct 11 2016

CMDUpload passes 'git_diff_args' into CMDUploadChange.
_RietveldChangelistImpl passes those args into upload.py, which uses them to generate the actual diff that gets uploaded to Rietveld.
_GerritChangelistImpl however only uses those args to generate the CL message.

It needs a codepath which respects the git diff args, if they exist, instead of inferring from the branch tracking layout.

Comment 5 by aga...@chromium.org, Oct 11 2016

Cc: aga...@chromium.org
Labels: -Milestone-Fishfood Milestone-Launch
Status: Started (was: Assigned)
https://chromium-review.googlesource.com/c/475229/ works for me locally.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/550e924d20906bc105215c617ca910e6859f569d

commit 550e924d20906bc105215c617ca910e6859f569d
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Tue Apr 18 15:03:24 2017

Gerrit git cl upload <diff_base>: respect diff_base argumnent.

This CL propagates <diff_base> all the way to become parent commit
of the syntentic commit generated by squashing the current branch.

BUG= 649846 
R=agable@chromium.org

Change-Id: Ided7ebbb5c3a1114cac18adb62b3a9c27610018c
Reviewed-on: https://chromium-review.googlesource.com/475229
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/550e924d20906bc105215c617ca910e6859f569d/tests/git_cl_test.py
[modify] https://crrev.com/550e924d20906bc105215c617ca910e6859f569d/git_cl.py

Status: Fixed (was: Started)
This should be done now. If there are problems, please re-open or file a new bug.

Sign in to add a comment