Patchwork [RFC] update: prevent merge of files with uncommitted changes (issue4404)

login
register
mail settings
Submitter Denis Laxalde
Date June 14, 2016, 1:28 p.m.
Message ID <7c0a16bdf21e7228a184.1465910912@sh77.tls.logilab.fr>
Download mbox | patch
Permalink /patch/15508/
State Changes Requested
Headers show

Comments

Denis Laxalde - June 14, 2016, 1:28 p.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1465833221 -7200
#      Mon Jun 13 17:53:41 2016 +0200
# Node ID 7c0a16bdf21e7228a1843fe5fca3000bb179d309
# Parent  60621cecc8c53d3a27e9984fb06fefc1f99797b3
update: prevent merge of files with uncommitted changes (issue4404)

The current behavior of "update" is to merge files with uncommitted changes in
the working directory with target changeset. This merge operation would either
overwrite these files if the merge is successful or leave the user with
unresolved merged conflicts otherwise. In the either cases, there's no way to
abort (often wanted in the latter case) or get back to the previous repository
state.

The idea of this patch is to prevent a merge to occur when files with
uncommitted changes differ from the target changeset to update to. The abort
message advices the user to commit or shelve their changes before updating to
target revision in order to let them a chance to handle the merge in a
recoverable way.

To disable this check and force the merge operation (previous behaviour),
added a --merge option to "update" command.

The --update option of "pull" command still goes into a merge unconditionally,
maybe it'd be better to add the safety belt here as well.

Only test-up-local-change.t got updated so far. A few others tests would need
update too (if this is approved).
timeless - June 17, 2016, 1:20 a.m.
> The abort message advices

It should be advises...

> the user to commit or shelve their changes before updating to
> target revision in order to let them a chance to handle the merge in a
recoverable way.

It'd be kinda nice if it gave a way to identify the relevant files.

Also note that we try not to suggest extensions that aren't enabled (in
some cases we might include instructions for enabling them).
Denis Laxalde - June 17, 2016, 7:09 a.m.
timeless a écrit :
> It'd be kinda nice if it gave a way to identify the relevant files.
>
> Also note that we try not to suggest extensions that aren't enabled (in
> some cases we might include instructions for enabling them).
>

I can certainly improve these aspects. Just waiting some more feedback 
to see if it's worth putting more effort on this topic.
Matt Mackall - June 17, 2016, 8:40 p.m.
On Tue, 2016-06-14 at 15:28 +0200, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1465833221 -7200
> #      Mon Jun 13 17:53:41 2016 +0200
> # Node ID 7c0a16bdf21e7228a1843fe5fca3000bb179d309
> # Parent  60621cecc8c53d3a27e9984fb06fefc1f99797b3
> update: prevent merge of files with uncommitted changes (issue4404)

I'm afraid the answer here is no. Changing update's behavior (copied from CVS
and thus literally predating Mercurial itself) is not an option.

HOWEVER, there is a way forward for broken commands:

- implement a new command with a different name with sane behavior
- eventually deprecate (hide) the old command

As it happens, I have an idea for precisely this, which I've been calling the
"goto" command. We can make a bunch of changes:

- aborts if the working copy is dirty
- use --merge to allow a merge or --clean to discard changes
- no need for the confusing --check flag
- allow merging working changes across branches
- no auto-advancing the active bookmark with no args

As a bonus, the common beginner mistake of thinking "update" only goes forward
and then trying to use "revert" to go backwards will probably go away.

As for the "merge-if-trivial" mode (which I agree is useful), I'd prefer to do
that by refactoring the merge code. There are lots of places where we'd like to
do such merges (via --tool, perhaps) and even a few places where we'd like to
ask the question of whether a merge is trivial without touching the working
copy.

-- 
Mathematics is the supreme nostalgia of our time.
Augie Fackler - June 27, 2016, 4:57 p.m.
> On Jun 17, 2016, at 4:40 PM, Matt Mackall <mpm@selenic.com> wrote:
> 
> On Tue, 2016-06-14 at 15:28 +0200, Denis Laxalde wrote:
>> # HG changeset patch
>> # User Denis Laxalde <denis.laxalde@logilab.fr>
>> # Date 1465833221 -7200
>> #      Mon Jun 13 17:53:41 2016 +0200
>> # Node ID 7c0a16bdf21e7228a1843fe5fca3000bb179d309
>> # Parent  60621cecc8c53d3a27e9984fb06fefc1f99797b3
>> update: prevent merge of files with uncommitted changes (issue4404)
> 
> I'm afraid the answer here is no. Changing update's behavior (copied from CVS
> and thus literally predating Mercurial itself) is not an option.
> 
> HOWEVER, there is a way forward for broken commands:
> 
> - implement a new command with a different name with sane behavior
> - eventually deprecate (hide) the old command
> 
> As it happens, I have an idea for precisely this, which I've been calling the
> "goto" command. We can make a bunch of changes:
> 
> - aborts if the working copy is dirty
> - use --merge to allow a merge or --clean to discard changes
> - no need for the confusing --check flag
> - allow merging working changes across branches
> - no auto-advancing the active bookmark with no args

