Patchwork [3,of,7,V3] statprof: require paths to save or load profile data

login
register
mail settings
Submitter Gregory Szorc
Date Nov. 2, 2016, 2:11 a.m.
Message ID <4cd53260f947639e140a.1478052667@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/17266/
State Accepted
Headers show

Comments

Gregory Szorc - Nov. 2, 2016, 2:11 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1471227245 25200
#      Sun Aug 14 19:14:05 2016 -0700
# Node ID 4cd53260f947639e140a61b0698d9da1032d3794
# Parent  4f740964f8843bdcb08a668ae1cf37ef153eba25
statprof: require paths to save or load profile data

Upstream appears to aggressively save statprof data in a well-defined
home directory path. Change the code to not do that.

We also change file saving to fail if an error has occurred
instead of silently failing. Callers can catch the exception.

This behavior is more suitable for a generic "library" module.
Yuya Nishihara - Nov. 2, 2016, 12:45 p.m.
On Tue, 01 Nov 2016 19:11:07 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1471227245 25200
> #      Sun Aug 14 19:14:05 2016 -0700
> # Node ID 4cd53260f947639e140a61b0698d9da1032d3794
> # Parent  4f740964f8843bdcb08a668ae1cf37ef153eba25
> statprof: require paths to save or load profile data

> -def load_data(path=None):
> -    path = path or (os.environ['HOME'] + '/statprof.data')
> +def load_data(path):
>      lines = open(path, 'r').read().splitlines()

Maybe we need to fix main() to not pass None, as a follow-up.

% ./mercurial/statprof.py hotpath
Traceback (most recent call last):
  File "./mercurial/statprof.py", line 803, in <module>
    sys.exit(main())
  File "./mercurial/statprof.py", line 796, in main
    load_data(path=path)
  File "./mercurial/statprof.py", line 338, in load_data
    lines = open(path, 'r').read().splitlines()
TypeError: coercing to Unicode: need string or buffer, NoneType found

Patch

diff --git a/mercurial/statprof.py b/mercurial/statprof.py
--- a/mercurial/statprof.py
+++ b/mercurial/statprof.py
@@ -311,13 +311,11 @@  def stop():
         state.accumulate_time(clock())
         state.last_start_time = None
         statprofpath = os.environ.get('STATPROF_DEST')
-        save_data(statprofpath)
+        if statprofpath:
+            save_data(statprofpath)
 
-def save_data(path=None):
-    try:
-        path = path or (os.environ['HOME'] + '/statprof.data')
-        file = open(path, "w+")
-
+def save_data(path):
+    with open(path, 'w+') as file:
         file.write(str(state.accumulated_time) + '\n')
         for sample in state.samples:
             time = str(sample.time)
@@ -326,13 +324,7 @@  def save_data(path=None):
                      for s in stack]
             file.write(time + '\0' + '\0'.join(sites) + '\n')
 
-        file.close()
-    except (IOError, OSError):
-        # The home directory probably didn't exist, or wasn't writable. Oh well.
-        pass
-
-def load_data(path=None):
-    path = path or (os.environ['HOME'] + '/statprof.data')
+def load_data(path):
     lines = open(path, 'r').read().splitlines()
 
     state.accumulated_time = float(lines[0])