Fix error when overwriting a bad symbolic link installing an nbextension

The unit test fails before this fix and works after this fix.
Basically, if the already-installed symlink is a bad link, .exists()
returns False (so the existing link is not removed), while .lexists()
will return True (so the existing link is removed).

IPython/html/nbextensions.py

@@ -1,350 +1,350 b''
 # coding: utf-8
 """Utilities for installing Javascript extensions for the notebook"""
 
 # Copyright (c) IPython Development Team.
 # Distributed under the terms of the Modified BSD License.
 
 from __future__ import print_function
 
 import os
 import shutil
 import sys
 import tarfile
 import zipfile
 import uuid
 from os.path import basename, join as pjoin
 
 # Deferred imports
 try:
     from urllib.parse import urlparse  # Py3
     from urllib.request import urlretrieve
 except ImportError:
     from urlparse import urlparse
     from urllib import urlretrieve
 
 from IPython.utils.path import get_ipython_dir, ensure_dir_exists
 from IPython.utils.py3compat import string_types, cast_unicode_py2
 from IPython.utils.tempdir import TemporaryDirectory
 
 class ArgumentConflict(ValueError):
     pass
 
 # Packagers: modify the next block if you store system-installed nbextensions elsewhere (unlikely)
 SYSTEM_NBEXTENSIONS_DIRS = []
 
 if os.name == 'nt':
     programdata = os.environ.get('PROGRAMDATA', None)
     if programdata: # PROGRAMDATA is not defined by default on XP.
         SYSTEM_NBEXTENSIONS_DIRS = [pjoin(programdata, 'jupyter', 'nbextensions')]
     prefixes = []
 else:
     prefixes = [os.path.sep + pjoin('usr', 'local'), os.path.sep + 'usr']
 
 # add sys.prefix at the front
 if sys.prefix not in prefixes:
     prefixes.insert(0, sys.prefix)
 
 for prefix in prefixes:
     nbext = pjoin(prefix, 'share', 'jupyter', 'nbextensions')
     if nbext not in SYSTEM_NBEXTENSIONS_DIRS:
         SYSTEM_NBEXTENSIONS_DIRS.append(nbext)
 
 if os.name == 'nt':
     # PROGRAMDATA
     SYSTEM_NBEXTENSIONS_INSTALL_DIR = SYSTEM_NBEXTENSIONS_DIRS[-1]
 else:
     # /usr/local
     SYSTEM_NBEXTENSIONS_INSTALL_DIR = SYSTEM_NBEXTENSIONS_DIRS[-2]
 
 
 def _should_copy(src, dest, verbose=1):
     """should a file be copied?"""
     if not os.path.exists(dest):
         return True
     if os.stat(src).st_mtime - os.stat(dest).st_mtime > 1e-6:
         # we add a fudge factor to work around a bug in python 2.x
         # that was fixed in python 3.x: http://bugs.python.org/issue12904
         if verbose >= 2:
             print("%s is out of date" % dest)
         return True
     if verbose >= 2:
         print("%s is up to date" % dest)
     return False
 
 
 def _maybe_copy(src, dest, verbose=1):
     """copy a file if it needs updating"""
     if _should_copy(src, dest, verbose):
         if verbose >= 1:
             print("copying %s -> %s" % (src, dest))
         shutil.copy2(src, dest)
 
 def _safe_is_tarfile(path):
     """safe version of is_tarfile, return False on IOError"""
     try:
         return tarfile.is_tarfile(path)
     except IOError:
         return False
 
 
 def _get_nbext_dir(nbextensions_dir=None, user=False, prefix=None):
     """Return the nbextension directory specified"""
     if sum(map(bool, [user, prefix, nbextensions_dir])) > 1:
         raise ArgumentConflict("Cannot specify more than one of user, prefix, or nbextensions_dir.")
     if user:
         nbext = pjoin(get_ipython_dir(), u'nbextensions')
     else:
         if prefix:
             nbext = pjoin(prefix, 'share', 'jupyter', 'nbextensions')
         elif nbextensions_dir:
             nbext = nbextensions_dir
         else:
             nbext = SYSTEM_NBEXTENSIONS_INSTALL_DIR
     return nbext
 
 
 def check_nbextension(files, user=False, prefix=None, nbextensions_dir=None):
     """Check whether nbextension files have been installed
     
     Returns True if all files are found, False if any are missing.
 
     Parameters
     ----------
 
     files : list(paths)
         a list of relative paths within nbextensions.
     user : bool [default: False]
         Whether to check the user's .ipython/nbextensions directory.
         Otherwise check a system-wide install (e.g. /usr/local/share/jupyter/nbextensions).
     prefix : str [optional]
         Specify install prefix, if it should differ from default (e.g. /usr/local).
         Will check prefix/share/jupyter/nbextensions
     nbextensions_dir : str [optional]
         Specify absolute path of nbextensions directory explicitly.
     """
     nbext = _get_nbext_dir(nbextensions_dir, user, prefix)
     # make sure nbextensions dir exists
     if not os.path.exists(nbext):
         return False
     
     if isinstance(files, string_types):
         # one file given, turn it into a list
         files = [files]
     
     return all(os.path.exists(pjoin(nbext, f)) for f in files)
 
 
 def install_nbextension(path, overwrite=False, symlink=False, user=False, prefix=None, nbextensions_dir=None, destination=None, verbose=1):
     """Install a Javascript extension for the notebook
     
     Stages files and/or directories into the nbextensions directory.
     By default, this compares modification time, and only stages files that need updating.
     If `overwrite` is specified, matching files are purged before proceeding.
     
     Parameters
     ----------
     
     path : path to file, directory, zip or tarball archive, or URL to install
         By default, the file will be installed with its base name, so '/path/to/foo'
         will install to 'nbextensions/foo'. See the destination argument below to change this.
         Archives (zip or tarballs) will be extracted into the nbextensions directory.
     overwrite : bool [default: False]
         If True, always install the files, regardless of what may already be installed.
     symlink : bool [default: False]
         If True, create a symlink in nbextensions, rather than copying files.
         Not allowed with URLs or archives. Windows support for symlinks requires
         Vista or above, Python 3, and a permission bit which only admin users
         have by default, so don't rely on it.
     user : bool [default: False]
         Whether to install to the user's .ipython/nbextensions directory.
         Otherwise do a system-wide install (e.g. /usr/local/share/jupyter/nbextensions).
     prefix : str [optional]
         Specify install prefix, if it should differ from default (e.g. /usr/local).
         Will install to prefix/share/jupyter/nbextensions
     nbextensions_dir : str [optional]
         Specify absolute path of nbextensions directory explicitly.
     destination : str [optional]
         name the nbextension is installed to.  For example, if destination is 'foo', then
         the source file will be installed to 'nbextensions/foo', regardless of the source name.
         This cannot be specified if an archive is given as the source.
     verbose : int [default: 1]
         Set verbosity level. The default is 1, where file actions are printed.
         set verbose=2 for more output, or verbose=0 for silence.
     """
     nbext = _get_nbext_dir(nbextensions_dir, user, prefix)
     # make sure nbextensions dir exists
     ensure_dir_exists(nbext)
     
     if isinstance(path, (list, tuple)):
         raise TypeError("path must be a string pointing to a single extension to install; call this function multiple times to install multiple extensions")
     
     path = cast_unicode_py2(path)
 
     if path.startswith(('https://', 'http://')):
         if symlink:
             raise ValueError("Cannot symlink from URLs")
         # Given a URL, download it
         with TemporaryDirectory() as td:
             filename = urlparse(path).path.split('/')[-1]
             local_path = os.path.join(td, filename)
             if verbose >= 1:
                 print("downloading %s to %s" % (path, local_path))
             urlretrieve(path, local_path)
             # now install from the local copy
             install_nbextension(local_path, overwrite=overwrite, symlink=symlink, nbextensions_dir=nbext, destination=destination, verbose=verbose)
     elif path.endswith('.zip') or _safe_is_tarfile(path):
         if symlink:
             raise ValueError("Cannot symlink from archives")
         if destination:
             raise ValueError("Cannot give destination for archives")
         if verbose >= 1:
             print("extracting %s to %s" % (path, nbext))
 
         if path.endswith('.zip'):
             archive = zipfile.ZipFile(path)
         elif _safe_is_tarfile(path):
             archive = tarfile.open(path)
         archive.extractall(nbext)
         archive.close()
     else:
         if not destination:
             destination = basename(path)
         destination = cast_unicode_py2(destination)
         full_dest = pjoin(nbext, destination)
