Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VCS not found message could be friendlier #21

Closed
maphew opened this issue Nov 19, 2013 · 10 comments
Closed

VCS not found message could be friendlier #21

maphew opened this issue Nov 19, 2013 · 10 comments

Comments

@maphew
Copy link

maphew commented Nov 19, 2013

It would be nice if the a friendlier message was printed when the source code control program is not available, e.g. bzr not found instead of:

[py32] B:\apps\leo
> check-manifest .
listing source files under version controlTraceback (most recent call last):
  File "C:\Python32\Scripts\check-manifest-script.py", line 9, in <module>
    load_entry_point('check-manifest==0.17', 'console_scripts', 'check-manifest')()
  File "C:\Python32\lib\site-packages\check_manifest.py", line 639, in main
    update=args.update, python=args.python):
  File "C:\Python32\lib\site-packages\check_manifest.py", line 554, in check_manifest
    all_source_files = sorted(get_vcs_files())
  File "C:\Python32\lib\site-packages\check_manifest.py", line 311, in get_vcs_files
    return normalize_names(vcs.get_versioned_files())
  File "C:\Python32\lib\site-packages\check_manifest.py", line 279, in get_versioned_files
    output = run(['bzr', 'ls', '-VR'])
  File "C:\Python32\lib\site-packages\check_manifest.py", line 137, in run
    stderr=subprocess.STDOUT)
  File "C:\Python32\lib\subprocess.py", line 741, in __init__
    restore_signals, start_new_session)
  File "C:\Python32\lib\subprocess.py", line 960, in _execute_child
    startupinfo)
WindowsError: [Error 2] The system cannot find the file specified
@mgedmin
Copy link
Owner

mgedmin commented Nov 20, 2013

Agreed.

@mgedmin
Copy link
Owner

mgedmin commented Nov 21, 2013

Ok, what is surprising to me is that WindowsError is not a subclass of OSError, because I already have error handling for this that looks like

    try:
        pipe = subprocess.Popen(command, stdout=subprocess.PIPE,
                                stderr=subprocess.STDOUT)
    except OSError as e:
        raise Failure("could not run %s: %s" % (command, e))

http://docs.python.org/2.7/library/exceptions#exceptions.WindowsError says WindowsError is a subclass of OSError. I am confused.

@mgedmin
Copy link
Owner

mgedmin commented Nov 21, 2013

Python 3.2 docs say the same: http://docs.python.org/3.2/library/exceptions#WindowsError

Python 3.3 says it was merged into OSError: http://docs.python.org/3.3/library/exceptions#OSError

@mgedmin
Copy link
Owner

mgedmin commented Nov 22, 2013

On the plus side, I managed to reproduce this uncaught WindowsError:
https://jenkins.gedmin.as/job/check-manifest-on-windows/12/console

@mgedmin
Copy link
Owner

mgedmin commented Nov 22, 2013

No, scratch that, that's a different place in the code (and one where I'm not catching OSError).

@mgedmin
Copy link
Owner

mgedmin commented Nov 22, 2013

I cannot reproduce this.

I open Git Bash on a Windows machine and run

$ PATH=nope c:/python27/python check_manifest.py .
listing source files under version control
could not run ['git', 'ls-files']: [Error 2] The system cannot find the file specified

i.e. the error is caught correctly.

I would dearly love to know how you managed :)

@maphew
Copy link
Author

maphew commented Nov 22, 2013

Sure :) Start > Run > cmd.exe

B:\>cd apps\leo

B:\apps\leo>py32.bat --version
PYTHONHOME=C:\Python32
Python 3.2.5

[py32] B:\apps\leo
> set path=%pythonhome%;%pythonhome%\Scripts

