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

Fix the fact that spyder_win_post_install.py can't be run from pip #2377

Merged
merged 5 commits into from
Jun 21, 2015
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 128 additions & 3 deletions scripts/spyder_win_post_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,100 @@
KEY_C1 = KEY_C0 + r"\command"


# ability to run spyder-win-post-install outside of bdist_wininst installer
# copied from pywin32-win-post-install.py
# http://pywin32.hg.sourceforge.net/hgweb/pywin32/pywin32/file/default/pywin32_postinstall.py
try:
# When this script is run from inside the bdist_wininst installer,
# file_created() and directory_created() are additional builtin
# functions which write lines to Python23\pywin32-install.log. This is
# a list of actions for the uninstaller, the format is inspired by what
# the Wise installer also creates.
# https://docs.python.org/2/distutils/builtdist.html#the-postinstallation-script
file_created
is_bdist_wininst = True
except NameError:
is_bdist_wininst = False # we know what it is not - but not what it is :)
# file_created() and directory_created() functions do nothing if post install script
# isn't run from bdist_wininst installer, instead if shortcuts and start menu directory
# exist, they are removed when the post install script is called with the -remote option
def file_created(file):
pass
def directory_created(directory):
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these couple of functions are empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the user runs the spyder_win_post_install.py manually after using pip to install Spyder, there isn't a registered uninstaller for Spyder. The uninstaller is only registered when using the bdist_wininst installer. The functions file_created() and directory_created() normally register files and directories to be removed with the uninstaller, but for pip users these functions should do nothing. Instead if the shortcut files and startmenu directory exist, they are removed in L174 and L186.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explanations should be added as comments in the code, it's far from obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a bit of explanation from Mark Hammond on L27. I added a link to bdist_wininst postinstaller in the standard referencce library and additional comments requested by @Nodd directly prior to the dummy method definitions in 330b0aa.

def get_root_hkey():
try:
winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE,
root_key_name, 0, winreg.KEY_CREATE_SUB_KEY)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

root_key_name is not defined anywhere in your PR, so this call is going to fail, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccordoba12 good catch, on windows this key is hkey_local_machine\Software\Python\PythonCore\2.7 for python installed with msi, however, I don't know what it would be on a windows system that has a bootstrapped Python, eg: WinPython, portable python or any python just unzipped in and executed from a standalone folder.

This is only run by bdist_wininst and it's sole purpose is to determine if the user has elevated their rights, if so then use hkey_local_machine and if not then use hkey_current_user. Therefore even if this call fails, it will just fall back to hkey_current_user which is what we want anyway, so it doesn't matter. But for those that have installed official python, and have admin rights, then it will use the system registry keys.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, then we can leave it without problems :-)

Please address my other comments :-)

return winreg.HKEY_LOCAL_MACHINE
except OSError, details:
# Either not exist, or no permissions to create subkey means
# must be HKCU
return winreg.HKEY_CURRENT_USER
try:
create_shortcut
except NameError:
# Create a function with the same signature as create_shortcut provided
# by bdist_wininst
def create_shortcut(path, description, filename,
arguments="", workdir="", iconpath="", iconindex=0):
try:
import pythoncom
except ImportError:
print("PyWin32 is required to run windows postinstall manually.",
file=sys.stderr)
sys.exit(1) # pywin32 required
from win32com.shell import shell, shellcon

ilink = pythoncom.CoCreateInstance(shell.CLSID_ShellLink, None,
pythoncom.CLSCTX_INPROC_SERVER,
shell.IID_IShellLink)
ilink.SetPath(path)
ilink.SetDescription(description)
if arguments:
ilink.SetArguments(arguments)
if workdir:
ilink.SetWorkingDirectory(workdir)
if iconpath or iconindex:
ilink.SetIconLocation(iconpath, iconindex)
# now save it.
ipf = ilink.QueryInterface(pythoncom.IID_IPersistFile)
ipf.Save(filename, 0)

# Support the same list of "path names" as bdist_wininst.
def get_special_folder_path(path_name):
import pythoncom
from win32com.shell import shell, shellcon

for maybe in """
CSIDL_COMMON_STARTMENU CSIDL_STARTMENU CSIDL_COMMON_APPDATA
CSIDL_LOCAL_APPDATA CSIDL_APPDATA CSIDL_COMMON_DESKTOPDIRECTORY
CSIDL_DESKTOPDIRECTORY CSIDL_COMMON_STARTUP CSIDL_STARTUP
CSIDL_COMMON_PROGRAMS CSIDL_PROGRAMS CSIDL_PROGRAM_FILES_COMMON
CSIDL_PROGRAM_FILES CSIDL_FONTS""".split():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not creating a list directly instead of splitting a string ?
Performance-wise it won't matter but it would be more obvious to understand. I had to read these lines 3 times to spot the .split() here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this last comment from @Nodd.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do this myself after merging.

