band-unfolding / bandup

BandUP: Band Unfolding code for Plane-wave based calculations
http://www.ifm.liu.se/theomod/compphys/band-unfolding
GNU General Public License v3.0
99 stars 49 forks source link

possible bug with python wrapper (shell=True) #2

Closed chrisewolf closed 7 years ago

chrisewolf commented 7 years ago

Hi guys, great tool!

Unfortunately it did not work "out of the box" (might be related to my python 2.7 on ubuntu), I had to add

shell=True

on line 36 in version.py

def get_latest_git_tag():
    tag = None
    get_tag_run = Popen(['git', 'describe', '--tags', '--dirty'],
                           stdout=PIPE, stderr=PIPE, shell=True)
    stdout, stderr = get_tag_run.communicate()
    if(not stderr.strip()):
        tag = stdout.strip()
    return tag

otherwise it would throw

Traceback (most recent call last):
  File "/home/chris/Downloads/bandup-master/bandup", line 71, in <module>
    parser = BandUpPythonArgumentParser()
  File "/home/chris/Downloads/bandup-master/src/python_interface/bandupy/argparse_wrapper.py", line 643, in __init__
    version='%(prog)s {version}'.format(version=__version__()))
  File "/home/chris/Downloads/bandup-master/src/python_interface/bandupy/version.py", line 43, in get_package_version
    pv = get_latest_git_tag()
  File "/home/chris/Downloads/bandup-master/src/python_interface/bandupy/version.py", line 36, in get_latest_git_tag
    stdout=PIPE, stderr=PIPE)
  File "/usr/lib/python2.7/subprocess.py", line 711, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1343, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

Best, Chris

paulovcmedeiros commented 7 years ago

Hi Chris!

Many thanks for your interest in BandUP and for reporting this issue. I've been able to reproduce it by running the code in an environment without git. I suspect this is the case in your system.

If subprocess tries to call a command that is not available, then an OSError exception is raised. Since all systems I use have git, I ended up overlooking this. I have now fixed the error by catching the exception. I prefer, however, not to use shell=True (see, for instance, https://docs.python.org/3.4/library/subprocess.html#security-considerations). This is avoided by catching the OSError exception.

Just FYI: The reason why shell=True works is because, by doing so, no exception is thrown. Instead, a "command not found" system error msg gets sent to the "stderr" variable and then "get_latest_git_tag" simply returns None. As discussed above, however, I prefer to avoid using shell=True if possible.

Many thanks once more for taking your time to report this issue. I appreciate and encourage this.

Best wishes, Paulo.