classification
Title: Improve detection of availability of bdist_msi command
Type: behavior Stage: resolved
Components: Distutils2 Versions: Python 3.3, 3rd party
process
Status: closed Resolution: out of date
Dependencies: 12703 Superseder:
Assigned To: eric.araujo Nosy List: alexis, eric.araujo, paul.moore, tarek, vinay.sajip
Priority: normal Keywords:

Created on 2011-10-13 18:47 by paul.moore, last changed 2014-03-13 03:28 by eric.araujo. This issue is now closed.

Messages (8)
msg145478 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-13 18:47
In a directory with 2 files, setup.cfg and a single C file containing source for an extension module. The same happens with a pure-python module. This is on Windows.

PS D:\Data\python-sample> D:\Data\cpython\PCbuild\python.exe -m packaging.run run --list-commands
List of available commands:
  bdist: create a built (binary) distribution
  bdist_dumb: create a "dumb" built distribution
Traceback (most recent call last):
  File "D:\Data\cpython\lib\packaging\util.py", line 652, in resolve_name
    ret = getattr(ret, part)
AttributeError: 'module' object has no attribute 'bdist_msi'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "D:\Data\cpython\lib\runpy.py", line 160, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "D:\Data\cpython\lib\runpy.py", line 73, in _run_code
    exec(code, run_globals)
  File "D:\Data\cpython\lib\packaging\run.py", line 666, in <module>
    sys.exit(main())
  File "D:\Data\cpython\lib\packaging\run.py", line 653, in main
    return dispatcher()
  File "D:\Data\cpython\lib\packaging\run.py", line 642, in __call__
    return func(self, self.args)
  File "D:\Data\cpython\lib\packaging\run.py", line 91, in wrapper
    return f(*args, **kwargs)
  File "D:\Data\cpython\lib\packaging\run.py", line 264, in _run
    cls = dispatcher.cmdclass.get(cmd) or get_command_class(cmd)
  File "D:\Data\cpython\lib\packaging\command\__init__.py", line 61, in get_command_class
    cls = resolve_name(cls)
  File "D:\Data\cpython\lib\packaging\util.py", line 654, in resolve_name
    raise ImportError(exc)
ImportError: 'module' object has no attribute 'bdist_msi'
msg145530 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-14 15:20
Can you tell me if this works:

>>> import _msi
>>> from packaging.command.bdist_msi import bdist_msi
msg145551 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-14 17:32
No it didn't - I had not built the _msi module when I built Python for some reason. I have built _msi now, and everything works. Sorry for the false alarm.

Arguably, the command shouldn't fail, it should simply omit the bdist_msi command from the listing. But as _msi is part of Python, the installation is broken if it isn't present so I guess that handling the issue gracefully isn't really important. (I assume the missing _msi extension isn't an issue on Unix).
msg145598 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-10-15 15:46
> so I guess that handling the issue gracefully isn't really important.

Then should this issue be closed?
msg145599 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-15 15:59
I suppose so, yes. But it feels symptomatic of a general lack of clean
error handling, which I think should be fixed :-(
msg145690 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-17 13:26
> Arguably, the command shouldn't fail, it should simply omit the bdist_msi command from the listing.
> But as _msi is part of Python, the installation is broken if it isn't present so I guess that
> handling the issue gracefully isn't really important.
My first feeling was agreement with your request, but then you added that _msi is part of Python, so I’m not sure we should increase the code complexity for a non-issue.

> (I assume the missing _msi extension isn't an issue on Unix).
Yep, see Lib/packaging/command/__init__.py:

  # XXX this is crappy
  if os.name == 'nt':
      _COMMANDS['bdist_msi'] = 'packaging.command.bdist_msi.bdist_msi'

> it feels symptomatic of a general lack of clean error handling, which I think should be fixed :-(
I suppose the traceback would have been much more useful with #12703 fixed.  (There are a number of resolve_name-like functions out there, it’s sad to see it reimplemented with various degree in the quality of error reporting over and over again.)

Besides the problem with resolve_name, I think there’s a specific problem with bdist_msi.  As you can see with the comment I added in the code and quoted above, I’m not satisfied with the way we add bdist_msi to the set of available commands.  It’s the only command to be special-cased; I guess it can’t be helped, so maybe I should remove the comment.  I could change the conditional to register the command if _msi can be successfully imported; however, I fear this would hide bugs, as _msi is a part of a Python installation on Windows, so I’d rather continue to check os.name.  Would you be satisfied with a more helpful traceback that would point you immediately to missing msi?  Do you prefer that bdist_msi catch an ImportError for _msi and print a short error message instead of a traceback in all its glory?
msg145733 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2011-10-17 17:18
> Would you be satisfied with a more helpful traceback that would point you immediately to missing msi?  Do you prefer that bdist_msi
> catch an ImportError for _msi and print a short error message instead of a traceback in all its glory?

I'd be happy with a more explicit error. As you say, it's a broken
build so not worth worrying too much about, but it did take me quite a
while to work out what the problem was, including having to raise this
bug report. (The existing traceback doesn't even mention _msi).

So yes, a simple error would be ideal.
msg145841 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-18 16:44
> The existing traceback doesn't even mention _msi.

Looks like #12703, which I’m going to fix shortly.
History
Date User Action Args
2014-03-13 03:28:33eric.araujosetstatus: open -> closed
resolution: out of date
stage: resolved
2011-10-18 16:44:24eric.araujosetdependencies: + Improve error reporting for packaging.util.resolve_name
messages: + msg145841
2011-10-17 17:18:58paul.mooresetmessages: + msg145733
2011-10-17 13:26:20eric.araujosetassignee: tarek -> eric.araujo
title: pysetup run --list-commands fails with a traceback -> Improve detection of availability of bdist_msi command
messages: + msg145690
versions: + 3rd party
2011-10-15 15:59:25paul.mooresetmessages: + msg145599
2011-10-15 15:46:12vinay.sajipsetnosy: + vinay.sajip
messages: + msg145598
2011-10-14 17:32:55paul.mooresetmessages: + msg145551
2011-10-14 15:20:15eric.araujosetmessages: + msg145530
2011-10-13 18:47:50paul.moorecreate