if maybe == path_name:
csidl = getattr(shellcon, maybe)
return shell.SHGetSpecialFolderPath(0, csidl, False)
raise ValueError("%s is an unknown path ID" % (path_name,))


def install():
"""Function executed when running the script with the -install switch"""
# Create Spyder start menu folder
start_menu = osp.join(get_special_folder_path('CSIDL_COMMON_PROGRAMS'),
# Don't use CSIDL_COMMON_PROGRAMS because it requres admin rights
# This is consistent with use of CSIDL_DESKTOPDIRECTORY below
# CSIDL_COMMON_PROGRAMS =
# C:\ProgramData\Microsoft\Windows\Start Menu\Programs
# CSIDL_PROGRAMS =
# C:\Users\<username>\AppData\Roaming\Microsoft\Windows\Start Menu\Programs
start_menu = osp.join(get_special_folder_path('CSIDL_PROGRAMS'),
'Spyder (Py%i.%i %i bit)' % (sys.version_info[0],
sys.version_info[1],
struct.calcsize('P')*8))
if not osp.isdir(start_menu):
os.mkdir(start_menu)
directory_created(start_menu)

# Create Spyder start menu entries
python = osp.abspath(osp.join(sys.prefix, 'python.exe'))
pythonw = osp.abspath(osp.join(sys.prefix, 'pythonw.exe'))
Expand All @@ -40,6 +123,9 @@ def install():
import distutils.sysconfig
lib_dir = distutils.sysconfig.get_python_lib(plat_specific=1)
ico_dir = osp.join(lib_dir, 'spyderlib', 'windows')
# if user is running -install manually then icons are in Scripts/
if not osp.isdir(ico_dir):
ico_dir = osp.dirname(osp.abspath(__file__))

desc = 'Scientific Python Development EnvironmEnt, an alternative to IDLE'
fname = osp.join(start_menu, 'Spyder (full).lnk')
Expand Down Expand Up @@ -67,7 +153,7 @@ def install():
winreg.SetValueEx(winreg.CreateKey(root, KEY_C1 % ("NoCon", EWS)),
"", 0, winreg.REG_SZ,
'"%s" "%s\Scripts\spyder" "%%1"' % (pythonw, sys.prefix))

# Create desktop shortcut file
desktop_folder = get_special_folder_path("CSIDL_DESKTOPDIRECTORY")
fname = osp.join(desktop_folder, 'Spyder.lnk')
Expand All @@ -87,6 +173,44 @@ def remove():
winreg.DeleteKey(root, key)
except WindowsError:
pass
else:
print("Successfully removed Spyder shortcuts from Windows "\
"Explorer context menu.", file=sys.stdout)
# clean up desktop
desktop_folder = get_special_folder_path("CSIDL_DESKTOPDIRECTORY")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code, i.e. from here to the end of remove should be ran only when is_bdist_wininst == False, right?

fname = osp.join(desktop_folder, 'Spyder.lnk')
if osp.isfile(fname):
try:
os.remove(fname)
except OSError:
print("Failed to remove %s; you may be able to remove it "\
"manually." % fname, file=sys.stderr)
else:
print("Successfully removed Spyder shortcuts from your desktop.",
file=sys.stdout)
# clean up startmenu
start_menu = osp.join(get_special_folder_path('CSIDL_PROGRAMS'),
'Spyder (Py%i.%i %i bit)' % (sys.version_info[0],
sys.version_info[1],
struct.calcsize('P')*8))
if osp.isdir(start_menu):
for fname in os.listdir(start_menu):
try:
os.remove(osp.join(start_menu,fname))
except OSError:
print("Failed to remove %s; you may be able to remove it "\
"manually." % fname, file=sys.stderr)
else:
print("Successfully removed Spyder shortcuts from your "\
" start menu.", file=sys.stdout)
try:
os.rmdir(start_menu)
except OSError:
print("Failed to remove %s; you may be able to remove it "\
"manually." % fname, file=sys.stderr)
else:
print("Successfully removed Spyder shortcut folder from your "\
" start menu.", file=sys.stdout)


if __name__=='__main__':
Expand All @@ -95,6 +219,7 @@ def remove():
try:
install()
except OSError:
# TODO: remove this because using user's startmenu not common
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please finish this TODO

print("Failed to create Start Menu items, try running "\
"installer as administrator.", file=sys.stderr)
elif sys.argv[1] == '-remove':
Expand Down