-        if overwrite and os.path.exists(full_dest):
+        if overwrite and os.path.lexists(full_dest):
             if verbose >= 1:
                 print("removing %s" % full_dest)
             if os.path.isdir(full_dest) and not os.path.islink(full_dest):
                 shutil.rmtree(full_dest)
             else:
                 os.remove(full_dest)
 
         if symlink:
             path = os.path.abspath(path)
             if not os.path.exists(full_dest):
                 if verbose >= 1:
                     print("symlink %s -> %s" % (full_dest, path))
                 os.symlink(path, full_dest)
         elif os.path.isdir(path):
             path = pjoin(os.path.abspath(path), '') # end in path separator
             for parent, dirs, files in os.walk(path):
                 dest_dir = pjoin(full_dest, parent[len(path):])
                 if not os.path.exists(dest_dir):
                     if verbose >= 2:
                         print("making directory %s" % dest_dir)
                     os.makedirs(dest_dir)
                 for file in files:
                     src = pjoin(parent, file)
                     # print("%r, %r" % (dest_dir, file))
                     dest_file = pjoin(dest_dir, file)
                     _maybe_copy(src, dest_file, verbose)
         else:
             src = path
             _maybe_copy(src, full_dest, verbose)
 
 #----------------------------------------------------------------------
 # install nbextension app
 #----------------------------------------------------------------------
 
 from IPython.utils.traitlets import Bool, Enum, Unicode, TraitError
 from IPython.core.application import BaseIPythonApplication
 
 flags = {
     "overwrite" : ({
         "NBExtensionApp" : {
             "overwrite" : True,
         }}, "Force overwrite of existing files"
     ),
     "debug" : ({
         "NBExtensionApp" : {
             "verbose" : 2,
         }}, "Extra output"
     ),
     "quiet" : ({
         "NBExtensionApp" : {
             "verbose" : 0,
         }}, "Minimal output"
     ),
     "symlink" : ({
         "NBExtensionApp" : {
             "symlink" : True,
         }}, "Create symlink instead of copying files"
     ),
     "user" : ({
         "NBExtensionApp" : {
             "user" : True,
         }}, "Install to the user's IPython directory"
     ),
 }
 flags['s'] = flags['symlink']
 
 aliases = {
     "ipython-dir" : "NBExtensionApp.ipython_dir",
     "prefix" : "NBExtensionApp.prefix",
     "nbextensions" : "NBExtensionApp.nbextensions_dir",
     "destination" : "NBExtensionApp.destination",
 }
 
 class NBExtensionApp(BaseIPythonApplication):
     """Entry point for installing notebook extensions"""
     
     description = """Install IPython notebook extensions
     
     Usage
     
         ipython install-nbextension path/url
     
     This copies a file or a folder into the IPython nbextensions directory.
     If a URL is given, it will be downloaded.
     If an archive is given, it will be extracted into nbextensions.
     If the requested files are already up to date, no action is taken
     unless --overwrite is specified.
     """
     
     examples = """
     ipython install-nbextension /path/to/myextension
     """
     aliases = aliases
     flags = flags
     
     overwrite = Bool(False, config=True, help="Force overwrite of existing files")
     symlink = Bool(False, config=True, help="Create symlinks instead of copying files")
     user = Bool(False, config=True, help="Whether to do a user install")
     prefix = Unicode('', config=True, help="Installation prefix")
     nbextensions_dir = Unicode('', config=True, help="Full path to nbextensions dir (probably use prefix or user)")
     destination = Unicode('', config=True, help="Destination for the copy or symlink")
     verbose = Enum((0,1,2), default_value=1, config=True,
         help="Verbosity level"
     )
     
     def install_extensions(self):
         if len(self.extra_args)>1:
             raise ValueError("only one nbextension allowed at a time.  Call multiple times to install multiple extensions.")
         install_nbextension(self.extra_args[0],
             overwrite=self.overwrite,
             symlink=self.symlink,
             verbose=self.verbose,
             user=self.user,
             prefix=self.prefix,
             destination=self.destination,
             nbextensions_dir=self.nbextensions_dir,
         )
     
     def start(self):
         if not self.extra_args:
             for nbext in [pjoin(self.ipython_dir, u'nbextensions')] + SYSTEM_NBEXTENSIONS_DIRS:
                 if os.path.exists(nbext):
                     print("Notebook extensions in %s:" % nbext)
                     for ext in os.listdir(nbext):
                         print(u"    %s" % ext)
         else:
             try:
                 self.install_extensions()
             except ArgumentConflict as e:
                 print(str(e), file=sys.stderr)
                 self.exit(1)
 
 
 if __name__ == '__main__':
     NBExtensionApp.launch_instance()
     