Should https://www.mercurial-scm.org/wiki/GotoCommandPlan exist and start capturing these ideas?

> 
> As a bonus, the common beginner mistake of thinking "update" only goes forward
> and then trying to use "revert" to go backwards will probably go away.
> 
> As for the "merge-if-trivial" mode (which I agree is useful), I'd prefer to do
> that by refactoring the merge code. There are lots of places where we'd like to
> do such merges (via --tool, perhaps) and even a few places where we'd like to
> ask the question of whether a merge is trivial without touching the working
> copy.
> 
> --
> Mathematics is the supreme nostalgia of our time.
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - July 2, 2016, 3:24 a.m.
On Mon, Jun 27, 2016 at 9:57 AM, Augie Fackler <raf@durin42.com> wrote:

>
> > On Jun 17, 2016, at 4:40 PM, Matt Mackall <mpm@selenic.com> wrote:
> >
> > On Tue, 2016-06-14 at 15:28 +0200, Denis Laxalde wrote:
> >> # HG changeset patch
> >> # User Denis Laxalde <denis.laxalde@logilab.fr>
> >> # Date 1465833221 -7200
> >> #      Mon Jun 13 17:53:41 2016 +0200
> >> # Node ID 7c0a16bdf21e7228a1843fe5fca3000bb179d309
> >> # Parent  60621cecc8c53d3a27e9984fb06fefc1f99797b3
> >> update: prevent merge of files with uncommitted changes (issue4404)
> >
> > I'm afraid the answer here is no. Changing update's behavior (copied
> from CVS
> > and thus literally predating Mercurial itself) is not an option.
> >
> > HOWEVER, there is a way forward for broken commands:
> >
> > - implement a new command with a different name with sane behavior
> > - eventually deprecate (hide) the old command
> >
> > As it happens, I have an idea for precisely this, which I've been
> calling the
> > "goto" command. We can make a bunch of changes:
> >
> > - aborts if the working copy is dirty
> > - use --merge to allow a merge or --clean to discard changes
> > - no need for the confusing --check flag
> > - allow merging working changes across branches
> > - no auto-advancing the active bookmark with no args
>
> Should https://www.mercurial-scm.org/wiki/GotoCommandPlan exist and start
> capturing these ideas?
>

Page created with many of mpm's words captured.


>
> >
> > As a bonus, the common beginner mistake of thinking "update" only goes
> forward
> > and then trying to use "revert" to go backwards will probably go away.
> >
> > As for the "merge-if-trivial" mode (which I agree is useful), I'd prefer
> to do
> > that by refactoring the merge code. There are lots of places where we'd
> like to
> > do such merges (via --tool, perhaps) and even a few places where we'd
> like to
> > ask the question of whether a merge is trivial without touching the
> working
> > copy.
> >
> > --
> > Mathematics is the supreme nostalgia of our time.
> >
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -359,6 +359,22 @@  def bailifchanged(repo, merge=True):
     for s in sorted(ctx.substate):
         ctx.sub(s).bailifchanged()
 
