Patchwork commands: graft uses a transaction (issue3628)

login
register
mail settings
Submitter Alexander Becher
Date Aug. 10, 2016, 10:39 a.m.
Message ID <b15d454d-fb9a-9c77-b042-f120c1d7d4de@rud-steuerungstechnik.de>
Download mbox | patch
Permalink /patch/16247/
State Rejected
Headers show

Comments

Alexander Becher - Aug. 10, 2016, 10:39 a.m.
Dear all,

I tried this change for issue3628:




Regards,
Alexander
Pierre-Yves David - Aug. 11, 2016, 4:08 p.m.
On 08/10/2016 12:39 PM, Alexander Becher wrote:
> Dear all,
>
> I tried this change for issue3628:
>
>
> diff -r db0095c83344 -r 7aa594597cb3 mercurial/commands.py
> --- a/mercurial/commands.py     Mon Jul 18 22:22:38 2016 +0200
> +++ b/mercurial/commands.py     Fri Jul 29 11:02:45 2016 +0200
> @@ -4085,7 +4085,7 @@
>
>      Returns 0 on successful completion.
>      '''
> -    with repo.wlock():
> +    with repo.wlock(), repo.lock(), repo.transaction('graft'):
>          return _dograft(ui, repo, *revs, **opts)
>
>  def _dograft(ui, repo, *revs, **opts):
>
>
> Seemed simple enough. However, this change causes test-graft.t to fail,
> see below. I don't see why, it's not the first test for graft
> --continue. Can anyone see the reason?

This won't do. If you do this, the failure of one graft will rollback 
all the previous (successful one). We need much more advance transaction 
mechanisme to solve this issue. (sorry)

> Also, do I need both repo.wlock() and repo.lock()? Without repo.lock(),
> repo.transaction('graft') raises a RuntimeError('programming error:
> transaction requires locking').

Then you probably needs the repo.lock ;-)
Alexander Becher - Aug. 12, 2016, 8:21 a.m.
* Pierre-Yves David [11.08.2016 18:08]
> On 08/10/2016 12:39 PM, Alexander Becher wrote:
>> diff -r db0095c83344 -r 7aa594597cb3 mercurial/commands.py
>> --- a/mercurial/commands.py     Mon Jul 18 22:22:38 2016 +0200
>> +++ b/mercurial/commands.py     Fri Jul 29 11:02:45 2016 +0200
>> @@ -4085,7 +4085,7 @@
>>
>>      Returns 0 on successful completion.
>>      '''
>> -    with repo.wlock():
>> +    with repo.wlock(), repo.lock(), repo.transaction('graft'):
>>          return _dograft(ui, repo, *revs, **opts)
>>
>>  def _dograft(ui, repo, *revs, **opts):
> 
> This won't do. If you do this, the failure of one graft will rollback
> all the previous (successful one). We need much more advance transaction
> mechanisme to solve this issue. (sorry)

Well, that's what I want, isn't it? Say, I graft a number of changesets,
then realise that I've been grafting them to the wrong branch, then I'd
like to remove all changesets that were newly created by the graft, like
this:

  $ hg par
  4710:ba420e33
  $ hg graft 'branch(issue3628)'
  grafting changeset 4711
  grafting changeset 4712
  grafting changeset 4713
  $ hg branch
  wrong_branch
  $ hg rollback
  undo graft (remove changeset 4714, 4715, 4716)
  working directory now based on 4710
  $ hg up correct_branch
  $ hg graft 'branch(issue3628)'
  grafting changeset 4711
  grafting changeset 4712
  grafting changeset 4713
  $ hg par
  4716 (grafted from 4713)

Reasoning: I did a command, hg graft, then realised it was a mistake,
and want to undo all the changes it made to my repository.

>> Also, do I need both repo.wlock() and repo.lock()? Without repo.lock(),
>> repo.transaction('graft') raises a RuntimeError('programming error:
>> transaction requires locking').
> 
> Then you probably needs the repo.lock ;-)

Yes, but do I still need the repo.wlock? Or does repo.lock imply repo.wlock?

Patch

diff -r db0095c83344 -r 7aa594597cb3 mercurial/commands.py
--- a/mercurial/commands.py     Mon Jul 18 22:22:38 2016 +0200
+++ b/mercurial/commands.py     Fri Jul 29 11:02:45 2016 +0200
@@ -4085,7 +4085,7 @@ 

     Returns 0 on successful completion.
     '''
-    with repo.wlock():
+    with repo.wlock(), repo.lock(), repo.transaction('graft'):
         return _dograft(ui, repo, *revs, **opts)

 def _dograft(ui, repo, *revs, **opts):


Seemed simple enough. However, this change causes test-graft.t to fail,
see below. I don't see why, it's not the first test for graft
--continue. Can anyone see the reason?

Also, do I need both repo.wlock() and repo.lock()? Without repo.lock(),
repo.transaction('graft') raises a RuntimeError('programming error:
transaction requires locking').


$ (cd tests && python run-tests.py --first test-graft.t)

--- /cygdrive/g/src/hg/tests/test-graft.t
+++ /cygdrive/g/src/hg/tests/test-graft.t.err
@@ -489,18 +489,22 @@ 
   continue: hg graft --continue
   $ hg graft -c
   grafting 2:5c095ad7e90f "2"
+  note: graft of 2:5c095ad7e90f created no changes to commit
   $ hg export tip --git
   # HG changeset patch
   # User test
   # Date 0 0
   #      Thu Jan 01 00:00:00 1970 +0000
-  # Node ID 9627f653b421c61fc1ea4c4e366745070fa3d2bc
-  # Parent  ee295f490a40b97f3d18dd4c4f1c8936c233b612
-  2
-
-  diff --git a/a b/b
-  rename from a
-  rename to b
+  # Node ID ee295f490a40b97f3d18dd4c4f1c8936c233b612
+  # Parent  f67661df0c4804d301f064f332b57e7d5ddaf2be
+  10
+
+  diff --git a/a b/a
+  --- a/a
+  +++ b/a
+  @@ -1,1 +1,1 @@
+  -b
+  +c

 Test simple origin(), with and without args
   $ hg log -r 'origin()'