1345554 - mozreview confused about changeset evolution

Status: RESOLVED INVALID

(MozReview Graveyard :: Integration: Mercurial, enhancement)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

https://reviewboard.mozilla.org/r/117676/

I originally had two commits, and used fbhistedit's "stop" to stop after the first, intending to add a new one:

(sandbox) dustin@lamport ~/p/m-c (bug1337903) $ hg histedit

  1 s 11598ee461ed 397216 Bug 1337903: handle platforms for source-test distin...
  2 pick 61e44d0de635 397217 Bug 1337903: add, and use, support for OS X in run-task

I later committed:

(sandbox) dustin@lamport ~/p/m-c (bug1337903|HISTEDIT) $ hg commit -m "Bug 1337903: add and use a job_try_name attribute; r?ahal"

and then updated the message:

(sandbox) dustin@lamport ~/p/m-c (bug1337903|HISTEDIT) $ hg commit --amend

  1 Bug 1337903: add and use a job_try_name attribute; r?ahal
  2 
  3 This fixes the ability to run mozbase via `-j mozbase`, with the
  4 added advantage that it will obey `-p` too.
  5 
  6 MozReview-Commit-ID: 1zkitUephXk

yet somehow mozreview thinks that this new commit is an evolution of the original (largely unchanged) first commit.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

BTW, if this is reproducible then it's a pretty good way to get around the need for r+ on a patch.

Comment 2

[Andrew Halberstadt [:ahal]]

Yes, it's definitely reproducible, I've noticed it too.

It looks review status is tied to the sequence number of the commit (i.e 1st commit, 2nd commit, etc) instead of the mozreview commit id. E.g, if you push two commits to review, get an r+ on the 1st one, then swap the order of the commits and repush, the r+ will still show up on the 1st commit.

Of course fixing that wouldn't help bypassing a malicious actor getting around r+ as they could easily also swap mozreview commit ids.. But my understanding is that the "no more r+ with nits" policy being implemented will make this aspect of the problem go away.

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

My understanding from irc is that the mozreview commit id "headers" are ignored, and are only useful for git where there's no changeset evolution history.  But, you're right - someone could easily modify that changset evolution history.

Comment 4

[BMO Automation]

MozReview is now obsolete. Please use Phabricator instead. Closing this bug.