import --no-commit and rollback

Greg Ward greg at gerg.ca
Fri Mar 16 21:27:37 CDT 2012


[moving to mercurial-devel]

On 16 March 2012, Andreas Bernauer said:
> On 3/16/12 3:08, Roman Neuhauser wrote:
> > hello,
> > 
> > i wouldn't expect import --no-commit to be considered rollback-worthy.
> > is looks like a bug.  the script is attached.
> 
> 
> I wouldn't expect that, either.  This patch against 594fc9329628 may fix it:
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -3598,7 +3598,10 @@
>          try:
>              wlock = repo.wlock()
>              lock = repo.lock()
> -            tr = repo.transaction('import')
> +            if opts.get('no_commit'):
> +                tr = None
> +            else:
> +                tr = repo.transaction('import')

Yuck: that duplicates the "if nocommit" case in widely separated
places, and makes the code even more deeply nested than it already is.

Maybe a better approach: outside the outer try, do this:

  if opts.get('no_commit'):
      def start():
          wlock = repo.wlock()
      def finish():
          release(wlock)
  else:
      def start():
          wlock = repo.wlock()
          lock = repo.lock()
          tr = repo.transaction('import')
      def finish():
          if tr:
              tr.release()
          release(lock, wlock)

Now there is the minor implementation detail that closures cannot
assign to variables in their containing scope. But they can mutate
objects in the containing scope, so there's probably a way to do this
without adding duplicate conditional logic in deeply nested and widely
separated try/try/finally blocks.

Give it a try!

        Greg
-- 
Greg Ward                                http://www.gerg.ca/
Sure, I'm paranoid... but am I paranoid ENOUGH?


More information about the Mercurial-devel mailing list