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
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
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