On Thu, Apr 16, 2009 at 11:28 PM, mabshoff <mabsh...@googlemail.com> wrote:
>
> Hi Ondrej,
>
> Why would you say that this bug is introduced in the modified version
> of Sage ships? Just because it fixes the problem for you when you
> remove the patch that does not mean it is the cause (I am not day it
> isn't without looking at the other changes you did for SPD). If you
> could argue what the bug is I think we would be happy to fix it.
I am a bit confused, what is the original pexpect and what is the
modified pexpect for Sage.
In the pexpect Sage package, there is the patch:
patches/pexpect.py-isdir_bug_fix.diff
and the file src/pexpect.py has the patch applied. Did you ship this
patch upstream? Or is it just locally modified pexpect.py for Sage?
Assuming it is locally modified for Sage, then I think the bug is just
in the Sage's pexpect, because clearly the patch is wrong, let me copy
it here again:
$ cat patches/pexpect.py-isdir_bug_fix.diff
--- pexpect.py 2007-04-16 07:08:24.000000000 -0700
+++ ../../pexpect-2.0.p1.0/pexpect-2.0.p1/src/pexpect.py 2009-01-23
01:49:18.000000000 -0800
@@ -1130,7 +1130,7 @@
"""
# Special case where filename already contains a path.
if os.path.dirname(filename) != '':
- if os.access (filename, os.X_OK):
+ if os.access (filename, os.X_OK) and not os.path.isdir(f):
return filename
if not os.environ.has_key('PATH') or os.environ['PATH'] == '':
@@ -1145,7 +1145,7 @@
for path in pathlist:
f = os.path.join(path, filename)
- if os.access(f, os.X_OK):
+ if os.access(f, os.X_OK) and not os.path.isdir(f):
return f
return None
The second hunk is fine, it just adds " and not os.path.isdir(f)" to
the if statement, where "f" is defined on the previous line. The first
hunk is wrong, because it also adds " and not os.path.isdir(f)" to the
if statement, but there the "filename" is a parameter of the which()
function, while "f" is not defined at all, that's why I got:
exceptions.UnboundLocalError: local variable 'f' referenced before assignment
in the traceback above.
Hopefully this makes it clear, that this is a bug, no matter what
other packages I modified. Or do you disagree? If so, what is the
purpose of introducing the undefined variable "f" then?
Ondrej
--~--~---------~--~----~------------~-------~--~----~
To post to this group, send email to sage-devel@googlegroups.com
To unsubscribe from this group, send email to
sage-devel-unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/sage-devel
URLs: http://www.sagemath.org
-~----------~----~----~----~------~----~------~--~---
