msg44232 - (view) |
Author: Zooko O'Whielacronx (zooko) |
Date: 2003-07-07 01:46 |
There is one bug and one maybe-bug in creation of the coverage file directory in trace.py. The maybe-bug is that it doesn't attempt to create the directory if it got the directory name from the name of the .py file (instead of getting the directory name from the --coverdir command line option). Normally the directory will already exist, but if you are writing out coverage files from a stored report (or, I suppose, if someone deleted the directory after the modules were loaded but before you wrote out the report), then it won't. This patch makes it so that it always attempts to create the directory before attempting to write files into it. The patch also adds a statement to that effect to the "--help" usage info. The other bug is that it attempts to create a directory with: if not os.path.exists(dir): os.makedirs(dir) , which is a race condition that will very rarely raise a spurious exception ("File exists:") if two threads or processes execute it at the same time. The fix provided in this patch is to copy from my pyutil project [1] a utility function that works around this race condition and invoke that function. On request I'll provide a slimmed-down version of that function, since we don't use all of its options in trace.py. This patch hasn't been tested at ALL. In fact, I haven't tested trace.py in general since before it moved from Tools/scripts to Lib/. As this patch ought to go into Python 2.3, it ought to be tested, and I promise to do so soon. [1] http://sf.net/projects/pyutil |
|
|
msg55404 - (view) |
Author: Skip Montanaro (skip.montanaro) *  |
Date: 2007-08-29 01:47 |
Zooko, is this patch still necessary? |
|
|
msg56196 - (view) |
Author: Zooko O'Whielacronx (zooko) |
Date: 2007-09-29 02:46 |
Hi! Sorry it took me so long to look at this. I just checked the source in current trunk, and the relevant code is the same so this patch is still useful. (See the initial post for details.) Here is an updated version of the patch which simply removes some dead code and updates a URL: regards, Zooko diff -rN -u old-up/setuptools-0.6c7/ez_setup.py new-up/ setuptools-0.6c7/ez_setup.py --- old-up/setuptools-0.6c7/ez_setup.py 2007-09-28 16:41:24.000000000 -0600 +++ new-up/setuptools-0.6c7/ez_setup.py 2007-09-28 16:41:25.000000000 -0600 @@ -1,4 +1,4 @@ -#!python +#!/usr/bin/env python """Bootstrap setuptools installation If you want to use setuptools in your package's setup.py, just include this @@ -13,7 +13,7 @@ This file can also be run as a script to install or upgrade setuptools. """ -import sys +import os, re, subprocess, sys DEFAULT_VERSION = "0.6c7" DEFAULT_URL = "http://pypi.python.org/packages/%s/s/setuptools/" % sys.version[:3] @@ -44,8 +44,6 @@ 'setuptools-0.6c6-py2.5.egg': 'b2f8a7520709a5b34f80946de5f02f53', } -import sys, os - def _validate_md5(egg_name, data): if egg_name in md5_data: from md5 import md5 @@ -58,6 +56,42 @@ sys.exit(2) return data +# The following code to parse versions is copied from pkg_resources.py so that +# we can parse versions without importing that module. +component_re = re.compile(r'(\d+ | [a-z]+ |
\. |
-)', re.VERBOSE) +replace = {'pre':'c', 'preview':'c','-':'final-','rc':'c','dev':'@'}.get + +def _parse_version_parts(s): + for part in component_re.split(s): + part = replace(part,part) + if not part or part=='.': + continue + if part[:1] in '0123456789': + yield part.zfill(8) # pad for numeric comparison + else: + yield '*'+part + + yield '*final' # ensure that alpha/beta/candidate are before final + +def parse_version(s): + parts = [] + for part in _parse_version_parts(s.lower()): + if part.startswith('*'): + if part<'*final': # remove '-' before a prerelease tag + while parts and parts[-1]=='*final-': parts.pop() + # remove trailing zeros from each series of numeric parts + while parts and parts[-1]=='00000000': + parts.pop() + parts.append(part) + return tuple(parts) + +def setuptools_is_new_enough(required_version): + """Return True if setuptools is already installed and has a version + number >= required_version.""" + sub = subprocess.Popen([sys.executable, "-c", "import setuptools;print setuptools.__version__"], stdout=subprocess.PIPE) + verstr = sub.stdout.read().strip() + ver = parse_version(verstr) + return ver and ver >= parse_version(required_version) def use_setuptools( version=DEFAULT_VERSION, download_base=DEFAULT_URL, to_dir=os.curdir, @@ -74,32 +108,11 @@ this routine will print a message to ``sys.stderr`` and raise SystemExit in an attempt to abort the calling script. """ - try: - import setuptools - if setuptools.__version__ == '0.0.1': - print >>sys.stderr, ( - "You have an obsolete version of setuptools installed. Please\n" - "remove it from your system entirely before rerunning this script." - ) - sys.exit(2) - except ImportError: + if not setuptools_is_new_enough(version): egg = download_setuptools(version, download_base, to_dir, download_delay) sys.path.insert(0, egg) import setuptools; setuptools.bootstrap_install_from = egg - import pkg_resources - try: - pkg_resources.require("setuptools>="+version) - - except pkg_resources.VersionConflict, e: - # XXX could we install in a subprocess here? - print >>sys.stderr, ( - "The required version of setuptools (>=%s) is not available, and\n" - "can't be installed while this script is running. Please install\n" - " a more recent version first.\n\n(Currently using %r)" - ) % (version, e.args[0]) - sys.exit(2) - def download_setuptools( version=DEFAULT_VERSION, download_base=DEFAULT_URL, to_dir=os.curdir, delay = 15 @@ -150,9 +163,14 @@ def main(argv, version=DEFAULT_VERSION): """Install or upgrade setuptools and EasyInstall""" - try: - import setuptools - except ImportError: + if setuptools_is_new_enough(version): + if argv: + from setuptools.command.easy_install import main + main(argv) + else: + print "Setuptools version",version,"or greater has been installed." + print '(Run "ez_setup.py -U setuptools" to reinstall or upgrade.)' + else: egg = None try: egg = download_setuptools(version, delay=0) @@ -162,31 +180,6 @@ finally: if egg and os.path.exists(egg): os.unlink(egg) - else: - if setuptools.__version__ == '0.0.1': - # tell the user to uninstall obsolete version - use_setuptools(version) - - req = "setuptools>="+version - import pkg_resources - try: - pkg_resources.require(req) - except pkg_resources.VersionConflict: - try: - from setuptools.command.easy_install import main - except ImportError: - from easy_install import main - main(list(argv)+[download_setuptools(delay=0)]) - sys.exit(0) # try to force an exit - else: - if argv: - from setuptools.command.easy_install import main - main(argv) - else: - print "Setuptools version",version,"or greater has been installed." - print '(Run "ez_setup.py -U setuptools" to reinstall or upgrade.)' - - def update_md5(filenames): """Update our built-in md5 registry""" |
msg56197 - (view) |
Author: Zooko O'Whielacronx (zooko) |
Date: 2007-09-29 02:50 |
> Here is an updated version of the patch which simply removes some > dead code and updates a URL: > > regards, > > Zooko > > diff -rN -u old-up/setuptools-0.6c7/ez_setup.py new-up/ > setuptools-0.6c7/ez_setup.py > --- old-up/setuptools-0.6c7/ez_setup.py 2007-09-28 > 16:41:24.000000000 -0600 > +++ new-up/setuptools-0.6c7/ez_setup.py 2007-09-28 > 16:41:25.000000000 -0600 Oops, the in-lined patch contents were a different patch entirely, but the attached patch file was correct. Just for completeness, here is the correct in-lined patch contents: Index: Lib/trace.py =================================================================== --- Lib/trace.py (revision 58282) +++ Lib/trace.py (working copy) @@ -85,7 +85,12 @@ -r, --report Generate a report from a counts file; do not execute any code. `--file' must specify the results file to read, which must have been created in a previous run - with `--count --file=FILE'. + with `--count --file=FILE'. If --coverdir is not + specified, the .cover files will be written into the + directory that the modules were in when the report was + generated. Whether or not --coverdir is specified, + --report will always create the cover file directory if + necessary. Modifiers: -f, --file= File to accumulate counts over several runs. @@ -197,6 +202,33 @@ filename, ext = os.path.splitext(base) return filename +# The following function is copied from the fileutil module from the pyutil +# project: +# http://pypi.python.org/pypi/pyutil +# We use this function instead of os.makedirs() so that we don't get a +# spurious exception when someone else creates the directory at the same +# moment we do. (For example, another thread or process that is also running +# trace.) +def make_dirs(dirname, mode=0777): + """ + A threadsafe and idempotent version of os.makedirs(). If the dir already + exists, do nothing and return without raising an exception. If this call + creates the dir, return without raising an exception. If there is an + error that prevents creation or if the directory gets deleted after + make_dirs() creates it and before make_dirs() checks that it exists, raise + an exception. + """ + tx = None + try: + os.makedirs(dirname, mode) + except OSError, x: + tx = x + + if not os.path.isdir(dirname): + if tx: + raise tx + raise exceptions.IOError, "unknown error prevented creation of directory, or deleted the directory immediately after creation: % s" % dirname # careful not to construct an IOError with a 2-tuple, as that has a special meaning... + class CoverageResults: def __init__(self, counts=None, calledfuncs=None, infile=None, callers=None, outfile=None): @@ -290,15 +322,15 @@ if filename.endswith((".pyc", ".pyo")): filename = filename[:-1] - if coverdir is None: + if coverdir is not None: + dir = coverdir + modulename = fullmodname(filename) + else: dir = os.path.dirname(os.path.abspath(filename)) modulename = modname(filename) - else: - dir = coverdir - if not os.path.exists(dir): - os.makedirs(dir) - modulename = fullmodname(filename) + make_dirs(dir) + # If desired, get a list of the line numbers which represent # executable content (returned as a dict for better lookup speed) if show_missing: |
|
|
msg58923 - (view) |
Author: Isaac Morland (ijmorlan) |
Date: 2007-12-20 20:02 |
I would suggest that the need to create directories that may already exist (really "ensure existence of" directories) is not exclusive to trace.py. I am suggesting this be added as an option to os.mkdir and os.makedirs. See Issue 1675. |
|
|
msg65432 - (view) |
Author: Skip Montanaro (skip.montanaro) *  |
Date: 2008-04-13 03:28 |
unassigning |
|
|
msg114249 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-08-18 17:15 |
Is there any interest in this issue? |
|
|
msg123005 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2010-12-01 18:51 |
Eli, Would you like to review this patch? |
|
|
msg123319 - (view) |
Author: Eli Bendersky (eli.bendersky) *  |
Date: 2010-12-04 08:33 |
Alexander, I reviewed the patch and ported the changes to the newest sources (since the fix to issue 9299, os.makedirs can be naturally used with its new flag to fix the bug Zooko refers to). However, while experimenting, I think I ran into much larger problems. Either that or I've forgotten how to use the module :-) Attaching two files (one imports the other) on which I try to run the following: python -m trace -c trace_target.py >> OK: I get trace_target.cover & traced_module.cover created However, now running: python -m trace -r --file=trace_target.cover >> ... pickle.load(open(self.infile, 'rb')) _pickle.UnpicklingError: invalid load key, ' '. Also, trying to provide --file to -c: python -m trace -c trace_target.py --file=xyz.cover >> xyz.cover is ignored and the same two .cover files are created. Can you take a look at this? |
|
|
msg126113 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2011-01-12 17:17 |
On Sat, Dec 4, 2010 at 3:34 AM, Eli Bendersky <report@bugs.python.org> wrote: >.. > However, while experimenting, I think I ran into much larger problems. Either that or I've forgotten > how to use the module :-) I am afraid it is the latter. :-) The file specified in --file option should be a pickle, not a coverage file from the previous run. $ ./python.exe -m trace -s -f counts.pickle -c trace_target.py K is 380 Skipping counts file 'counts.pickle': [Errno 2] No such file or directory: 'counts.pickle' lines cov% module (path) 1 100% trace (/Users/sasha/Work/python-svn/py3k-commit/Lib/trace.py) 9 100% trace_target (trace_target.py) 6 100% traced_module (traced_module.py) $ ./python.exe -m pickletools counts.pickle 0: ( MARK 1: } EMPTY_DICT 2: q BINPUT 0 4: ( MARK 5: ( MARK 6: X BINUNICODE 'trace_target.py' ... However, there is a problem here, I think: $ ./python.exe -m trace -s -f counts.pickle -c trace_target.py K is 380 lines cov% module (path) 1 100% trace (/Users/sasha/Work/python-svn/py3k-commit/Lib/trace.py) 9 100% trace_target (trace_target.py) 6 100% traced_module (traced_module.py) $ ./python.exe -m trace -s -f counts.pickle -c trace_target.py K is 380 lines cov% module (path) 1 100% trace (/Users/sasha/Work/python-svn/py3k-commit/Lib/trace.py) 9 100% trace_target (trace_target.py) 6 100% traced_module (traced_module.py) The counts should grow in repeated runs. |
|
|
msg126154 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2011-01-13 01:31 |
On Wed, Jan 12, 2011 at 12:17 PM, Alexander Belopolsky <report@bugs.python.org> wrote: .. > The counts should grow in repeated runs. I did not pay attention: the numbers in summary are numbers of lines, not execution counts. The execution counts in .cover file grow as expected. |
|
|
msg126155 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2011-01-13 01:38 |
On Sat, Dec 4, 2010 at 3:34 AM, Eli Bendersky <report@bugs.python.org> wrote: .. > I reviewed the patch and ported the changes to the newest sources Eli, I don't think you ever posted an updated patch. Do you still have it? This may be a good starting issue for you as a committer. |
|
|
msg190040 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2013-05-26 00:28 |
Just a gentle bump. |
|
|
msg190088 - (view) |
Author: Eli Bendersky (eli.bendersky) *  |
Date: 2013-05-26 13:30 |
Ah, no time, no time... :-/ I may get back to this in the future. Bumping to more relevant versions for now. |
|
|