1270346 - "hg push review" should work even if there are uncommit changes

Status: RESOLVED INVALID

(MozReview Graveyard :: Integration: Mercurial, defect)

Description

[Xidorn Quan [:xidorn] UTC+10]

If I run
> hg push -r xxx..xxx review
when I have uncommit changes, it reports:
> pushing to ssh://reviewboard-hg.mozilla.org/gecko
> abort: uncommitted changes

But I don't understand why pushing requires a clean working directory? It seems to me normal push doesn't require that, and pushing to MozReview shouldn't have the requirement either.

Comment 1

[Steven MacLeod [:smacleod]]

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #0)
> If I run
> > hg push -r xxx..xxx review
> when I have uncommit changes, it reports:
> > pushing to ssh://reviewboard-hg.mozilla.org/gecko
> > abort: uncommitted changes
> 
> But I don't understand why pushing requires a clean working directory? It
> seems to me normal push doesn't require that, and pushing to MozReview
> shouldn't have the requirement either.

We need to rewrite the commit message of the commits you're pushing for review to include the MozReview-Commit-ID, which requires a clean working directory. The shelve extension[1] makes cleaning the working directory and restoring it quite simple.

[1] https://www.mercurial-scm.org/wiki/ShelveExtension

Comment 2

[Gregory Szorc [:gps]]

Steven's explanation is correct.

However, we can leave this open to integrate shelve into the extension so things "just work."

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

But all my commits already have MozReview-ID when they are created. It should also be able to detect this and skip rewriting if all commits being pushed have that.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

In addition, doing unconditionally shelve may trigger large range of recompile due to file update on headers.

I'd also hope mercurial has a mechanism to automatically restore the mtime somehow... which would be helpful for rebase and histedit as well.

Comment 5

[BMO Automation]

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