Patchwork convert: move svn config initializer out of the module level

login
register
mail settings
Submitter Durham Goode
Date Aug. 2, 2016, 12:43 a.m.
Message ID <8f033343d96b42e87d90.1470098638@dev8486.prn1.facebook.com>
Download mbox | patch
Permalink /patch/16024/
State Accepted
Headers show

Comments

Durham Goode - Aug. 2, 2016, 12:43 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1470098281 25200
#      Mon Aug 01 17:38:01 2016 -0700
# Node ID 8f033343d96b42e87d90cac108ef8256a0797b27
# Parent  73ff159923c1f05899c27238409ca398342d9ae0
convert: move svn config initializer out of the module level

The svn_config_get_config config call was being called at the module level, but
had the potential to throw permission denied errors if ~/.subversion/servers was
not readable. This could happen in certain test environments where the user
permissions were very particular.

This prevented the remotenames extension from loading, since it imports
convert's hg module, which imports convert's subversion module, which calls
this. The config is only ever used from this one constructor, so let's just move
it in to there.
Yuya Nishihara - Aug. 2, 2016, 12:21 p.m.
On Mon, 1 Aug 2016 17:43:58 -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1470098281 25200
> #      Mon Aug 01 17:38:01 2016 -0700
> # Node ID 8f033343d96b42e87d90cac108ef8256a0797b27
> # Parent  73ff159923c1f05899c27238409ca398342d9ae0
> convert: move svn config initializer out of the module level
> 
> The svn_config_get_config config call was being called at the module level, but
> had the potential to throw permission denied errors if ~/.subversion/servers was
> not readable. This could happen in certain test environments where the user
> permissions were very particular.

LGTM. Perhaps we'd better put the fix in stable?
Augie Fackler - Aug. 2, 2016, 2:52 p.m.
On Tue, Aug 02, 2016 at 09:21:53PM +0900, Yuya Nishihara wrote:
> On Mon, 1 Aug 2016 17:43:58 -0700, Durham Goode wrote:
> > # HG changeset patch
> > # User Durham Goode <durham@fb.com>
> > # Date 1470098281 25200
> > #      Mon Aug 01 17:38:01 2016 -0700
> > # Node ID 8f033343d96b42e87d90cac108ef8256a0797b27
> > # Parent  73ff159923c1f05899c27238409ca398342d9ae0
> > convert: move svn config initializer out of the module level
> >
> > The svn_config_get_config config call was being called at the module level, but
> > had the potential to throw permission denied errors if ~/.subversion/servers was
> > not readable. This could happen in certain test environments where the user
> > permissions were very particular.
>
> LGTM. Perhaps we'd better put the fix in stable?

Sounds fine to me. Queued for stable.


> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/convert/transport.py b/hgext/convert/transport.py
--- a/hgext/convert/transport.py
+++ b/hgext/convert/transport.py
@@ -34,8 +34,7 @@  from mercurial import (
 # won't work worth a darn against those libraries anyway!
 svn.ra.initialize()
 
-svn_config = svn.core.svn_config_get_config(None)
-
+svn_config = None
 
 def _create_auth_baton(pool):
     """Create a Subversion authentication baton. """
@@ -88,6 +87,9 @@  class SvnRaTransport(object):
                 svn.core.svn_auth_set_parameter(
                     ab, svn.core.SVN_AUTH_PARAM_DEFAULT_PASSWORD, self.password)
             self.client.auth_baton = ab
+            global svn_config
+            if svn_config is None:
+                svn_config = svn.core.svn_config_get_config(None)
             self.client.config = svn_config
             try:
                 self.ra = svn.client.open_ra_session(