Merge lp:~manishsinha/microfiber/fix-hardcoded-sphinx-build-in-usrbin into lp:microfiber

[Manish Sinha (मनीष सिन्हा)]

Fixed bugs #847044 where sphinx-build was hardcoded at /usr/bin/sphinx-build

Read the $PATH environment variable. Split it by : character. For every entry append
'sphinx-build' and check whether the file exists. If the file exists, check if it
is executable. If yes, then return the path. If none is found then return None.
The method calling this search method has to handle None return by showing a WARNING

Fixed bugs #847044 where sphinx-build was hardcoded at /usr/bin/sphinx-build

Read the $PATH environment variable. Split it by : character. For every entry append
'sphinx-build' and check whether the file exists. If the file exists, check if it
is executable. If yes, then return the path. If none is found then return None.
The method calling this search method has to handle None return by showing a WARNING

[Jason Gerard DeRose (jderose)]

After thinking on this a bit, I feel something like this is a better solution:

class build_with_docs(build):

    def run(self):
        build.run(self)
        try:
            sphinx = subprocess.check_output(['/bin/which', 'sphinx-build'])
        except subprocess.CalledProcessError:
            print("WARNING: Documentation not generated. python-sphinx missing")
            return
        tree = path.dirname(path.abspath(__file__))
        src = path.join(tree, 'doc')
        dst = path.join(tree, 'doc', '_build', 'html')
        cmd = [
            sphinx,
            '-b', 'html',
            src,
            dst
        ]
        subprocess.check_call(cmd)

What do you think?

review: Needs Fixing

[Manish Sinha (मनीष सिन्हा) (manishsinha)]

In this case you are again hardcoding the path to be /bin or something. The actual path has to be taken from PATH environment variable

[Manish Sinha (मनीष सिन्हा) (manishsinha)]

My mistake. I mistook it. Now there are two ways
* Open subprocesses
* check the path now.

Your call now

[lp:~manishsinha/microfiber/fix-hardcoded-sphinx-build-in-usrbin]

79. By Manish Sinha (मनीष सिन्हा) on 2011-09-11

Changed manual filename creation with os.path.join

80. By Manish Sinha (मनीष सिन्हा) on 2011-09-11

Change os.path.isfile to path.isfile

81. By Manish Sinha (मनीष सिन्हा) on 2011-09-11

Removed the name clash with 'path'. Renamed path to file_path

[Jason Gerard DeRose (jderose)]

That's the ticket. Thanks!

review: Approve

=== modified file 'setup.py'
--- setup.py	2011-09-11 17:41:32 +0000
+++ setup.py	2011-09-11 20:10:24 +0000
@@ -102,10 +102,18 @@
 
 class build_with_docs(build):
 
+    def find_sphinx_path(self):
+        for prefix in os.environ['PATH'].split(':'):
+            file_path = os.path.join(prefix, 'sphinx-build')
+            if path.isfile(file_path) and os.access(file_path, os.X_OK):
+                return file_path
+
+        return None
+
     def run(self):
         build.run(self)
-        sphinx = '/usr/bin/sphinx-build'
-        if not path.isfile(sphinx):
+        sphinx = self.find_sphinx_path() 
+        if sphinx is None:
             print("WARNING: Documentation not generated. python-sphinx missing")
             return
         tree = path.dirname(path.abspath(__file__))