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

setuptools.command.bdist_wininst may crash if distutils.command.bdist_wininst is missing #857

Closed
zooba opened this issue Nov 23, 2016 · 6 comments

Comments

@zooba
Copy link
Contributor

zooba commented Nov 23, 2016

In the new Python Nuget packages, bdist_wininst is omitted (because, you know, stop making wininst installers). However, because setuptools tries to import it unconditionally, when it is missing, you get a stack like this:

      File "<string>", line 1, in <module>
      File "C:\Resources\temp\68608a9b20b04c22861a4d169c0d7feb.WorkerRole1\RoleTemp\pip-build-g3r7dcl7\azure\setup.py", line 61, in <module>
        'azure-servicemanagement-legacy==0.20.2',
      File "F:\approot\bin\python.3.5.2\tools\lib\distutils\core.py", line 148, in setup
        dist.run_commands()
      File "F:\approot\bin\python.3.5.2\tools\lib\distutils\dist.py", line 955, in run_commands
        self.run_command(cmd)
      File "F:\approot\bin\python.3.5.2\tools\lib\distutils\dist.py", line 974, in run_command
        cmd_obj.run()
      File "F:\approot\bin\python.3.5.2\tools\lib\site-packages\setuptools\command\install.py", line 61, in run
        return orig.install.run(self)
      File "F:\approot\bin\python.3.5.2\tools\lib\distutils\command\install.py", line 551, in run
        self.run_command(cmd_name)
      File "F:\approot\bin\python.3.5.2\tools\lib\distutils\cmd.py", line 313, in run_command
        self.distribution.run_command(command)
      File "F:\approot\bin\python.3.5.2\tools\lib\distutils\dist.py", line 974, in run_command
        cmd_obj.run()
      File "F:\approot\bin\python.3.5.2\tools\lib\site-packages\setuptools\command\install_scripts.py", line 34, in run
        bw_cmd = self.get_finalized_command("bdist_wininst")
      File "F:\approot\bin\python.3.5.2\tools\lib\distutils\cmd.py", line 298, in get_finalized_command
        cmd_obj = self.distribution.get_command_obj(command, create)
      File "F:\approot\bin\python.3.5.2\tools\lib\distutils\dist.py", line 846, in get_command_obj
        klass = self.get_command_class(command)
      File "F:\approot\bin\python.3.5.2\tools\lib\site-packages\setuptools\dist.py", line 430, in get_command_class
        self.cmdclass[command] = cmdclass = ep.load()
      File "F:\approot\bin\python.3.5.2\tools\lib\site-packages\pkg_resources\__init__.py", line 2229, in load
        return self.resolve()
      File "F:\approot\bin\python.3.5.2\tools\lib\site-packages\pkg_resources\__init__.py", line 2235, in resolve
        module = __import__(self.module_name, fromlist=['__name__'], level=0)
      File "F:\approot\bin\python.3.5.2\tools\lib\site-packages\setuptools\command\bdist_wininst.py", line 1, in <module>
        import distutils.command.bdist_wininst as orig
    ImportError: No module named 'distutils.command.bdist_wininst'
@zooba
Copy link
Contributor Author

zooba commented Nov 23, 2016

I'm willing to make and verify any fix here, but I'll need a bit of guidance.

I can see that setup.py scrapes distutils.command.__init__.__all__ for commands, but because this happens when building the wheel it has no impact on most machines. (Also, you can't install setuptools from source without setuptools, and you can't install setuptools without bdist_wininst because of this issue.)

So my guess is that we really ought to handle ImportError somewhere in this stack, but I'm not sure exactly where.

@zooba
Copy link
Contributor Author

zooba commented Nov 23, 2016

Adding handling for ImportError in install_scripts.run works:

        try:
            bs_cmd = self.get_finalized_command('build_scripts')
        except ImportError:
            bs_cmd = None
        exec_param = getattr(bs_cmd, 'executable', None)
        try:
            bw_cmd = self.get_finalized_command("bdist_wininst")
        except ImportError:
            bw_cmd = None

I'm a little surprised that get_finalized_command has no handling for the case where someone requests an arbitrary command without knowing whether it actually exists, but it's going to be real hard to go back in time and add that to old versions of Python.

@zooba
Copy link
Contributor Author

zooba commented Nov 23, 2016

Alternatively, adding this to setuptools.command.bdist_wininst seems to work:

try:
    import distutils.command.bdist_wininst as orig
except ImportError:
    from distutils.core import Command
    from distutils.errors import DistutilsModuleError
    
    class bdist_wininst(Command):
        def initialize_options(self): pass
        def finalize_options(self): pass
        
        def run(self):
            raise DistutilsModuleError("bdist_wininst is not supported in this distribution")
else:
    class bdist_wininst(orig.bdist_wininst):
        ...

@zooba
Copy link
Contributor Author

zooba commented Jan 10, 2017

I've fixed this in the CPython nuget packages instead, so they once again have bdist_wininst, but if you try and actually use it then your install will come crashing down (as it should).

@zooba zooba closed this as completed Jan 10, 2017
@jaraco
Copy link
Member

jaraco commented Jan 14, 2017

@zooba: Should we not update setuptools to deprecate this command (issuing a warning when it is available and used) and prepare to remove support for it entirely in a future, backward-incompatible release?

@zooba
Copy link
Contributor Author

zooba commented Jan 17, 2017

I am totally okay with dropping support for bdist_wininst.

It would be great if we had a UI-based installer for .whl files that would let you choose an install location when you double-click them, since that's the only compelling feature of wininst, but the inherent problems in wininst are bad enough that I'm in favour of getting rid of them as soon as possible. (Personally, I always wheel convert them.)

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