Patchwork patch for issue 4423

login
register
mail settings
Submitter Robert Collins
Date Oct. 28, 2014, 8:39 a.m.
Message ID <CAJ3HoZ2PbEDkJa+c-A1BX3qQJySHifWT73XHy4CObfZV2+ejGA@mail.gmail.com>
Download mbox | patch
Permalink /patch/6478/
State Changes Requested
Headers show

Comments

Robert Collins - Oct. 28, 2014, 8:39 a.m.
Hi - resending here as apparently they can't be discussed in the bug
tracker. This is a driveby - it fixed it for me, but I make no claims
about correctness, other use cases or testing.

-Rob
Mads Kiilerich - Oct. 29, 2014, 4:01 p.m.
On 10/28/2014 09:39 AM, Robert Collins wrote:
> Hi - resending here as apparently they can't be discussed in the bug
> tracker. This is a driveby - it fixed it for me, but I make no claims
> about correctness, other use cases or testing.

Thanks. How driveby is it?

Could you contribute an updated patch that comply with 
http://mercurial.selenic.com/wiki/ContributingChanges ? (Or actually two 
patches for what seems to be two different issues, and including test 
coverage/updates for both.)

Or is it just thrown out here take-it-or-leave-it-ish so someone else 
who cares can pick it up and take it the last mile?

/Mads

> diff -r 6df4bc39f682 hgext/transplant.py
> --- a/hgext/transplant.py Mon Oct 27 23:47:41 2014 -0500
> +++ b/hgext/transplant.py Tue Oct 28 19:55:15 2014 +1300
> @@ -302,8 +302,12 @@
>           '''recover last transaction and apply remaining changesets'''
>           if os.path.exists(os.path.join(self.path, 'journal')):
>               n, node = self.recover(repo, source, opts)
> -            self.ui.status(_('%s transplanted as %s\n') % (short(node),
> -                                                           short(n)))
> +            if n:
> +                self.ui.status(_('%s transplanted as %s\n') % (short(node),
> +                                                               short(n)))
> +            else:
> +                self.ui.status(
> +                    _('%s skipped due to empty diff\n' % (short(node),)))
>           seriespath = os.path.join(self.path, 'series')
>           if not os.path.exists(seriespath):
>               self.transplants.write()
> @@ -344,12 +348,16 @@
>                                    revlog.hex(parent))
>               if merge:
>                   repo.setparents(p1, parents[1])
> -            n = repo.commit(message, user, date, extra=extra,
> -                            editor=self.getcommiteditor())
> -            if not n:
> -                raise util.Abort(_('commit failed'))
> -            if not merge:
> -                self.transplants.set(n, node)
> +            modified, added, removed, deleted = repo.status()[:4]
> +            if modified or added or removed or deleted:
> +                n = repo.commit(message, user, date, extra=extra,
> +                                editor=self.getcommiteditor())
> +                if not n:
> +                    raise util.Abort(_('commit failed'))
> +                if not merge:
> +                    self.transplants.set(n, node)
> +            else:
> +                n = None
>               self.unlog()
>
>               return n, node
Robert Collins - Oct. 29, 2014, 7:21 p.m.
On 30 October 2014 05:01, Mads Kiilerich <mads@kiilerich.com> wrote:
> On 10/28/2014 09:39 AM, Robert Collins wrote:
>>
>> Hi - resending here as apparently they can't be discussed in the bug
>> tracker. This is a driveby - it fixed it for me, but I make no claims
>> about correctness, other use cases or testing.
>
>
> Thanks. How driveby is it?

It's pretty driveby. I had about 20K patches in cpython to sort
through, and this was essential in resolving many of them.

If its conceptually interesting the to Mercurial community, then I
certainly want to make sure you can take the code - so if there is
anything you need (e.g. a DCO) I'll be most happy to do that.

As far as polishing the code, writing tests etc, I have very little
interest in that - and a long list of much more pressing issues.
Sorry!

> Could you contribute an updated patch that comply with
> http://mercurial.selenic.com/wiki/ContributingChanges ? (Or actually two
> patches for what seems to be two different issues, and including test
> coverage/updates for both.)

Its one issue - the code structure is such that two changes were
needed: one to the resume function, and one to the code that calls it
to handle None as the return value for the new commit... indicating no
new commit. Alternative implementations such as an exception to signal
'ok but no new commit' are also possible, but would still need two
alterations, and committing either without the other wouldn't make
sense.

> Or is it just thrown out here take-it-or-leave-it-ish so someone else who
> cares can pick it up and take it the last mile?

This :).

Patch

diff -r 6df4bc39f682 hgext/transplant.py
--- a/hgext/transplant.py	Mon Oct 27 23:47:41 2014 -0500
+++ b/hgext/transplant.py	Tue Oct 28 19:55:15 2014 +1300
@@ -302,8 +302,12 @@ 
         '''recover last transaction and apply remaining changesets'''
         if os.path.exists(os.path.join(self.path, 'journal')):
             n, node = self.recover(repo, source, opts)
-            self.ui.status(_('%s transplanted as %s\n') % (short(node),
-                                                           short(n)))
+            if n:
+                self.ui.status(_('%s transplanted as %s\n') % (short(node),
+                                                               short(n)))
+            else:
+                self.ui.status(
+                    _('%s skipped due to empty diff\n' % (short(node),)))
         seriespath = os.path.join(self.path, 'series')
         if not os.path.exists(seriespath):
             self.transplants.write()
@@ -344,12 +348,16 @@ 
                                  revlog.hex(parent))
             if merge:
                 repo.setparents(p1, parents[1])
-            n = repo.commit(message, user, date, extra=extra,
-                            editor=self.getcommiteditor())
-            if not n:
-                raise util.Abort(_('commit failed'))
-            if not merge:
-                self.transplants.set(n, node)
+            modified, added, removed, deleted = repo.status()[:4]
+            if modified or added or removed or deleted:
+                n = repo.commit(message, user, date, extra=extra,
+                                editor=self.getcommiteditor())
+                if not n:
+                    raise util.Abort(_('commit failed'))
+                if not merge:
+                    self.transplants.set(n, node)
+            else:
+                n = None
             self.unlog()
 
             return n, node