| Submitter | Matt Harbison |
|---|---|
| Date | March 6, 2015, 1:43 a.m. |
| Message ID | <0828a7e7f402374dca9c.1425606212@Envy> |
| Download | mbox | patch |
| Permalink | /patch/7906/ |
| State | Changes Requested |
| Delegated to: | Pierre-Yves David |
| Headers | show |
Comments
On 03/05/2015 05:43 PM, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1425603727 18000 > # Thu Mar 05 20:02:07 2015 -0500 > # Branch stable > # Node ID 0828a7e7f402374dca9cc089a9ff958fb478163c > # Parent 1e7c8046a9f461bda3c8f6003dae65e235af8bb6 > evolve: prevent a crash in httpclient_pushobsmarkers() when pushing Pushing to main with an eyes brown raised.
On Mon, 09 Mar 2015 22:50:53 -0400, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > > On 03/05/2015 05:43 PM, Matt Harbison wrote: >> # HG changeset patch >> # User Matt Harbison <matt_harbison@yahoo.com> >> # Date 1425603727 18000 >> # Thu Mar 05 20:02:07 2015 -0500 >> # Branch stable >> # Node ID 0828a7e7f402374dca9cc089a9ff958fb478163c >> # Parent 1e7c8046a9f461bda3c8f6003dae65e235af8bb6 >> evolve: prevent a crash in httpclient_pushobsmarkers() when pushing > > Pushing to main with an eyes brown raised. Given the previous code, I can't believe it used to work at all over http, even though the test says it does. Any thoughts? Any debug steps you think would be useful on this repo to see why it is different? --Matt
On 03/09/2015 07:59 PM, Matt Harbison wrote: > On Mon, 09 Mar 2015 22:50:53 -0400, Pierre-Yves David > <pierre-yves.david@ens-lyon.org> wrote: > >> >> >> On 03/05/2015 05:43 PM, Matt Harbison wrote: >>> # HG changeset patch >>> # User Matt Harbison <matt_harbison@yahoo.com> >>> # Date 1425603727 18000 >>> # Thu Mar 05 20:02:07 2015 -0500 >>> # Branch stable >>> # Node ID 0828a7e7f402374dca9cc089a9ff958fb478163c >>> # Parent 1e7c8046a9f461bda3c8f6003dae65e235af8bb6 >>> evolve: prevent a crash in httpclient_pushobsmarkers() when pushing >> >> Pushing to main with an eyes brown raised. > > Given the previous code, I can't believe it used to work at all over > http, even though the test says it does. Any thoughts? Any debug steps > you think would be useful on this repo to see why it is different? The tests are supposed to ensure that. If the http push is actually untested, making it tested sounds like a first step.
On Mon, 09 Mar 2015 23:13:39 -0400, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > > On 03/09/2015 07:59 PM, Matt Harbison wrote: >> On Mon, 09 Mar 2015 22:50:53 -0400, Pierre-Yves David >> <pierre-yves.david@ens-lyon.org> wrote: >> >>> >>> >>> On 03/05/2015 05:43 PM, Matt Harbison wrote: >>>> # HG changeset patch >>>> # User Matt Harbison <matt_harbison@yahoo.com> >>>> # Date 1425603727 18000 >>>> # Thu Mar 05 20:02:07 2015 -0500 >>>> # Branch stable >>>> # Node ID 0828a7e7f402374dca9cc089a9ff958fb478163c >>>> # Parent 1e7c8046a9f461bda3c8f6003dae65e235af8bb6 >>>> evolve: prevent a crash in httpclient_pushobsmarkers() when pushing >>> >>> Pushing to main with an eyes brown raised. >> >> Given the previous code, I can't believe it used to work at all over >> http, even though the test says it does. Any thoughts? Any debug steps >> you think would be useful on this repo to see why it is different? > > > The tests are supposed to ensure that. If the http push is actually > untested, making it tested sounds like a first step. I think test-simple4server.t, line 90 covers it: $ hg push pushing to http://localhost:$HGPORT/ searching for changes remote: adding changesets remote: adding manifests remote: adding file changes remote: added 1 changesets with 1 changes to 1 files (+1 heads) pushing 2 obsolescence markers (* bytes) (glob) I can't believe specifying a path or revision like I did in the commit message demo makes any difference. The code looked like it would fail any time there was a "pushing xx obsolete markers" line, so I must be missing something. --Matt
On 03/09/2015 08:48 PM, Matt Harbison wrote: > On Mon, 09 Mar 2015 23:13:39 -0400, Pierre-Yves David > <pierre-yves.david@ens-lyon.org> wrote: > >> >> >> On 03/09/2015 07:59 PM, Matt Harbison wrote: >>> On Mon, 09 Mar 2015 22:50:53 -0400, Pierre-Yves David >>> <pierre-yves.david@ens-lyon.org> wrote: >>> >>>> >>>> >>>> On 03/05/2015 05:43 PM, Matt Harbison wrote: >>>>> # HG changeset patch >>>>> # User Matt Harbison <matt_harbison@yahoo.com> >>>>> # Date 1425603727 18000 >>>>> # Thu Mar 05 20:02:07 2015 -0500 >>>>> # Branch stable >>>>> # Node ID 0828a7e7f402374dca9cc089a9ff958fb478163c >>>>> # Parent 1e7c8046a9f461bda3c8f6003dae65e235af8bb6 >>>>> evolve: prevent a crash in httpclient_pushobsmarkers() when pushing >>>> >>>> Pushing to main with an eyes brown raised. >>> >>> Given the previous code, I can't believe it used to work at all over >>> http, even though the test says it does. Any thoughts? Any debug steps >>> you think would be useful on this repo to see why it is different? >> >> >> The tests are supposed to ensure that. If the http push is actually >> untested, making it tested sounds like a first step. > > I think test-simple4server.t, line 90 covers it: > > $ hg push > pushing to http://localhost:$HGPORT/ > searching for changes > remote: adding changesets > remote: adding manifests > remote: adding file changes > remote: added 1 changesets with 1 changes to 1 files (+1 heads) > pushing 2 obsolescence markers (* bytes) (glob) > > I can't believe specifying a path or revision like I did in the commit > message demo makes any difference. The code looked like it would fail > any time there was a "pushing xx obsolete markers" line, so I must be > missing something. Your initial message is talking about situation when no changeset are to be pushed. This might be related ?
On Tue, 10 Mar 2015 00:10:11 -0400, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > > On 03/09/2015 08:48 PM, Matt Harbison wrote: >> On Mon, 09 Mar 2015 23:13:39 -0400, Pierre-Yves David >> <pierre-yves.david@ens-lyon.org> wrote: >> >>> >>> >>> On 03/09/2015 07:59 PM, Matt Harbison wrote: >>>> On Mon, 09 Mar 2015 22:50:53 -0400, Pierre-Yves David >>>> <pierre-yves.david@ens-lyon.org> wrote: >>>> >>>>> >>>>> >>>>> On 03/05/2015 05:43 PM, Matt Harbison wrote: >>>>>> # HG changeset patch >>>>>> # User Matt Harbison <matt_harbison@yahoo.com> >>>>>> # Date 1425603727 18000 >>>>>> # Thu Mar 05 20:02:07 2015 -0500 >>>>>> # Branch stable >>>>>> # Node ID 0828a7e7f402374dca9cc089a9ff958fb478163c >>>>>> # Parent 1e7c8046a9f461bda3c8f6003dae65e235af8bb6 >>>>>> evolve: prevent a crash in httpclient_pushobsmarkers() when pushing >>>>> >>>>> Pushing to main with an eyes brown raised. >>>> >>>> Given the previous code, I can't believe it used to work at all over >>>> http, even though the test says it does. Any thoughts? Any debug >>>> steps >>>> you think would be useful on this repo to see why it is different? >>> >>> >>> The tests are supposed to ensure that. If the http push is actually >>> untested, making it tested sounds like a first step. >> >> I think test-simple4server.t, line 90 covers it: >> >> $ hg push >> pushing to http://localhost:$HGPORT/ >> searching for changes >> remote: adding changesets >> remote: adding manifests >> remote: adding file changes >> remote: added 1 changesets with 1 changes to 1 files (+1 heads) >> pushing 2 obsolescence markers (* bytes) (glob) >> >> I can't believe specifying a path or revision like I did in the commit >> message demo makes any difference. The code looked like it would fail >> any time there was a "pushing xx obsolete markers" line, so I must be >> missing something. > > Your initial message is talking about situation when no changeset are to > be pushed. This might be related ? I see what is going on here. In the test case, the string that was unpacked into "ret, output" was '0\n'. In my case when there was a problem, the string being unpacked is '0\n8 obsolescence markers added'. I'll see if I can put together a test case when I get a chance. --Matt
Patch
diff --git a/hgext/evolve.py b/hgext/evolve.py --- a/hgext/evolve.py +++ b/hgext/evolve.py @@ -2519,11 +2519,20 @@ (Cannot simply use _callpush as http is doing some special handling)""" self.requirecap('_evoext_pushobsmarkers_0', _('push obsolete markers faster')) - ret, output = self._call('evoext_pushobsmarkers_0', data=obsfile) - for l in output.splitlines(True): - if l.strip(): - self.ui.status(_('remote: '), l) - return ret + try: + r = self._call('evoext_pushobsmarkers_0', data=obsfile) + vals = r.split('\n', 1) + if len(vals) < 2: + raise error.ResponseError(_("unexpected response:"), r) + + for l in vals[1].splitlines(True): + if l.strip(): + self.ui.status(_('remote: '), l) + return vals[0] + except socket.error, err: + if err.args[0] in (errno.ECONNRESET, errno.EPIPE): + raise util.Abort(_('push failed: %s') % err.args[1]) + raise util.Abort(err.args[1]) @eh.wrapfunction(localrepo.localrepository, '_restrictcapabilities') def local_pushobsmarker_capabilities(orig, repo, caps):
# HG changeset patch # User Matt Harbison <matt_harbison@yahoo.com> # Date 1425603727 18000 # Thu Mar 05 20:02:07 2015 -0500 # Branch stable # Node ID 0828a7e7f402374dca9cc089a9ff958fb478163c # Parent 1e7c8046a9f461bda3c8f6003dae65e235af8bb6 evolve: prevent a crash in httpclient_pushobsmarkers() when pushing I've been running into a crash when pushing from my hg repo in a Fedora 16 VM to Win7 running 'hg serve', even with extensions disabled on both sides: ../hg push -r . pc pushing to http://192.168.1.4:8000/ searching for changes no changes found pushing 2 obsolescence markers (263 bytes) ** unknown exception encountered, please report by visiting ... File "hg-evolve/hgext/evolve.py", line 2482, in _pushobsolete remote.evoext_pushobsmarkers_0(obsdata) File "hg-evolve/hgext/evolve.py", line 2522, in httpclient_pushobsmarkers ret, output = self._call('evoext_pushobsmarkers_0', data=obsfile) ValueError: too many values to unpack I'm not sure how this repo differs from the one in the test suite, so I'm not sure how to craft a test for this. The failure occurs even when there _are_ csets to push. There was no crash if no obsolete markers needed to be pushed. At any rate, this code was stolen from httppeer._callpush(), where it calls self._call(). The socket exception handling wasn't necessary to fix the crash, but the calling code might as well be duplicated in its entirety. A successful push with this patch looks like this. Note the final line is _not_ in the output of the http push in test-simple4server.t: ../hg push -r . pc pushing to http://192.168.1.4:8000/ searching for changes remote has heads on branch 'default' that are not known locally: 3af110194a0c 56000e3ae44d 57ac6e51d290 7da4355c21b8 and 8 others remote: adding changesets remote: adding manifests remote: adding file changes remote: added 1 changesets with 0 changes to 1 files (+1 heads) pushing 4 obsolescence markers (525 bytes) remote: 2 obsolescence markers added