Patchwork subrepo: don't push mercurial subrepositories unless necessary (issue3867)

login
register
mail settings
Submitter Nathan Hoad
Date March 25, 2013, 10:45 p.m.
Message ID <5150d3b4.21b4440a.6597.ffffdb7a@mx.google.com>
Download mbox | patch
Permalink /patch/1194/
State Not Applicable, archived
Headers show

Comments

Nathan Hoad - March 25, 2013, 10:45 p.m.
# HG changeset patch
# User Nathan Hoad <nathan@getoffmalawn.com>
# Date 1364251459 -39600
# Node ID 02259346e572480c53b47b01b6f12a2227bb75d2
# Parent  af9ddea2cb99b6e4314a6e79b793b5fc30ea4f19
subrepo: don't push mercurial subrepositories unless necessary (issue3867)

When I try to push a repository containing a mixture of mercurial and git
subrepositories, mercurial always attempts to push the mercurial subrepos,
instead of only pushing if necessary.
Bryan O'Sullivan - March 26, 2013, 12:11 a.m.
On Mon, Mar 25, 2013 at 3:45 PM, Nathan Hoad <nathan@getoffmalawn.com>wrote:

> subrepo: don't push mercurial subrepositories unless necessary (issue3867)
>

This patch makes Mercurial perform a whole lot of extra work if some
changes must be pushed (it has to discover the set of changes to push
twice), but it does not reduce the amount of work done if nothing needs to
be pushed. I believe you are trying to fix a non-bug.
Sean Farley - March 26, 2013, 3:12 a.m.
Bryan O'Sullivan writes:

> On Mon, Mar 25, 2013 at 3:45 PM, Nathan Hoad <nathan@getoffmalawn.com>wrote:
>
>> subrepo: don't push mercurial subrepositories unless necessary (issue3867)
>>
>
> This patch makes Mercurial perform a whole lot of extra work if some
> changes must be pushed (it has to discover the set of changes to push
> twice), but it does not reduce the amount of work done if nothing needs to
> be pushed. I believe you are trying to fix a non-bug.

Well, I don't know about "non-bug" but Angel has a patch series (that
needs some love) that tackles it another way:

http://markmail.org/message/zt76am36n2yjiio6
Angel Ezquerra - March 26, 2013, 3:50 a.m.
On Mar 26, 2013 4:12 AM, "Sean Farley" <sean.michael.farley@gmail.com>
wrote:
>
>
> Bryan O'Sullivan writes:
>
> > On Mon, Mar 25, 2013 at 3:45 PM, Nathan Hoad <nathan@getoffmalawn.com
>wrote:
> >
> >> subrepo: don't push mercurial subrepositories unless necessary
(issue3867)
> >>
> >
> > This patch makes Mercurial perform a whole lot of extra work if some
> > changes must be pushed (it has to discover the set of changes to push
> > twice), but it does not reduce the amount of work done if nothing needs
to
> > be pushed. I believe you are trying to fix a non-bug.
>
> Well, I don't know about "non-bug" but Angel has a patch series (that
> needs some love) that tackles it another way:
>
> http://markmail.org/message/zt76am36n2yjiio6
>

Yes, the approach I took was proposed by Matt during the London sprint.

My series has already been reviewed by Augie, but I think he wanted someone
else to give his opinion as well. I'm waiting for comments on the latest
version that I sent a while ago.

BTW, this is a real issue. Push times when you have a fair number of
subrepos increase dramatically (proportionally to the number of subrepos).

Cheers,

Angel
Nathan Hoad - March 26, 2013, 3:58 a.m.
For me it is a usability issue, not so much an issue of how long it
takes. Some of the subrepos require auth to push, and are not mine, so
I can't actually push to them. Because of this, I get prompted for
auth, and can't continue with pushing any of the other subrepos, or
the parent repository.

I should have been more descriptive of the issue in the commit
message, apologies.

Thanks,

Nathan.
--
Nathan Hoad
Software Developer
www.getoffmalawn.com


