swcarpentry / windows-installer

Software Carpentry installer for Windows.
MIT License
21 stars 17 forks source link

Only download if file doesn't exist in current directory #40

Open twhitehead opened 8 years ago

twhitehead commented 8 years ago

This changes the URL download code to first check if the file exists in the current directory and uses that if it does before going ahead and download it.

I think this is a good thing as it allows us to package the installer alongside the packages it downloads on a USB key and have people use it at workshops without a working network.

Note that I have only tested these changes by directly running the installer script on my Linux machine. I don't expect a Windows machine will be any different, but I don't have easy access to test it.

wking commented 8 years ago

On Fri, Jan 08, 2016 at 01:09:04PM -0800, Tyson Whitehead wrote:

I think this is a good thing as it allows us to package the installer alongside the packages it downloads on a USB key and have people use it at workshops without a working network.

Yeah, sounds good to me.

Note that I have only tested these changes by directly running the installer script on my Linux machine. I don't expect a Windows machine will be any different, but I don't have easy access to test it.

My concern here would be that os.path.basename would be splitting on backslashes on Windows. I expect we want:

url.split('/')[-1]

And to avoid a race between isfile and open, I'd just try/catch:

try: r = open(url.split('/')[-1]) except (IOError, FileNotFoundError): r = _urlopen(url)

The two exceptions allow us to work with either Python 2 or 3 (after the great exception reorganization [1,2,3]).

twhitehead commented 8 years ago

Those both sound like good points to me. I'll make the corrections

twhitehead commented 8 years ago

Specifying the Python 2 and 3 exceptions doesn't work under Python 2. Here is the backtrace

Traceback (most recent call last): File "./swc-windows-installer.py", line 329, in main() File "./swc-windows-installer.py", line 296, in main install_nano(install_directory=nano_dir) File "./swc-windows-installer.py", line 176, in install_nano install_directory=install_directory) File "./swc-windows-installer.py", line 144, in zip_install zip_bytes = download(url=url, sha1=sha1) File "./swc-windows-installer.py", line 74, in download except (IOError,FileNotFoundError): NameError: global name 'FileNotFoundError' is not defined

One solution that occurs to me would be to take advantage of your existing Python 2 vs 3 branch to make sure FileNotFoundError is always defined and then just catch it.

try:  # Python 3
    from urllib.request import urlopen as _urlopen
except ImportError:  # Python 2
    from urllib2 import urlopen as _urlopen
    FileNotFoundError = IOError
wking commented 8 years ago

On Fri, Jan 08, 2016 at 05:31:10PM -0800, Tyson Whitehead wrote:

One solution that occurs to me would be to take advantage of your existing Python 2 vs 3 branch to make sure FileNotFoundError is always defined and then just catch it.

try:  # Python 3
    from urllib.request import urlopen as _urlopen
except ImportError:  # Python 2
    from urllib2 import urlopen as _urlopen
    FileNotFoundError = IOError

Ah, that works. I'd prefer a separate:

if sys.version_info.major < 3: FileNotFoundError = IOError

after the imports though, to avoid crowding the urlopen section.

embray commented 8 years ago

LGTM modulo the change @wking suggested. I can finish this up if you don't have the chance @twhitehead