should avoid loading modules from working directory

Good idea, thanks.

--
Martin

On Fri, Nov 17, 2006 at 11:23:51PM -0000, Martin Pool wrote:
> Good idea, thanks.

Sure. You probably want something like this:

--------------------------------------------------------------------------------
import os, sys

cwd = os.getcwd()
if not (cwd == os.path.dirname(os.__file__))):
    sys.path.remove(cwd)
sys.path.remove('')
--------------------------------------------------------------------------------

Not to tell you what to do :) Just felt bad for recommending the wrong option
before.

-Forest

marking inprogress as there's a suggested fix

I'm pretty sure this code is wrong:

cwd = os.getcwd()
if not (cwd == os.path.dirname(os.__file__))):
    sys.path.remove(cwd)

I think you want:

cwd = os.getcwd()
if not (cwd == os.path.dirname(__file__))):
    sys.path.remove(cwd)

I'm don't understand why you would want the path to the 'os' module.

This does introduce some edge cases, like doing:

./bzr selftest

When in a directory that has a symlink in its path. I suppose you could do:

cwd = os.path.realpath(os.getcwd())
if os.path.realpath(os.path.dirname(__file__) != cwd:

to handle symlinks.

You also should be using "os.getcwdu()" on Windows in case one of the path segments has characters that aren't valid in your current codepage. Since otherwise you get "????" paths. I'm *not* sure what something like os.path.realpath() would do in that case, but there isn't really any reason that we should fail because of it. (Similarly, we should probably be using os.getcwd() on other systems, because you may be in a directory structure that isn't valid in your current fs encoding. os.getcwdu() will fail if it can't decode all the parts of the path.)

I'll note that the *actual* bug is in the specific site's default search path. Which hints towards an install issue outside of bzr's control. I would guess that adding something like this is a reasonable workaround for broken installs (as they seem to be fairly common), I just caution that applying a bandaid can introduce bugs at the same time.

Actually, Python's default search path always includes the current directory (up until 2.6 or so, I think).

It was a long time ago that I suggested the above fix. I think that the reasoning was that I was trying to avoid removing the standard library, but the whole thing looks a bit half-baked to me now.

I think sys.path.remove('') would probably be sufficient...

Bumping importance because of bug #304891.

Fixing bzr itself souldn't be that hard but there may be consequences on packaging it or running it locally (as ./bzr).

I am unable to reproduce this on Ubunty Jaunty, using either the bzr from the official Jaunty repository (1.13.1-1) or the current development version. I've tried the following procedure with both python 2.5.4-1ubuntu4 and 2.6.2-0ubuntu1:

hmeland@octarine:~$ mkdir /tmp/,72227/
hmeland@octarine:~$ cd /tmp/,72227/
hmeland@octarine:/tmp/,72227$ /usr/bin/python2.5 /usr/bin/bzr --no-plugins init
Created a standalone tree (format: pack-0.92)
hmeland@octarine:/tmp/,72227$ mkdir -p foo/codecs
hmeland@octarine:/tmp/,72227$ touch foo/codecs/__init__.py
hmeland@octarine:/tmp/,72227$ cd foo
hmeland@octarine:/tmp/,72227/foo$ /usr/bin/python2.5 /usr/bin/bzr --no-plugins add
adding foo
adding foo/codecs
adding foo/codecs/__init__.py

I've also tried a similar procedure using the standalone 1.16.1-3 installer on a virtual Windows XP box. I was unable to reproduce the bug there, too.

FWIW, the behaviour I'm seeing (i.e. not being able to reproduce the bug) seems to correspond well with Python's documentation (http://docs.python.org/library/sys.html#sys.path): sys.path[0] should only be an empty string when "the script directory is not available":

hmeland@octarine:/tmp/,72227/foo$ cat > test.py
#!/usr/bin/env python
import sys
import pprint
pprint.pprint(sys.path)
hmeland@octarine:/tmp/,72227/foo$ chmod +x test.py
hmeland@octarine:/tmp/,72227/foo$ ./test.py
['/tmp/,72227/foo',
 '/usr/lib/python2.6',
 '/usr/lib/python2.6/plat-linux2',
 '/usr/lib/python2.6/lib-tk',
 '/usr/lib/python2.6/lib-old',
 '/usr/lib/python2.6/lib-dynload',
 '/usr/lib/python2.6/dist-packages',
 '/usr/lib/python2.6/dist-packages/Numeric',
 '/usr/lib/python2.6/dist-packages/PIL',
 '/usr/lib/python2.6/dist-packages/gst-0.10',
 '/var/lib/python-support/python2.6',
 '/usr/lib/python2.6/dist-packages/gtk-2.0',
 '/var/lib/python-support/python2.6/gtk-2.0',
 '/usr/lib/python2.6/dist-packages/wx-2.8-gtk2-unicode',
 '/usr/local/lib/python2.6/dist-packages']

i.e. when I invoke a Python script on my system, there is no empty string element in sys.path.

Does anyone know how to reliably reproduce this bug?

I think at one point the site-packages put '' in sys.path; also people
can often accidentally have '' in their PYTHONPATH.

I'm inclined to mark this invalid - as its not a bzr bug but rather an
environmental one.

-Rob

[~]
06:45 forest@W038$ touch logging.py
[~]
06:46 forest@W038$ bzr stat
Traceback (most recent call last):
  File "/usr/bin/bzr", line 125, in <module>
    import bzrlib.trace
  File "/usr/lib/python2.6/dist-packages/bzrlib/trace.py", line 106, in <module>
    _bzr_logger = logging.getLogger('bzr')
AttributeError: 'module' object has no attribute 'getLogger'

Some of the earlier comments rather confuse the point. As the bug title says, bzr "should avoid loading modules from working directory," whether that is the result of '' being in sys.path or otherwise.

On Thu, 2009-07-16 at 10:48 +0000, Forest Bond wrote:
> Some of the earlier comments rather confuse the point. As the bug title
> says, bzr "should avoid loading modules from working directory," whether
> that is the result of '' being in sys.path or otherwise.

bzr needs to load them when running from source :).

I'll think about this more tomorrow, internet dragons to kill right now!

-Rob

On Thu, Jul 16, 2009 at 12:47 PM, Forest Bond<email address hidden> wrote:
> [~]
> 06:45 forest@W038$ touch logging.py
> [~]
> 06:46 forest@W038$ bzr stat
> Traceback (most recent call last):
>  File "/usr/bin/bzr", line 125, in <module>
>    import bzrlib.trace
>  File "/usr/lib/python2.6/dist-packages/bzrlib/trace.py", line 106, in <module>
>    _bzr_logger = logging.getLogger('bzr')
> AttributeError: 'module' object has no attribute 'getLogger'

I can't reproduce this, either. Forest, what does the output of the
"test.py" script at the end of my first message look like on your
system?
--
Harald

Harald,

I'm afraid what you're saying isn't adding up. I can see from your test script that the CWD is in your sys.path. That clearly indicates that modules in your CWD should get imported.

Why don't you tell me what you're doing to reproduce, and I'll see if I can help you figure out what the problem is.

Thanks,
Forest

On Thu, Jul 16, 2009 at 2:36 PM, Forest Bond<email address hidden> wrote:
> Harald,
>
> I'm afraid what you're saying isn't adding up.  I can see from your test
> script that the CWD is in your sys.path.  That clearly indicates that
> modules in your CWD should get imported.

The _script directory_ is in my sys.path. When I run my test script
as "./test.py", the script directory and the CWD coincide. However,
as my bzr typically isn't run as "./bzr" when I'm in danger of
triggering this bug, the CWD will not be in my sys.path.

What I'm wondering is, does the test.py script, when run on your
system, indicate the absolute path to the script directory is in
sys.path, or is there an empty-string element in your sys.path? And,
if there is any empty-string elements there, is the script directory
there, as well? And finally, is the absolute path to the CWD somehow
included in your sys.path?

> Why don't you tell me what you're doing to reproduce, and I'll see if I
> can help you figure out what the problem is.

I performed the exact same steps that you indicated:

hmeland@octarine:/tmp/,72227$ touch logging.py
hmeland@octarine:/tmp/,72227$ bzr stat
unknown:
  logging.py

... and as you can see, I was unable to reproduce the bug in this way, too.

My thinking is that if the fix for this issue is as simple as removing
any empty-string elements from sys.path (on Python installations that
somehow isn't conforming to the sys.path documentation I linked to),
then that should be easy to fix.

If, on the other hand, your python somehow includes one or more
absolute path to directories that we don't really want to import
modules from, the fix might have to be more involved (one possible
remedy in this case might be to run Python with the -S command-line
option).
--
Harald

Howdie,

On Thu, Jul 16, 2009 at 01:30:42PM -0000, Harald Meland wrote:
> On Thu, Jul 16, 2009 at 2:36 PM, Forest Bond<email address hidden> wrote:
> > Harald,
> >
> > I'm afraid what you're saying isn't adding up.  I can see from your test
> > script that the CWD is in your sys.path.  That clearly indicates that
> > modules in your CWD should get imported.
>
> The _script directory_ is in my sys.path. When I run my test script
> as "./test.py", the script directory and the CWD coincide. However,
> as my bzr typically isn't run as "./bzr" when I'm in danger of
> triggering this bug, the CWD will not be in my sys.path.

... then ./test.py isn't very helpful in diagnosing the problem.

[~]
09:54 forest@W038$ mkdir /tmp/foo
[~]
09:54 forest@W038$ echo 'import sys; print sys.path' >/tmp/foo/test.py
[~]
09:54 forest@W038$ mkdir /tmp/bar
[~]
09:54 forest@W038$ cd /tmp/bar
[/tmp/bar]
09:54 forest@W038$ python /tmp/foo/test.py
['/tmp/foo', '/home/forest/usr/lib/python2.5/site-packages/setuptools-0.6c9-py2.5.egg', '/home/forest/usr/lib/python2.5/site-packages/Satchmo-0.9_pre-py2.5.egg', '/home/forest/lib/python', '/home/forest/usr/lib/python2.6/site-packages', '/home/forest/usr/lib/python2.5/site-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages', '/tmp/bar', '/usr/lib/python2.6', '/usr/lib/python2.6/plat-linux2', '/usr/lib/python2.6/lib-tk', '/usr/lib/python2.6/lib-old', '/usr/lib/python2.6/lib-dynload', '/usr/lib/python2.6/dist-packages', '/usr/lib/python2.6/dist-packages/PIL', '/usr/lib/python2.6/dist-packages/gst-0.10', '/var/lib/python-support/python2.6', '/usr/lib/python2.6/dist-packages/gtk-2.0', '/var/lib/python-support/python2.6/gtk-2.0', '/usr/lib/python2.6/dist-packages/wx-2.8-gtk2-unicode', '/usr/local/lib/python2.6/dist-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages']
[/tmp/bar]
09:54 forest@W038$ python -S /tmp/foo/test.py
['/tmp/foo', '/home/forest/lib/python', '/home/forest/usr/lib/python2.6/site-packages', '/home/forest/usr/lib/python2.5/site-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages', '', '/usr/lib/python2.6/', '/usr/lib/python2.6/plat-linux2', '/usr/lib/python2.6/lib-tk', '/usr/lib/python2.6/lib-old', '/usr/lib/python2.6/lib-dynload']

> My thinking is that if the fix for this issue is as simple as removing
> any empty-string elements from sys.path (on Python installations that
> somehow isn't conforming to the sys.path documentation I linked to),
> then that should be easy to fix.
>
> If, on the other hand, your python somehow includes one or more
> absolute path to directories that we don't really want to import
> modules from, the fix might have to be more involved (one possible
> remedy in this case might be to run Python with the -S command-line
> option).

-S isn't great. What if I have bzr plugins installed paths set up by the site
module (likely)?

Thanks,
Forest
--
Forest Bond
http://www.alittletooquiet.net
http://www.pytagsfs....

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

> 09:54 forest@W038$ python /tmp/foo/test.py
> ['/tmp/foo', '/home/forest/usr/lib/python2.5/site-packages/setuptools-0.6c9-py2.5.egg', '/home/forest/usr/lib/python2.5/site-packages/Satchmo-0.9_pre-py2.5.egg', '/home/forest/lib/python', '/home/forest/usr/lib/python2.6/site-packages', '/home/forest/usr/lib/python2.5/site-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages',
> '/tmp/bar', '/usr/lib/python2.6', '/usr/lib/python2.6/plat-linux2', '/usr/lib/python2.6/lib-tk', '/usr/lib/python2.6/lib-old', '/usr/lib/python2.6/lib-dynload', '/usr/lib/python2.6/dist-packages', '/usr/lib/python2.6/dist-packages/PIL', '/usr/lib/python2.6/dist-packages/gst-0.10', '/var/lib/python-support/python2.6', '/usr/lib/python2.6/dist-packages/gtk-2.0', '/var/lib/python-support/python2.6/gtk-2.0', '/usr/lib/python2.6/dist-packages/wx-2.8-gtk2-unicode', '/usr/local/lib/python2.6/dist-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages']

> [/tmp/bar]

^- So indeed, /tmp/bar is in your path.

What I've seen in the past, is that if you insert debug statements into
*site.py* it gets a '' in sys.path, and then it calls a function to turn
all paths in 'sys.path' into absolute paths (and thus '' => $PWD).

Obviously you are getting $PWD into your path somehow, the question is
figuring out how.

For bzrlib, one possibility could be to do:

cwd = os.getcwd()
bzr_path = os.path.dirname(sys.argv[0])
if bzr_path != cwd:
  while cwd in sys.path:
    sys.path.remove(cwd)

Alternatively, if we do it in the 'bzr' main script, we can change the
line to:

bzr_path = os.path.dirname(__file__)

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpfN38ACgkQJdeBCYSNAANh7QCgtPkEGdc4GykL51ahAQa4LADT
VMUAn3PAW7mNRidgRdJnDeCOrYiAO3Ol
=pIQS
-----END PGP SIGNATURE-----

Hi,

 status invalid

On Thu, Jul 16, 2009 at 02:24:24PM -0000, John A Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> ...
>
> > 09:54 forest@W038$ python /tmp/foo/test.py
> > ['/tmp/foo', '/home/forest/usr/lib/python2.5/site-packages/setuptools-0.6c9-py2.5.egg', '/home/forest/usr/lib/python2.5/site-packages/Satchmo-0.9_pre-py2.5.egg', '/home/forest/lib/python', '/home/forest/usr/lib/python2.6/site-packages', '/home/forest/usr/lib/python2.5/site-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages',
> > '/tmp/bar', '/usr/lib/python2.6', '/usr/lib/python2.6/plat-linux2', '/usr/lib/python2.6/lib-tk', '/usr/lib/python2.6/lib-old', '/usr/lib/python2.6/lib-dynload', '/usr/lib/python2.6/dist-packages', '/usr/lib/python2.6/dist-packages/PIL', '/usr/lib/python2.6/dist-packages/gst-0.10', '/var/lib/python-support/python2.6', '/usr/lib/python2.6/dist-packages/gtk-2.0', '/var/lib/python-support/python2.6/gtk-2.0', '/usr/lib/python2.6/dist-packages/wx-2.8-gtk2-unicode', '/usr/local/lib/python2.6/dist-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages']
>
> > [/tmp/bar]
>
> ^- So indeed, /tmp/bar is in your path.

Correct.

> What I've seen in the past, is that if you insert debug statements into
> *site.py* it gets a '' in sys.path, and then it calls a function to turn
> all paths in 'sys.path' into absolute paths (and thus '' => $PWD).

Indeed, this is what is happening. Here's why:

My .bashrc does export PYTHONPATH="foo:$PYTHONPATH". Given that PYTHONPATH is
empty before it executes, I end up with PYTHONPATH "foo:". The trailing :
results in '' being in sys.path. Python then translates this to $PWD.

So this is my fault, more-or-less. My apologies. I'll go make my .bashrc more
complicated.

-Forest
--
Forest Bond
http://www.alittletooquiet.net
http://www.pytagsfs.org

Hi,

 affects bzr
 status invalid

 affects ubuntu/bzr
 status invalid

On Thu, Jul 16, 2009 at 02:24:24PM -0000, John A Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> ...
>
> > 09:54 forest@W038$ python /tmp/foo/test.py
> > ['/tmp/foo', '/home/forest/usr/lib/python2.5/site-packages/setuptools-0.6c9-py2.5.egg', '/home/forest/usr/lib/python2.5/site-packages/Satchmo-0.9_pre-py2.5.egg', '/home/forest/lib/python', '/home/forest/usr/lib/python2.6/site-packages', '/home/forest/usr/lib/python2.5/site-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages',
> > '/tmp/bar', '/usr/lib/python2.6', '/usr/lib/python2.6/plat-linux2', '/usr/lib/python2.6/lib-tk', '/usr/lib/python2.6/lib-old', '/usr/lib/python2.6/lib-dynload', '/usr/lib/python2.6/dist-packages', '/usr/lib/python2.6/dist-packages/PIL', '/usr/lib/python2.6/dist-packages/gst-0.10', '/var/lib/python-support/python2.6', '/usr/lib/python2.6/dist-packages/gtk-2.0', '/var/lib/python-support/python2.6/gtk-2.0', '/usr/lib/python2.6/dist-packages/wx-2.8-gtk2-unicode', '/usr/local/lib/python2.6/dist-packages', '/home/forest/usr/lib/python2.4/site-packages', '/home/forest/usr/lib/python2.3/site-packages']
>
> > [/tmp/bar]
>
> ^- So indeed, /tmp/bar is in your path.

Correct.

> What I've seen in the past, is that if you insert debug statements into
> *site.py* it gets a '' in sys.path, and then it calls a function to turn
> all paths in 'sys.path' into absolute paths (and thus '' => $PWD).

Indeed, this is what is happening. Here's why:

My .bashrc does export PYTHONPATH="foo:$PYTHONPATH". Given that PYTHONPATH is
empty before it executes, I end up with PYTHONPATH "foo:". The trailing :
results in '' being in sys.path. Python then translates this to $PWD.

So this is my fault, more-or-less. My apologies. I'll go make my .bashrc more
complicated.

-Forest
--
Forest Bond
http://www.alittletooquiet.net
http://www.pytagsfs.org