On Tue, Mar 26, 2013 at 2:50 PM, Angel Ezquerra <ezquerra@gmail.com> wrote:
>
> On Mar 26, 2013 4:12 AM, "Sean Farley" <sean.michael.farley@gmail.com>
> wrote:
>>
>>
>> Bryan O'Sullivan writes:
>>
>> > On Mon, Mar 25, 2013 at 3:45 PM, Nathan Hoad
>> > <nathan@getoffmalawn.com>wrote:
>> >
>> >> subrepo: don't push mercurial subrepositories unless necessary
>> >> (issue3867)
>> >>
>> >
>> > This patch makes Mercurial perform a whole lot of extra work if some
>> > changes must be pushed (it has to discover the set of changes to push
>> > twice), but it does not reduce the amount of work done if nothing needs
>> > to
>> > be pushed. I believe you are trying to fix a non-bug.
>>
>> Well, I don't know about "non-bug" but Angel has a patch series (that
>> needs some love) that tackles it another way:
>>
>> http://markmail.org/message/zt76am36n2yjiio6
>>
>
> Yes, the approach I took was proposed by Matt during the London sprint.
>
> My series has already been reviewed by Augie, but I think he wanted someone
> else to give his opinion as well. I'm waiting for comments on the latest
> version that I sent a while ago.
>
> BTW, this is a real issue. Push times when you have a fair number of
> subrepos increase dramatically (proportionally to the number of subrepos).
>
> Cheers,
>
> Angel
Angel Ezquerra - March 26, 2013, 4:22 a.m.
On Mar 26, 2013 4:58 AM, "Nathan Hoad" <nathan@getoffmalawn.com> wrote:
>
> For me it is a usability issue, not so much an issue of how long it
> takes. Some of the subrepos require auth to push, and are not mine, so
> I can't actually push to them. Because of this, I get prompted for
> auth, and can't continue with pushing any of the other subrepos, or
> the parent repository.

My patch series should address your problem as well. It keeps track of
which subrepos have been "modified" versus each of its push/pull sources
and if you have not modified a subrepo it won't be pushed.

Another thing we discussed during the sprint is adding a way to configure
subrepo properties. For example you could tell mercurial that a subrepo is
"read only" and then it would never be pushed.

I have not done anything along those lines yet because I'd rather be done
with the two subrepo related patch series that I have pending first.

Cheers,

Angel

> I should have been more descriptive of the issue in the commit
> message, apologies.
>
> Thanks,
>
> Nathan.
> --
> Nathan Hoad
> Software Developer
> www.getoffmalawn.com
>
>
> On Tue, Mar 26, 2013 at 2:50 PM, Angel Ezquerra <ezquerra@gmail.com>
wrote:
> >
> > On Mar 26, 2013 4:12 AM, "Sean Farley" <sean.michael.farley@gmail.com>
> > wrote:
> >>
> >>
> >> Bryan O'Sullivan writes:
> >>
> >> > On Mon, Mar 25, 2013 at 3:45 PM, Nathan Hoad
> >> > <nathan@getoffmalawn.com>wrote:
> >> >
> >> >> subrepo: don't push mercurial subrepositories unless necessary
> >> >> (issue3867)
> >> >>
> >> >
> >> > This patch makes Mercurial perform a whole lot of extra work if some
> >> > changes must be pushed (it has to discover the set of changes to push
> >> > twice), but it does not reduce the amount of work done if nothing
needs
> >> > to
> >> > be pushed. I believe you are trying to fix a non-bug.
> >>
> >> Well, I don't know about "non-bug" but Angel has a patch series (that
> >> needs some love) that tackles it another way:
> >>
> >> http://markmail.org/message/zt76am36n2yjiio6
> >>
> >
> > Yes, the approach I took was proposed by Matt during the London sprint.
> >
> > My series has already been reviewed by Augie, but I think he wanted
someone
> > else to give his opinion as well. I'm waiting for comments on the latest
> > version that I sent a while ago.
> >
> > BTW, this is a real issue. Push times when you have a fair number of
> > subrepos increase dramatically (proportionally to the number of
subrepos).
> >
> > Cheers,
> >
> > Angel
Nathan Hoad - March 26, 2013, 2 p.m.
Great, that sounds good to me. I'm happy to forget about my patch,
given that knowledge. Is there a ticket on the bugtracker for your
patch series, so I can close mine as a duplicate of yours?

Also, do you have any idea what release this will be in?

Thanks,

Nathan.
--
Nathan Hoad
Software Developer
www.getoffmalawn.com