+def bailifuncommitteddifferfromrev(repo, rev):
+    """Bail if files with uncommitted changes differ between working
+    directory and `rev` to update to.
+    """
+    msg = _("uncommitted changes would be overwritten by update")
+    hint = _("commit or shelve your changes first or use --merge")
+    wctx, tctx = repo[None], repo[rev]
+    modified, added, removed, deleted = repo.status()[:4]
+    for fpath in modified + added:
+        if (fpath in tctx
+                and tctx.filectx(fpath).cmp(wctx.filectx(fpath))):
+            raise error.UpdateAbort(msg, hint=hint)
+    for fpath in removed + deleted:
+        if fpath in tctx:
+            raise error.UpdateAbort(msg, hint=hint)
+
 def logmessage(ui, opts):
     """ get the log message according to -m and -l option """
     message = opts.get('message')
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5739,7 +5739,7 @@  def postincoming(ui, repo, modheads, opt
         return
     if optupdate:
         try:
-            return hg.updatetotally(ui, repo, checkout, brev)
+            return hg.updatetotally(ui, repo, checkout, brev, merge=True)
         except error.UpdateAbort as inst:
             msg = _("not updating: %s") % str(inst)
             hint = inst.hint
@@ -7124,12 +7124,13 @@  def unbundle(ui, repo, fname1, *fnames, 
 @command('^update|up|checkout|co',
     [('C', 'clean', None, _('discard uncommitted changes (no backup)')),
     ('c', 'check', None, _('require clean working directory')),
+    ('m', 'merge', False, _('merge working directory with target changeset')),
     ('d', 'date', '', _('tipmost revision matching date'), _('DATE')),
     ('r', 'rev', '', _('revision'), _('REV'))
      ] + mergetoolopts,
     _('[-c] [-C] [-d DATE] [[-r] REV]'))
 def update(ui, repo, node=None, rev=None, clean=False, date=None, check=False,
-           tool=None):
+           merge=False, tool=None):
     """update working directory (or switch revisions)
 
     Update the repository's working directory to the specified
@@ -7160,10 +7161,15 @@  def update(ui, repo, node=None, rev=None
          branch), the update is aborted and the uncommitted changes
          are preserved.
 
-      2. With the -c/--check option, the update is aborted and the
+      2. Without -C/--clean option, if some files with uncommitted
+         changes differ between the working directory and the requested
+         changeset, the update is aborted. This can be by-passed with
+         the --merge option.
+
+      3. With the -c/--check option, the update is aborted and the
          uncommitted changes are preserved.
 
-      3. With the -C/--clean option, uncommitted changes are discarded and
+      4. With the -C/--clean option, uncommitted changes are discarded and
          the working directory is updated to the requested changeset.
 
     To cancel an uncommitted merge (and lose your changes), use
@@ -7206,7 +7212,8 @@  def update(ui, repo, node=None, rev=None
 
         repo.ui.setconfig('ui', 'forcemerge', tool, 'update')
 
-        return hg.updatetotally(ui, repo, rev, brev, clean=clean, check=check)
+        return hg.updatetotally(ui, repo, rev, brev, clean=clean, check=check,
+                                merge=merge)
 
 @command('verify', [])
 def verify(ui, repo):
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -698,7 +698,8 @@  def clean(repo, node, show_stats=True, q
 # naming conflict in updatetotally()
 _clean = clean
 
-def updatetotally(ui, repo, checkout, brev, clean=False, check=False):
+def updatetotally(ui, repo, checkout, brev, clean=False, check=False,
+                  merge=True):
     """Update the working directory with extra care for non-file components
 
     This takes care of non-file components below:
@@ -711,6 +712,8 @@  def updatetotally(ui, repo, checkout, br
     :brev: a name, which might be a bookmark to be activated after updating
     :clean: whether changes in the working directory can be discarded
     :check: whether changes in the working directory should be checked
+    :merge: force merge between changes in the working directory and
+        `checkout` revision
 
     This returns whether conflict is detected at updating or not.
     """
@@ -725,6 +728,8 @@  def updatetotally(ui, repo, checkout, br
         if clean:
             ret = _clean(repo, checkout)
         else:
+            if not merge:
+                cmdutil.bailifuncommitteddifferfromrev(repo, checkout)
             ret = _update(repo, checkout)
 
         if not ret and movemarkfrom:
diff --git a/tests/test-up-local-change.t b/tests/test-up-local-change.t
--- a/tests/test-up-local-change.t
+++ b/tests/test-up-local-change.t
@@ -39,7 +39,7 @@ 
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     1
   
-  $ hg --debug up
+  $ hg --debug up --merge
     searching for copies back to rev 1
     unmatched files in other:
      b
@@ -66,7 +66,7 @@ 
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     2
   
-  $ hg --debug up 0
+  $ hg --debug up 0 --merge
   resolving manifests
    branchmerge: False, force: False, partial: False
    ancestor: 1e71731e6fbb, local: 1e71731e6fbb+, remote: c19d34741b0a
@@ -96,7 +96,7 @@ 
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     1
   
-  $ hg --debug up
+  $ hg --debug up --merge
     searching for copies back to rev 1
     unmatched files in other:
      b
@@ -172,10 +172,50 @@  create a second head
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     2
   
-  $ hg --debug up
+  $ hg --debug up --merge
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   1 other heads for branch "default"
 
+prevent merge conflicts for modified files
+
+  $ hg up
+  abort: uncommitted changes would be overwritten by update
+  (commit or shelve your changes first or use --merge)
+  [255]
+
+prevent merge conflicts for added file
+
+  $ hg up -qC 0
+  $ hg cat -r 1 b
+  b
+  $ echo c > b
+  $ hg add b
+  $ hg st
+  A b
+  $ hg up 1
+  abort: uncommitted changes would be overwritten by update
+  (commit or shelve your changes first or use --merge)
+  [255]
+  $ rm b
+
+prevent merge conflicts for removed file
+
+  $ hg rm a
+  $ hg up 1
+  abort: uncommitted changes would be overwritten by update
+  (commit or shelve your changes first or use --merge)
+  [255]
+  $ hg up -qC 0
+
+prevent merge conflicts for deleted file
+
+  $ rm a
+  $ hg up 1
+  abort: uncommitted changes would be overwritten by update
+  (commit or shelve your changes first or use --merge)
+  [255]
+  $ hg up -qC 0
+
 test conflicting untracked files
 
   $ hg up -qC 0