Merge lp:~vila/bzr/614713-https-pycurl-activity into lp:bzr

[Vincent Ladeuil]

The issue was the activity tests were failing on gentoo for
*some* curl implementations.

We use a debug callback to track the bytes read/written. This
callback is called with a kind and a text.

Some curl implementations calls our callback for
INFOTYPE_SSL_DATA_OUT and INFOTYPE_SSL_DATA_IN (i.e. the
encrypted text), some don't.

The bug is that we should never take these kinds into account
anyway but until I could reproduce the issue I didn't realize
that.

These kinds will still be reported with -Dhttp for debug purposes.

[John A Meinel (jameinel)]

Merge:approve
This sort of thing seems reasonable to backport.

John
=:->
On Aug 18, 2011 11:17 AM, "Vincent Ladeuil" <email address hidden> wrote:
> Vincent Ladeuil has proposed merging
lp:~vila/bzr/614713-https-pycurl-activity into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #614713 in Bazaar: "selftests fail assertActivitiesMatch for pycurl
compiled against openssl"
> https://bugs.launchpad.net/bzr/+bug/614713
>
> For more details, see:
>
https://code.launchpad.net/~vila/bzr/614713-https-pycurl-activity/+merge/72009
>
> The issue was the activity tests were failing on gentoo for
> *some* curl implementations.
>
> We use a debug callback to track the bytes read/written. This
> callback is called with a kind and a text.
>
> Some curl implementations calls our callback for
> INFOTYPE_SSL_DATA_OUT and INFOTYPE_SSL_DATA_IN (i.e. the
> encrypted text), some don't.
>
> The bug is that we should never take these kinds into account
> anyway but until I could reproduce the issue I didn't realize
> that.
>
> These kinds will still be reported with -Dhttp for debug purposes.
> --
>
https://code.launchpad.net/~vila/bzr/614713-https-pycurl-activity/+merge/72009
> Your team bzr-core is requested to review the proposed merge of
lp:~vila/bzr/614713-https-pycurl-activity into lp:bzr.

=== modified file 'bzrlib/transport/http/_pycurl.py'
--- bzrlib/transport/http/_pycurl.py	2011-06-14 01:26:41 +0000
+++ bzrlib/transport/http/_pycurl.py	2011-08-18 09:16:24 +0000
@@ -327,14 +327,12 @@
                 % (code, msg, plaintext_body))
 
     def _debug_cb(self, kind, text):
-        if kind in (pycurl.INFOTYPE_HEADER_IN, pycurl.INFOTYPE_DATA_IN,
-                    pycurl.INFOTYPE_SSL_DATA_IN):
+        if kind in (pycurl.INFOTYPE_HEADER_IN, pycurl.INFOTYPE_DATA_IN):
             self._report_activity(len(text), 'read')
             if (kind == pycurl.INFOTYPE_HEADER_IN
                 and 'http' in debug.debug_flags):
                 trace.mutter('< %s' % (text.rstrip(),))
-        elif kind in (pycurl.INFOTYPE_HEADER_OUT, pycurl.INFOTYPE_DATA_OUT,
-                      pycurl.INFOTYPE_SSL_DATA_OUT):
+        elif kind in (pycurl.INFOTYPE_HEADER_OUT, pycurl.INFOTYPE_DATA_OUT):
             self._report_activity(len(text), 'write')
             if (kind == pycurl.INFOTYPE_HEADER_OUT
                 and 'http' in debug.debug_flags):
@@ -350,7 +348,9 @@
                         line = '%s: <masked>' % (header,)
                     lines.append(line)
                 trace.mutter('> ' + '\n> '.join(lines))
-        elif kind == pycurl.INFOTYPE_TEXT and 'http' in debug.debug_flags:
+        elif (kind in (pycurl.INFOTYPE_TEXT, pycurl.INFOTYPE_SSL_DATA_IN,
+                       pycurl.INFOTYPE_SSL_DATA_OUT)
+              and 'http' in debug.debug_flags):
             trace.mutter('* %s' % text.rstrip())
 
     def _set_curl_options(self, curl):

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-08-18 04:23:06 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-08-18 09:16:24 +0000
@@ -102,6 +102,9 @@
 * Decode ``BZR_HOME`` with fs encoding on posix platforms to avoid unicode
   errors.  (Vincent Ladeuil, #822571)
 
+* Fix activity reporting for pycurl when using https with some
+  implementations of curl. (Vincent Ladeuil, #614713)
+
 * Fix i18n use when no environment variables are set. (Jelmer Vernooij, #810701)
 
 * TreeTransformBase.fixup_new_roots no longer forces trees to have a root, so