On Tue, Mar 26, 2013 at 3:22 PM, Angel Ezquerra
<angel.ezquerra@gmail.com> wrote:
>
> On Mar 26, 2013 4:58 AM, "Nathan Hoad" <nathan@getoffmalawn.com> wrote:
>>
>> For me it is a usability issue, not so much an issue of how long it
>> takes. Some of the subrepos require auth to push, and are not mine, so
>> I can't actually push to them. Because of this, I get prompted for
>> auth, and can't continue with pushing any of the other subrepos, or
>> the parent repository.
>
> My patch series should address your problem as well. It keeps track of which
> subrepos have been "modified" versus each of its push/pull sources and if
> you have not modified a subrepo it won't be pushed.
>
> Another thing we discussed during the sprint is adding a way to configure
> subrepo properties. For example you could tell mercurial that a subrepo is
> "read only" and then it would never be pushed.
>
> I have not done anything along those lines yet because I'd rather be done
> with the two subrepo related patch series that I have pending first.
>
> Cheers,
>
> Angel
>
>> I should have been more descriptive of the issue in the commit
>> message, apologies.
>>
>> Thanks,
>>
>> Nathan.
>> --
>> Nathan Hoad
>> Software Developer
>> www.getoffmalawn.com
>>
>>
>> On Tue, Mar 26, 2013 at 2:50 PM, Angel Ezquerra <ezquerra@gmail.com>
>> wrote:
>> >
>> > On Mar 26, 2013 4:12 AM, "Sean Farley" <sean.michael.farley@gmail.com>
>> > wrote:
>> >>
>> >>
>> >> Bryan O'Sullivan writes:
>> >>
>> >> > On Mon, Mar 25, 2013 at 3:45 PM, Nathan Hoad
>> >> > <nathan@getoffmalawn.com>wrote:
>> >> >
>> >> >> subrepo: don't push mercurial subrepositories unless necessary
>> >> >> (issue3867)
>> >> >>
>> >> >
>> >> > This patch makes Mercurial perform a whole lot of extra work if some
>> >> > changes must be pushed (it has to discover the set of changes to push
>> >> > twice), but it does not reduce the amount of work done if nothing
>> >> > needs
>> >> > to
>> >> > be pushed. I believe you are trying to fix a non-bug.
>> >>
>> >> Well, I don't know about "non-bug" but Angel has a patch series (that
>> >> needs some love) that tackles it another way:
>> >>
>> >> http://markmail.org/message/zt76am36n2yjiio6
>> >>
>> >
>> > Yes, the approach I took was proposed by Matt during the London sprint.
>> >
>> > My series has already been reviewed by Augie, but I think he wanted
>> > someone
>> > else to give his opinion as well. I'm waiting for comments on the latest
>> > version that I sent a while ago.
>> >
>> > BTW, this is a real issue. Push times when you have a fair number of
>> > subrepos increase dramatically (proportionally to the number of
>> > subrepos).
>> >
>> > Cheers,
>> >
>> > Angel
Angel Ezquerra - March 26, 2013, 2:58 p.m.
On Tue, Mar 26, 2013 at 3:00 PM, Nathan Hoad <nathan@getoffmalawn.com> wrote:
> Great, that sounds good to me. I'm happy to forget about my patch,
> given that knowledge. Is there a ticket on the bugtracker for your
> patch series, so I can close mine as a duplicate of yours?

I did not open any ticket for it.

> Also, do you have any idea what release this will be in?

As always, no guarantees can be made. That being said my hope was that
it would be included on the next major mercurial release if all goes
well during review.

Cheers,

Angel