[py32] B:\apps\leo
> check-manifest .
listing source files under version controlTraceback (most recent call last):
  File "C:\Python32\Scripts\check-manifest-script.py", line 9, in <module>
    load_entry_point('check-manifest==0.17', 'console_scripts', 'check-manifest')()
  File "C:\Python32\lib\site-packages\check_manifest.py", line 639, in main
    update=args.update, python=args.python):
  File "C:\Python32\lib\site-packages\check_manifest.py", line 554, in check_manifest
    all_source_files = sorted(get_vcs_files())
  File "C:\Python32\lib\site-packages\check_manifest.py", line 311, in get_vcs_files
    return normalize_names(vcs.get_versioned_files())
  File "C:\Python32\lib\site-packages\check_manifest.py", line 279, in get_versioned_files
    output = run(['bzr', 'ls', '-VR'])
  File "C:\Python32\lib\site-packages\check_manifest.py", line 137, in run
    stderr=subprocess.STDOUT)
  File "C:\Python32\lib\subprocess.py", line 744, in __init__
    restore_signals, start_new_session)
  File "C:\Python32\lib\subprocess.py", line 977, in _execute_child
    startupinfo)
WindowsError: [Error 2] The system cannot find the file specified

py32.bat:

@echo off
if not defined PYTHONHOME call :set_env %~n0
python %*
goto :eof

:set_env
  :: extract python version from last 2 characters of batch filename
  set _ver=%1
  set _ver=%_ver:~-2%

  set PYTHONHOME=C:\Python%_ver%
  set PROMPT=[py%_ver%] $p$_$g

  set _path=C:\Python%_ver%
  call addpath _path
  set _path=C:\Python%_ver%\Scripts
  call addpath _path

  set python

  set _path=
  set _ver=
  goto :eof

In the console output above, I redefined path explicitly in order rule out contamination from another directory. I get the same result with python 2.7.

@mgedmin
Copy link
Owner

mgedmin commented Nov 23, 2013

I have reproduced this! In Git Bash, with Python 2.7

$ virtualenv env
$ env/scripts/pip install check-manifest
$ mkdir fakepackage
$ cd fakepackage
$ mkdir .bzr
$ touch setup.py
$ ../env/scripts/check-manifest .
listing source files under version controlTraceback (most recent call last):
  File "C:\Users\mg\AppData\Local\Temp\3\env\scripts\check-manifest-script.py", line 9, in <module>
    load_entry_point('check-manifest==0.17', 'console_scripts', 'check-manifest')()
  File "C:\Users\mg\AppData\Local\Temp\3\env\lib\site-packages\check_manifest.py", line 639, in main
    update=args.update, python=args.python):
  File "C:\Users\mg\AppData\Local\Temp\3\env\lib\site-packages\check_manifest.py", line 554, in check_manifest
    all_source_files = sorted(get_vcs_files())
  File "C:\Users\mg\AppData\Local\Temp\3\env\lib\site-packages\check_manifest.py", line 311, in get_vcs_files
    return normalize_names(vcs.get_versioned_files())
  File "C:\Users\mg\AppData\Local\Temp\3\env\lib\site-packages\check_manifest.py", line 279, in get_versioned_files
    output = run(['bzr', 'ls', '-VR'])
  File "C:\Users\mg\AppData\Local\Temp\3\env\lib\site-packages\check_manifest.py", line 137, in run
    stderr=subprocess.STDOUT)
  File "c:\python27\Lib\subprocess.py", line 709, in __init__
    errread, errwrite)
  File "c:\python27\Lib\subprocess.py", line 957, in _execute_child
    startupinfo)
WindowsError: [Error 2] The system cannot find the file specified

@mgedmin
Copy link
Owner

mgedmin commented Nov 23, 2013

Oh I am so stupid.

The bug is present in the last release (0.17) but fixed in git master (in commit 29b0d59, with the so-clear commit message "Start improving test coverage").

@mgedmin mgedmin closed this as completed Nov 23, 2013
@maphew
Copy link
Author

maphew commented Nov 25, 2013

"Oh I am so stupid."

ahh, but now you know it, so are wise. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants