Patchwork localrepository: remove None as default value of path argument in __init__()

login
register
mail settings
Submitter Pulkit Goyal
Date Dec. 4, 2016, 8:35 p.m.
Message ID <14c06a896fd6bb59752e.1480883735@pulkit-goyal>
Download mbox | patch
Permalink /patch/17819/
State Accepted
Headers show

Comments

Pulkit Goyal - Dec. 4, 2016, 8:35 p.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1480873954 -19800
#      Sun Dec 04 23:22:34 2016 +0530
# Node ID 14c06a896fd6bb59752e16ce43c4715b96449d27
# Parent  93275b09d8977ad5700ca56a017f48ce602058ba
localrepository: remove None as default value of path argument in __init__()

The path variable in localrepository.__init__() has a default value None. So
it gives us a option to create an object to localrespository class without
path variable. But things break if you try to do so. The second line in the
init which will be executed when we try to create a localrepository object
will call os.path.expandvars(path) which returns

TypeError: argument of type 'NoneType' is not iterable

I checked occurences when it is called and can't find any piece of code
which calls it without path variable. Also if something is calling it, its
should break.
Jun Wu - Dec. 5, 2016, 12:28 a.m.
Looks good to me.

Excerpts from Pulkit Goyal's message of 2016-12-05 02:05:35 +0530:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1480873954 -19800
> #      Sun Dec 04 23:22:34 2016 +0530
> # Node ID 14c06a896fd6bb59752e16ce43c4715b96449d27
> # Parent  93275b09d8977ad5700ca56a017f48ce602058ba
> localrepository: remove None as default value of path argument in __init__()
> 
> The path variable in localrepository.__init__() has a default value None. So
> it gives us a option to create an object to localrespository class without
> path variable. But things break if you try to do so. The second line in the
> init which will be executed when we try to create a localrepository object
> will call os.path.expandvars(path) which returns
> 
> TypeError: argument of type 'NoneType' is not iterable
> 
> I checked occurences when it is called and can't find any piece of code
> which calls it without path variable. Also if something is calling it, its
> should break.
> 
> diff -r 93275b09d897 -r 14c06a896fd6 mercurial/localrepo.py
> --- a/mercurial/localrepo.py    Thu Dec 01 13:12:04 2016 +0530
> +++ b/mercurial/localrepo.py    Sun Dec 04 23:22:34 2016 +0530
> @@ -249,7 +249,7 @@
>      # only functions defined in module of enabled extensions are invoked
>      featuresetupfuncs = set()
>  
> -    def __init__(self, baseui, path=None, create=False):
> +    def __init__(self, baseui, path, create=False):
>          self.requirements = set()
>          self.wvfs = scmutil.vfs(path, expandpath=True, realpath=True)
>          self.wopener = self.wvfs
Yuya Nishihara - Dec. 5, 2016, 1:17 p.m.
On Mon, 05 Dec 2016 02:05:35 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1480873954 -19800
> #      Sun Dec 04 23:22:34 2016 +0530
> # Node ID 14c06a896fd6bb59752e16ce43c4715b96449d27
> # Parent  93275b09d8977ad5700ca56a017f48ce602058ba
> localrepository: remove None as default value of path argument in __init__()

Good catch. Queued per Jun's review, thanks.

Patch

diff -r 93275b09d897 -r 14c06a896fd6 mercurial/localrepo.py
--- a/mercurial/localrepo.py	Thu Dec 01 13:12:04 2016 +0530
+++ b/mercurial/localrepo.py	Sun Dec 04 23:22:34 2016 +0530
@@ -249,7 +249,7 @@ 
     # only functions defined in module of enabled extensions are invoked
     featuresetupfuncs = set()
 
-    def __init__(self, baseui, path=None, create=False):
+    def __init__(self, baseui, path, create=False):
         self.requirements = set()
         self.wvfs = scmutil.vfs(path, expandpath=True, realpath=True)
         self.wopener = self.wvfs