support for setting binary paths via environment varaibles

added component::core tor/chutney owner::nickm priority::medium reporter::dave2008 resolution::implemented status::closed type::enhancement labels

Possible bug: If I have 'tor' and 'tor-gencert' in my path, and I don't set the environment variables, won't this fail?

That is, when you call this code:

+        tor_path = self._env['tor']
+        if not os.path.exists(tor_path):
+            print "Cannot find Tor binary: {0}".format(tor_path)
+            return False

I think that tor_path will be the string "tor", and "os.path.exists(tor_path)" will check whether there is a file named "tor" in the cwd, but it won't check for whether there is a file named "tor" in your path.

One option is to remove those checks, and to allow subprocess.Popen to fail instead. That won't give such a nice error message, though.

Trac:
Status: new to needs_revision

Oh, good catch. How about remove the check and catch the error from subprocess?

Trac:
Username: dave2008

Better than nothing! Let's try to distinguish that particular error ( os.OSError, errno=ENOENT) from the others. And when we catch it, let's make sure to tell the user what path we were looking for, and how to set the location of tor or tor-gencert.

hi nick,

sorry the the late reply, was out for travel :)

I have updated my branch accordingly. There is duplicate code mainly because subprocess is called with different arguments. maybe we should clean up that a little bit and make a wrapper for code reuse on the try/catch code?

Also should we switch to argparse module as well?

Trac:
Username: dave2008

Replying to dave2008:

hi nick,

sorry the the late reply, was out for travel :)

I have updated my branch accordingly.

There is duplicate code mainly because subprocess is called with different arguments. maybe we should clean up that a little bit and make a wrapper for code reuse on the try/catch code?

Hm. It's a little irksome, but I think it's not actually the end of the world here. I'd take a patch to remove the redundancy if you want to write one.

In any case, this looks fine, so I merged it.

BTW, I also changed errno.errorcode[e.errno] == 'ENOENT' to e.errno == errno.ENOENT`, and made the log messages include the name of the binary that we tried to open.

Also should we switch to argparse module as well?

The documentation says that module is new in Python 3.2. I think we still have Chutney working with python 2.

Trac:
Resolution: N/A to implemented
Status: needs_revision to closed

Argparse is included as standard lib in Python start from 2.7. So if we want to support 2.6, then we might want to use optparse. But I think it's reasonable to assume that most Chuntney users will have at least Python 2.7 and for those who are still using legacy 2.6, they can pip install argparse.

Any comment?

Trac:
Username: dave2008

Let's revisit that once debian squeeze hits end-of-life (in a few months); I believe it shipped Python 2.6 and Python 3.1, and Debian "oldstable" is usually a good guideline for what it's safe to insist on. Want to make a ticket to remind us of that?

(Some people try to only insist on what's in RHEL/CentOS , but that seems unreasonable for a project like Chutney).

OK, done, see #11115 (moved).

Trac:
Username: dave2008

closed

mentioned in issue #11115 (moved)

moved to tpo/core/chutney#10933 (closed)

mentioned in issue tpo/core/chutney#11115

support for setting binary paths via environment varaibles

Child items 0

Activity