Patchwork [2,of,6] dirstate: use open/read of vfs(opener) explicitly instead of read

login
register
mail settings
Submitter Katsunori FUJIWARA
Date May 19, 2015, 4:42 p.m.
Message ID <2d2eff9ee053025e8e02.1432053722@feefifofum>
Download mbox | patch
Permalink /patch/9179/
State Accepted
Delegated to: Pierre-Yves David
Headers show

Comments

Katsunori FUJIWARA - May 19, 2015, 4:42 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1432051569 -32400
#      Wed May 20 01:06:09 2015 +0900
# Node ID 2d2eff9ee053025e8e0267b8d6c6d07326f607ee
# Parent  9832a882c21e23139b6bda2b5004c4253b3b35df
dirstate: use open/read of vfs(opener) explicitly instead of read

This simplifies changes in subsequent patch, which tries to open
`.pending` file when HG_PENDING environment variable is defined.
Augie Fackler - May 19, 2015, 11:28 p.m.
On Wed, May 20, 2015 at 01:42:02AM +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1432051569 -32400
> #      Wed May 20 01:06:09 2015 +0900
> # Node ID 2d2eff9ee053025e8e0267b8d6c6d07326f607ee
> # Parent  9832a882c21e23139b6bda2b5004c4253b3b35df
> dirstate: use open/read of vfs(opener) explicitly instead of read
>
> This simplifies changes in subsequent patch, which tries to open
> `.pending` file when HG_PENDING environment variable is defined.
>
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -320,7 +320,11 @@
>          self._map = {}
>          self._copymap = {}
>          try:
> -            st = self._opener.read(self._filename)
> +            fp = self._opener.open(self._filename)
> +            try:
> +                st = fp.read()
> +            finally:
> +                fp.close()

This try/finally could be written as:

with self._opener.open(self._filename) as fp:
  st = fp.read()

and then the finally happens for free.

(I don't feel strongly, so don't do a resend for just this - just
thought I'd mention that it can be done that way.)

>          except IOError, err:
>              if err.errno != errno.ENOENT:
>                  raise
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - May 20, 2015, 3:13 a.m.
On 05/19/2015 11:42 AM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1432051569 -32400
> #      Wed May 20 01:06:09 2015 +0900
> # Node ID 2d2eff9ee053025e8e0267b8d6c6d07326f607ee
> # Parent  9832a882c21e23139b6bda2b5004c4253b3b35df
> dirstate: use open/read of vfs(opener) explicitly instead of read
>
> This simplifies changes in subsequent patch, which tries to open
> `.pending` file when HG_PENDING environment variable is defined.

The first two are pushed to the clowncopter.

I agree with augies "with" remark, I'm not sure if this actually works 
today, but if it's not we should get it to work and use it!

I've various reservation of the other 4, I'll write more about them 
soon, but probably tomorrow.

Regards

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -320,7 +320,11 @@ 
         self._map = {}
         self._copymap = {}
         try:
-            st = self._opener.read(self._filename)
+            fp = self._opener.open(self._filename)
+            try:
+                st = fp.read()
+            finally:
+                fp.close()
         except IOError, err:
             if err.errno != errno.ENOENT:
                 raise