Patchwork [STABLE] update: do not pass in user revspec as default destination (issue6044)

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 2, 2019, 1:04 a.m.
Message ID <035f7a392747ac3fd720.1546391089@mimosa>
Download mbox | patch
Permalink /patch/37417/
State Accepted
Headers show

Comments

Yuya Nishihara - Jan. 2, 2019, 1:04 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1546389664 -32400
#      Wed Jan 02 09:41:04 2019 +0900
# Branch stable
# Node ID 035f7a392747ac3fd720d3181e387bc42ca12845
# Parent  7542466b94e25ec658e64bd3d33105a59bebc53e
update: do not pass in user revspec as default destination (issue6044)

When the revsingle() was introduced at 61c0df2b089a, it couldn't handle
revspec=0 (not '0') properly. That's probably why the default was set to
rev.

This is technically BC since "hg update ''" was identical to "hg update '.'"
whereas "hg update -r ''" is "hg update", but I believe that's a bug given
no test fails with this change.
Boris Feld - Jan. 2, 2019, 9:03 a.m.
LGTM but a second review is necessary I think.

On 02/01/2019 02:04, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1546389664 -32400
> #      Wed Jan 02 09:41:04 2019 +0900
> # Branch stable
> # Node ID 035f7a392747ac3fd720d3181e387bc42ca12845
> # Parent  7542466b94e25ec658e64bd3d33105a59bebc53e
> update: do not pass in user revspec as default destination (issue6044)
>
> When the revsingle() was introduced at 61c0df2b089a, it couldn't handle
> revspec=0 (not '0') properly. That's probably why the default was set to
> rev.
>
> This is technically BC since "hg update ''" was identical to "hg update '.'"
> whereas "hg update -r ''" is "hg update", but I believe that's a bug given
> no test fails with this change.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -6038,7 +6038,7 @@ def update(ui, repo, node=None, **opts):
>          brev = rev
>          if rev:
>              repo = scmutil.unhidehashlikerevs(repo, [rev], 'nowarn')
> -        ctx = scmutil.revsingle(repo, rev, rev)
> +        ctx = scmutil.revsingle(repo, rev, default=None)
>          rev = ctx.rev()
>          hidden = ctx.hidden()
>          overrides = {('ui', 'forcemerge'): opts.get(r'tool', '')}
> diff --git a/tests/test-simple-update.t b/tests/test-simple-update.t
> --- a/tests/test-simple-update.t
> +++ b/tests/test-simple-update.t
> @@ -57,6 +57,26 @@ update to rev 0 with a date
>    abort: you can't specify a revision and a date
>    [255]
>  
> +update to default destination (with empty revspec)
> +
> +  $ hg update -q null
> +  $ hg update
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg id
> +  30aff43faee1 tip
> +
> +  $ hg update -q null
> +  $ hg update -r ''
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg id
> +  30aff43faee1 tip
> +
> +  $ hg update -q null
> +  $ hg update ''
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg id
> +  30aff43faee1 tip
> +
>    $ cd ..
>  
>  update with worker processes
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pulkit Goyal - Jan. 3, 2019, 10:12 a.m.
On Wed, Jan 2, 2019 at 2:34 PM Boris FELD <boris.feld@octobus.net> wrote:

> LGTM but a second review is necessary I think.
>
> On 02/01/2019 02:04, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1546389664 -32400
> > #      Wed Jan 02 09:41:04 2019 +0900
> > # Branch stable
> > # Node ID 035f7a392747ac3fd720d3181e387bc42ca12845
> > # Parent  7542466b94e25ec658e64bd3d33105a59bebc53e
> > update: do not pass in user revspec as default destination (issue6044)
> >
> > When the revsingle() was introduced at 61c0df2b089a, it couldn't handle
> > revspec=0 (not '0') properly. That's probably why the default was set to
> > rev.
> >
> > This is technically BC since "hg update ''" was identical to "hg update
> '.'"
> > whereas "hg update -r ''" is "hg update", but I believe that's a bug
> given
> > no test fails with this change.
>

Looks fine to me. Queued this, many thanks!

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -6038,7 +6038,7 @@  def update(ui, repo, node=None, **opts):
         brev = rev
         if rev:
             repo = scmutil.unhidehashlikerevs(repo, [rev], 'nowarn')
-        ctx = scmutil.revsingle(repo, rev, rev)
+        ctx = scmutil.revsingle(repo, rev, default=None)
         rev = ctx.rev()
         hidden = ctx.hidden()
         overrides = {('ui', 'forcemerge'): opts.get(r'tool', '')}
diff --git a/tests/test-simple-update.t b/tests/test-simple-update.t
--- a/tests/test-simple-update.t
+++ b/tests/test-simple-update.t
@@ -57,6 +57,26 @@  update to rev 0 with a date
   abort: you can't specify a revision and a date
   [255]
 
+update to default destination (with empty revspec)
+
+  $ hg update -q null
+  $ hg update
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg id
+  30aff43faee1 tip
+
+  $ hg update -q null
+  $ hg update -r ''
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg id
+  30aff43faee1 tip
+
+  $ hg update -q null
+  $ hg update ''
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg id
+  30aff43faee1 tip
+
   $ cd ..
 
 update with worker processes