Give 'gclient revert' a --merge option to match sync

Give 'gclient revert' a --merge option to match sync

This is a workaround for the fact that 'gclient revert' calls
'gclient update' with the same `options` object, despite not
actually having the same set of command line options.

R=iannucci@chromium.org, kevcheng@chromium.org
BUG=631220
Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/c903d73f3cf002e7004d0bd51d130c5bf48f3fbc

[agable, 2016-07-25 23:28:16]

Robbie, PTAL ASAP, this is for a Pri-1 trooper bug.

Other options include:
* Revert https://codereview.chromium.org/61823002, which introduced the
references to options.merge. But that's really old, and probably relied upon by
developers somewhere.
* Make it so that gclient revert doesn't call gclient sync/update. Unfortunately
I tried this a few weeks ago, and it got reverted (see the diff between
patchsets 0 and 1 of https://codereview.chromium.org/2181743003, where the fix
for the revert is literally the cause of the current issue here)
* Something else entirely

[dnj, 2016-07-25 23:33:45]

dnj@chromium.org changed reviewers:
+ dnj@chromium.org

[dnj, 2016-07-25 23:33:46]

Could we make it so "gclient revert" stripped out options that "gclient update"
doesn't understand?

[agable, 2016-07-25 23:48:11]

On 2016/07/25 at 23:33:46, dnj wrote:
> Could we make it so "gclient revert" stripped out options that "gclient
update" doesn't understand?

It's the other way around -- gclient update is expecting an option to be
present, which gclient revert doesn't provide.

[dnj, 2016-07-25 23:53:08]

On 2016/07/25 23:48:11, agable wrote:
> On 2016/07/25 at 23:33:46, dnj wrote:
> > Could we make it so "gclient revert" stripped out options that "gclient
> update" doesn't understand?
> 
> It's the other way around -- gclient update is expecting an option to be
> present, which gclient revert doesn't provide.

Oh the description made it sound like revert -> update. Either way.

[agable, 2016-07-26 00:19:09]

On 2016/07/25 at 23:53:08, dnj wrote:
> On 2016/07/25 23:48:11, agable wrote:
> > On 2016/07/25 at 23:33:46, dnj wrote:
> > > Could we make it so "gclient revert" stripped out options that "gclient
> > update" doesn't understand?
> > 
> > It's the other way around -- gclient update is expecting an option to be
> > present, which gclient revert doesn't provide.
> 
> Oh the description made it sound like revert -> update. Either way.

Either way what? There are no options to be stripped out here -- the problem is
that there *aren't enough* options on the options object. That's why this CL is
adding one more, despite how stupid it is.

[dnj, 2016-07-26 00:31:34]

On 2016/07/26 00:19:09, agable wrote:
> On 2016/07/25 at 23:53:08, dnj wrote:
> > On 2016/07/25 23:48:11, agable wrote:
> > > On 2016/07/25 at 23:33:46, dnj wrote:
> > > > Could we make it so "gclient revert" stripped out options that "gclient
> > > update" doesn't understand?
> > > 
> > > It's the other way around -- gclient update is expecting an option to be
> > > present, which gclient revert doesn't provide.
> > 
> > Oh the description made it sound like revert -> update. Either way.
> 
> Either way what? There are no options to be stripped out here -- the problem
is
> that there *aren't enough* options on the options object. That's why this CL
is
> adding one more, despite how stupid it is.

Oh I read it as the flag was being forwarded from "update" to "revert", so I was
thinking, "just don't forward it." You're saying that the object has the
expectation of having a "merge" parameter.

Rather than add it as an actual command-line flag, WDYT about putting (in
"revert"):
setattr(options, 'merge', False)

?

[agable, 2016-07-26 00:43:17]

Good call, done. Even simpler than that, in fact.

[dnj, 2016-07-26 00:47:24]

Is consistent w/ other params, so lgtm.

I'm not depot_tools owner though, so get an rs.

[iannucci, 2016-07-26 00:58:28]

lgtm

[agable, 2016-07-26 16:04:24]

The CQ bit was checked by agable@chromium.org

[commit-bot: I haz the power, 2016-07-26 16:04:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 16:07:27]

Description was changed from

==========
Give 'gclient revert' a --merge option to match sync

This is a workaround for the fact that 'gclient revert' calls
'gclient update' with the same `options` object, despite not
actually having the same set of command line options.

R=iannucci@chromium.org, kevcheng@chromium.org
BUG=631220
==========

to

==========
Give 'gclient revert' a --merge option to match sync

This is a workaround for the fact that 'gclient revert' calls
'gclient update' with the same `options` object, despite not
actually having the same set of command line options.

R=iannucci@chromium.org, kevcheng@chromium.org
BUG=631220

Committed:
https://chromium.googlesource.com/chromium/tools/depot_tools/+/c903d73f3cf002...
==========

[commit-bot: I haz the power, 2016-07-26 16:07:28]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/tools/depot_tools/+/c903d73f3cf002...