> On Tue, Mar 26, 2013 at 3:22 PM, Angel Ezquerra
> <angel.ezquerra@gmail.com> wrote:
>>
>> On Mar 26, 2013 4:58 AM, "Nathan Hoad" <nathan@getoffmalawn.com> wrote:
>>>
>>> For me it is a usability issue, not so much an issue of how long it
>>> takes. Some of the subrepos require auth to push, and are not mine, so
>>> I can't actually push to them. Because of this, I get prompted for
>>> auth, and can't continue with pushing any of the other subrepos, or
>>> the parent repository.
>>
>> My patch series should address your problem as well. It keeps track of which
>> subrepos have been "modified" versus each of its push/pull sources and if
>> you have not modified a subrepo it won't be pushed.
>>
>> Another thing we discussed during the sprint is adding a way to configure
>> subrepo properties. For example you could tell mercurial that a subrepo is
>> "read only" and then it would never be pushed.
>>
>> I have not done anything along those lines yet because I'd rather be done
>> with the two subrepo related patch series that I have pending first.
>>
>> Cheers,
>>
>> Angel
>>
>>> I should have been more descriptive of the issue in the commit
>>> message, apologies.
>>>
>>> Thanks,
>>>
>>> Nathan.
>>> --
>>> Nathan Hoad
>>> Software Developer
>>> www.getoffmalawn.com
>>>
>>>
>>> On Tue, Mar 26, 2013 at 2:50 PM, Angel Ezquerra <ezquerra@gmail.com>
>>> wrote:
>>> >
>>> > On Mar 26, 2013 4:12 AM, "Sean Farley" <sean.michael.farley@gmail.com>
>>> > wrote:
>>> >>
>>> >>
>>> >> Bryan O'Sullivan writes:
>>> >>
>>> >> > On Mon, Mar 25, 2013 at 3:45 PM, Nathan Hoad
>>> >> > <nathan@getoffmalawn.com>wrote:
>>> >> >
>>> >> >> subrepo: don't push mercurial subrepositories unless necessary
>>> >> >> (issue3867)
>>> >> >>
>>> >> >
>>> >> > This patch makes Mercurial perform a whole lot of extra work if some
>>> >> > changes must be pushed (it has to discover the set of changes to push
>>> >> > twice), but it does not reduce the amount of work done if nothing
>>> >> > needs
>>> >> > to
>>> >> > be pushed. I believe you are trying to fix a non-bug.
>>> >>
>>> >> Well, I don't know about "non-bug" but Angel has a patch series (that
>>> >> needs some love) that tackles it another way:
>>> >>
>>> >> http://markmail.org/message/zt76am36n2yjiio6
>>> >>
>>> >
>>> > Yes, the approach I took was proposed by Matt during the London sprint.
>>> >
>>> > My series has already been reviewed by Augie, but I think he wanted
>>> > someone
>>> > else to give his opinion as well. I'm waiting for comments on the latest
>>> > version that I sent a while ago.
>>> >
>>> > BTW, this is a real issue. Push times when you have a fair number of
>>> > subrepos increase dramatically (proportionally to the number of
>>> > subrepos).
>>> >
>>> > Cheers,
>>> >
>>> > Angel
Dave S - March 27, 2013, 8:18 p.m.
On Tue, Mar 26, 2013 at 7:58 AM, Angel Ezquerra
<angel.ezquerra@gmail.com> wrote:
> On Tue, Mar 26, 2013 at 3:00 PM, Nathan Hoad <nathan@getoffmalawn.com> wrote:
>> Great, that sounds good to me. I'm happy to forget about my patch,
>> given that knowledge. Is there a ticket on the bugtracker for your
>> patch series, so I can close mine as a duplicate of yours?
>
> I did not open any ticket for it.

However, it is visible on the patchworks server:
<http://patchwork.serpentine.com/patch/1090/>
(for the specific patch in the series , but you can see the whole
series from the front page)

And the overview of the series, as well as the whole series it
introduces,  is on the mail archive:
<http://selenic.com/pipermail/mercurial-devel/2013-March/049548.html>
and following.

/dps

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -9,6 +9,7 @@  import errno, os, re, xml.dom.minidom, s
 import stat, subprocess, tarfile
 from i18n import _
 import config, scmutil, util, node, error, cmdutil, bookmarks, match as matchmod
+import discovery
 hg = None
 propertycache = util.propertycache
 
@@ -599,9 +600,18 @@  class hgsubrepo(abstractsubrepo):
                 return False
 
         dsturl = _abssource(self._repo, True)
+
+        other = hg.peer(self._repo, {'ssh': ssh}, dsturl)
+
+        if not force:
+            outgoing = discovery.findcommonoutgoing(self._repo, other)
+            if not len(outgoing.missing):
+                self._repo.ui.status(_('nothing to push for %s\n') %
+                    subrepo.subrelpath(self))
+                return True
+
         self._repo.ui.status(_('pushing subrepo %s to %s\n') %
             (subrelpath(self), dsturl))
-        other = hg.peer(self._repo, {'ssh': ssh}, dsturl)
         return self._repo.push(other, force, newbranch=newbranch)
 
     @annotatesubrepoerror