IPython/html/tests/test_nbextensions.py

@@ -1,316 +1,332 b''
 # coding: utf-8
 """Test installation of notebook extensions"""
 
 # Copyright (c) IPython Development Team.
 # Distributed under the terms of the Modified BSD License.
 
 import glob
 import os
 import re
 import tarfile
 import zipfile
 from io import BytesIO
 from os.path import basename, join as pjoin
 from unittest import TestCase
 
 import IPython.testing.tools as tt
 import IPython.testing.decorators as dec
 from IPython.utils import py3compat
 from IPython.utils.tempdir import TemporaryDirectory
 from IPython.html import nbextensions
 from IPython.html.nbextensions import install_nbextension, check_nbextension
 
 
 def touch(file, mtime=None):
     """ensure a file exists, and set its modification time
     
     returns the modification time of the file
     """
     open(file, 'a').close()
     # set explicit mtime
     if mtime:
         atime = os.stat(file).st_atime
         os.utime(file, (atime, mtime))
     return os.stat(file).st_mtime
 
 class TestInstallNBExtension(TestCase):
     
     def tempdir(self):
         td = TemporaryDirectory()
         self.tempdirs.append(td)
         return py3compat.cast_unicode(td.name)
 
     def setUp(self):
         self.tempdirs = []
         src = self.src = self.tempdir()
         self.files = files = [
             pjoin(u'ƒile'),
             pjoin(u'∂ir', u'ƒile1'),
             pjoin(u'∂ir', u'∂ir2', u'ƒile2'),
         ]
         for file in files:
             fullpath = os.path.join(self.src, file)
             parent = os.path.dirname(fullpath)
             if not os.path.exists(parent):
                 os.makedirs(parent)
             touch(fullpath)
         
         self.ipdir = self.tempdir()
         self.save_get_ipython_dir = nbextensions.get_ipython_dir
         nbextensions.get_ipython_dir = lambda : self.ipdir
         self.save_system_dir = nbextensions.SYSTEM_NBEXTENSIONS_INSTALL_DIR
         nbextensions.SYSTEM_NBEXTENSIONS_INSTALL_DIR = self.system_nbext = self.tempdir()
     
     def tearDown(self):
         nbextensions.get_ipython_dir = self.save_get_ipython_dir
         nbextensions.SYSTEM_NBEXTENSIONS_INSTALL_DIR = self.save_system_dir
         for td in self.tempdirs:
             td.cleanup()
 
     def assert_dir_exists(self, path):
         if not os.path.exists(path):
             do_exist = os.listdir(os.path.dirname(path))
             self.fail(u"%s should exist (found %s)" % (path, do_exist))
     
     def assert_not_dir_exists(self, path):
         if os.path.exists(path):
             self.fail(u"%s should not exist" % path)
     
     def assert_installed(self, relative_path, user=False):
         if user:
             nbext = pjoin(self.ipdir, u'nbextensions')
         else:
             nbext = self.system_nbext
         self.assert_dir_exists(
             pjoin(nbext, relative_path)
         )
     
     def assert_not_installed(self, relative_path, user=False):
         if user:
             nbext = pjoin(self.ipdir, u'nbextensions')
         else:
             nbext = self.system_nbext
         self.assert_not_dir_exists(
             pjoin(nbext, relative_path)
         )
     
     def test_create_ipython_dir(self):
         """install_nbextension when ipython_dir doesn't exist"""
         with TemporaryDirectory() as td:
             self.ipdir = ipdir = pjoin(td, u'ipython')
             install_nbextension(self.src, user=True)
             self.assert_dir_exists(ipdir)
             for file in self.files:
                 self.assert_installed(
                     pjoin(basename(self.src), file),
                     user=bool(ipdir)
                 )
     
     def test_create_nbextensions_user(self):
         with TemporaryDirectory() as td:
             self.ipdir = ipdir = pjoin(td, u'ipython')
             install_nbextension(self.src, user=True)
             self.assert_installed(
                 pjoin(basename(self.src), u'ƒile'),
                 user=True
             )
     
     def test_create_nbextensions_system(self):
         with TemporaryDirectory() as td:
             nbextensions.SYSTEM_NBEXTENSIONS_INSTALL_DIR = self.system_nbext = pjoin(td, u'nbextensions')
             install_nbextension(self.src, user=False)
             self.assert_installed(
                 pjoin(basename(self.src), u'ƒile'),
                 user=False
             )
     
     def test_single_file(self):
         file = self.files[0]
         install_nbextension(pjoin(self.src, file))
         self.assert_installed(file)
     
     def test_single_dir(self):
         d = u'∂ir'
         install_nbextension(pjoin(self.src, d))
         self.assert_installed(self.files[-1])
     
 
     def test_destination_file(self):
         file = self.files[0]
         install_nbextension(pjoin(self.src, file), destination = u'ƒiledest')
         self.assert_installed(u'ƒiledest')
 
     def test_destination_dir(self):
         d = u'∂ir'
         install_nbextension(pjoin(self.src, d), destination = u'ƒiledest2')
         self.assert_installed(pjoin(u'ƒiledest2', u'∂ir2', u'ƒile2'))
     
     def test_install_nbextension(self):
         with self.assertRaises(TypeError):
             install_nbextension(glob.glob(pjoin(self.src, '*')))
     
     def test_overwrite_file(self):
         with TemporaryDirectory() as d:
             fname = u'ƒ.js'
             src = pjoin(d, fname)
             with open(src, 'w') as f:
                 f.write('first')
             mtime = touch(src)
             dest = pjoin(self.system_nbext, fname)
             install_nbextension(src)
             with open(src, 'w') as f:
                 f.write('overwrite')
             mtime = touch(src, mtime - 100)
             install_nbextension(src, overwrite=True)
             with open(dest) as f:
                 self.assertEqual(f.read(), 'overwrite')
     
     def test_overwrite_dir(self):
         with TemporaryDirectory() as src:
             base = basename(src)
             fname = u'ƒ.js'
             touch(pjoin(src, fname))
             install_nbextension(src)
             self.assert_installed(pjoin(base, fname))
             os.remove(pjoin(src, fname))
             fname2 = u'∂.js'
             touch(pjoin(src, fname2))
             install_nbextension(src, overwrite=True)
             self.assert_installed(pjoin(base, fname2))
             self.assert_not_installed(pjoin(base, fname))
     
     def test_update_file(self):
         with TemporaryDirectory() as d:
             fname = u'ƒ.js'
             src = pjoin(d, fname)
             with open(src, 'w') as f:
                 f.write('first')
             mtime = touch(src)
             install_nbextension(src)
             self.assert_installed(fname)
             dest = pjoin(self.system_nbext, fname)
             old_mtime = os.stat(dest).st_mtime
             with open(src, 'w') as f:
                 f.write('overwrite')
             touch(src, mtime + 10)
             install_nbextension(src)
             with open(dest) as f:
                 self.assertEqual(f.read(), 'overwrite')
     
     def test_skip_old_file(self):
         with TemporaryDirectory() as d:
             fname = u'ƒ.js'
             src = pjoin(d, fname)
             mtime = touch(src)
             install_nbextension(src)
             self.assert_installed(fname)
             dest = pjoin(self.system_nbext, fname)
             old_mtime = os.stat(dest).st_mtime
             
             mtime = touch(src, mtime - 100)
             install_nbextension(src)
             new_mtime = os.stat(dest).st_mtime
             self.assertEqual(new_mtime, old_mtime)
 
     def test_quiet(self):
         with tt.AssertNotPrints(re.compile(r'.+')):
             install_nbextension(self.src, verbose=0)
     
     def test_install_zip(self):
         path = pjoin(self.src, "myjsext.zip")
         with zipfile.ZipFile(path, 'w') as f:
             f.writestr("a.js", b"b();")
             f.writestr("foo/a.js", b"foo();")
         install_nbextension(path)
         self.assert_installed("a.js")
         self.assert_installed(pjoin("foo", "a.js"))
     
     def test_install_tar(self):
         def _add_file(f, fname, buf):
             info = tarfile.TarInfo(fname)
             info.size = len(buf)
             f.addfile(info, BytesIO(buf))
         
         for i,ext in enumerate((".tar.gz", ".tgz", ".tar.bz2")):
             path = pjoin(self.src, "myjsext" + ext)
             with tarfile.open(path, 'w') as f:
                 _add_file(f, "b%i.js" % i, b"b();")
                 _add_file(f, "foo/b%i.js" % i, b"foo();")
             install_nbextension(path)
             self.assert_installed("b%i.js" % i)
             self.assert_installed(pjoin("foo", "b%i.js" % i))
     
     def test_install_url(self):
         def fake_urlretrieve(url, dest):
             touch(dest)
         save_urlretrieve = nbextensions.urlretrieve
         nbextensions.urlretrieve = fake_urlretrieve
         try:
             install_nbextension("http://example.com/path/to/foo.js")
             self.assert_installed("foo.js")
             install_nbextension("https://example.com/path/to/another/bar.js")
             self.assert_installed("bar.js")
             install_nbextension("https://example.com/path/to/another/bar.js", 
                                 destination = 'foobar.js')
             self.assert_installed("foobar.js")
         finally:
             nbextensions.urlretrieve = save_urlretrieve
     
     def test_check_nbextension(self):
         with TemporaryDirectory() as d:
             f = u'ƒ.js'
             src = pjoin(d, f)
             touch(src)
             install_nbextension(src, user=True)
         
         assert check_nbextension(f, user=True)
         assert check_nbextension([f], user=True)
         assert not check_nbextension([f, pjoin('dne', f)], user=True)
     
     @dec.skip_win32
     def test_install_symlink(self):
         with TemporaryDirectory() as d:
             f = u'ƒ.js'
             src = pjoin(d, f)
             touch(src)
             install_nbextension(src, symlink=True)
         dest = pjoin(self.system_nbext, f)
         assert os.path.islink(dest)
         link = os.readlink(dest)
         self.assertEqual(link, src)
     
     @dec.skip_win32
+    def test_overwrite_broken_symlink(self):
+        with TemporaryDirectory() as d:
+            f = u'ƒ.js'
+            f2 = u'ƒ2.js'
+            src = pjoin(d, f)
+            src2 = pjoin(d, f2)
+            touch(src)
+            install_nbextension(src, symlink=True)
+            os.rename(src, src2)
+            install_nbextension(src2, symlink=True, overwrite=True, destination=f)
+        dest = pjoin(self.system_nbext, f)
+        assert os.path.islink(dest)
+        link = os.readlink(dest)
+        self.assertEqual(link, src2)
+
+    @dec.skip_win32
     def test_install_symlink_destination(self):
         with TemporaryDirectory() as d:
             f = u'ƒ.js'
             flink = u'ƒlink.js'
             src = pjoin(d, f)
             touch(src)
             install_nbextension(src, symlink=True, destination=flink)
         dest = pjoin(self.system_nbext, flink)
         assert os.path.islink(dest)
         link = os.readlink(dest)
         self.assertEqual(link, src)
 
     def test_install_symlink_bad(self):
         with self.assertRaises(ValueError):
             install_nbextension("http://example.com/foo.js", symlink=True)
         
         with TemporaryDirectory() as d:
             zf = u'ƒ.zip'
             zsrc = pjoin(d, zf)
             with zipfile.ZipFile(zsrc, 'w') as z:
                 z.writestr("a.js", b"b();")
         
             with self.assertRaises(ValueError):
                 install_nbextension(zsrc, symlink=True)
 
     def test_install_destination_bad(self):
         with TemporaryDirectory() as d:
             zf = u'ƒ.zip'
             zsrc = pjoin(d, zf)
             with zipfile.ZipFile(zsrc, 'w') as z:
                 z.writestr("a.js", b"b();")
         
             with self.assertRaises(ValueError):
                 install_nbextension(zsrc, destination='foo')