Add option to use templates from Zip files or Zip URLs #961

freakboy3742 · 2017-06-17T05:23:31Z

There are two reasons to add this feature:

Firstly, zip files are a convenient way to distribute a large number of files in a fixed structure. This means a cookiecutter can literally be a single file, rather than a directory, making it easier to distribute.

Secondly, this can be used to address #845 without cookiecutter itself needing to incorporate commercialisation features. The potential commercialisation path is as follows:

Developer uploads a Zip file to a private S3 bucket.
Developer deploys a website to sell their template.
User buys a license from the website
Developer website provides the user a public url, protected by username/password or access credentials of some kind
Any attempt to access the public URL is validated against the sales database. This can limit template uses to a particular count, or only allow access for a period of time, or any other scheme that the developer chooses.

Of course, this won't stop a malicious user from copying the template from the cache or anything like that - but from a "make it easy to do the right thing" perspective, this would be sufficient to monetise a template.

codecov-io · 2017-06-17T05:42:39Z

Codecov Report

Merging #961 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #961   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          17     18    +1     
  Lines         695    780   +85     
=====================================
+ Hits          695    780   +85

Impacted Files	Coverage Δ
cookiecutter/cli.py	`100% <ø> (ø)`	⬆️
cookiecutter/repository.py	`100% <100%> (ø)`	⬆️
cookiecutter/main.py	`100% <100%> (ø)`	⬆️
cookiecutter/vcs.py	`100% <100%> (ø)`	⬆️
cookiecutter/zipfile.py	`100% <100%> (ø)`
cookiecutter/utils.py	`100% <100%> (ø)`	⬆️
cookiecutter/prompt.py	`100% <100%> (ø)`	⬆️
cookiecutter/exceptions.py	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22df3a8...3936a80. Read the comment docs.

hackebrot · 2017-06-22T15:44:35Z

Hey @freakboy3742! 👋

Thanks for this PR! I like the idea and would be happy to add this functionality.

I haven't had the time to give this a thorough review yet. Hopefully I'll find some time during/after PyData Berlin and EuroPython. 🤞

What do you think @audreyr @pydanny @michaeljoseph?

michaeljoseph · 2017-06-22T16:26:49Z

Yeah, I'm 💯👍 on this- I'll try to find some time to review this weekend...

hackebrot

Thank you for your work on this @freakboy3742! 🙇 I made some comments, if could have a look please.

My main questions is whether we want to keep the downloaded zip files and how properly clean up after cookiecutter generated the project.

hackebrot · 2017-07-02T15:35:21Z

cookiecutter/repository.py

@@ -23,6 +24,11 @@ def is_repo_url(value):
    return bool(REPO_REGEX.match(value))


+def is_zip_file(value):
+    """Return True if value is a repository URL."""


This doc string needs updated.

hackebrot · 2017-07-02T15:38:35Z

cookiecutter/zipfile.py

+        ok_to_delete = read_user_yes_no(question, 'yes')
+
+    if ok_to_delete:
+        if os.path.isdir(path):


I think we can probably remove this as zip archives are always files?

Zip archives are; but the unpacked zip directory isn't. There's a second usage of this function on line 81 to purge an unpacked zip directory.

hackebrot · 2017-07-02T16:37:57Z

cookiecutter/zipfile.py

+        # Build the name of the cached zipfile,
+        # and prompt to delete if it already exists.
+        identifier = zip_url.rsplit('/', 1)[1]
+        zip_path = os.path.join(clone_to_dir, identifier)


I wonder if we should maybe download the zip archive to a temp directory, so ~/.cookiecutters (or w/e dir the user specified) will only contain "template directories". 🤔

We could then also delete the zip archive once it has been extracted

I did think about this; I ended up keeping both on the "what's a few more bytes?" principle. Effectively, we have three options:

Keep both (what is currently implemented)

Keep the zip file, and unpack to a temp directory

Keep the unpacked victory, and delete the zip file

Moving to (2) is a simple solution; but (3) gives us the opportunity (at some point in the future) use a password encoded zip file, adding to the potential security for commercial templates.

So - any preferences?

I'm voting for (3), largely because the mechanics of downloading and unzipping are just a means to an end (the template).

Also, I like my victory unpacked 😉

I think this would also make it a no-brainer to subsequently re-use prompt_and_delete (cookiecutter.repository is probably the natural destination of this refactor?)

hackebrot · 2017-07-02T16:42:30Z

cookiecutter/zipfile.py

+        zip_path = os.path.join(clone_to_dir, identifier)
+
+        if os.path.exists(zip_path):
+            ok_to_delete = prompt_and_delete(zip_path, no_input=no_input)


Related to above comment, we could prompt the user for deletion if the unzipped template with its identifier matches a directory in their cookiecutters_dir and then prompt for confirmation to delete.

hackebrot · 2017-07-02T17:00:35Z

cookiecutter/zipfile.py

+    # prompt for deletion. If we've previously OK'd deletion,
+    # don't ask again.
+    zip_file = ZipFile(zip_path)
+    unzip_name = zip_file.namelist()[0][:-1]


I think we should have some extra check here that the zipfile contains only one directory and error if it contains more than one.

Good point. I'll beef up the validation here.

Perhaps we should also attempt to catch exceptions related to ZipFile errors to wrap in a custom exception?

hackebrot · 2017-07-02T17:13:29Z

tests/repository/test_determine_repo_dir_clones_repo.py

+    return request.param
+
+
+def test_zipfile_unzip(


I'd probably go with a marker here:

@pytest.mark.parametrize('template, is_url', [ ('/path/to/zipfile.zip', False), ('https://example.com/path/to/zipfile.zip', True), ('http://example.com/path/to/zipfile.zip', True), ]) def test_zipfile_unzip(mocker, template, is_url, user_config_data): pass

michaeljoseph

This is a great PR, thanks @freakboy3742 🥇

michaeljoseph · 2017-07-03T20:10:26Z

tests/zipfile/test_unzip.py

+        autospec=True
+    )
+
+    def mock_download():


This inline function can be removed in favour of the existing module level one right?

michaeljoseph · 2017-07-03T20:57:04Z

cookiecutter/zipfile.py

+    return ok_to_delete
+
+
+def unzip(zip_url, is_url, clone_to_dir='.', no_input=False):


Could you add a docstring for this function please?

michaeljoseph · 2017-07-03T21:17:14Z

cookiecutter/zipfile.py

+        # Build the name of the cached zipfile,
+        # and prompt to delete if it already exists.
+        identifier = zip_url.rsplit('/', 1)[1]
+        zip_path = os.path.join(clone_to_dir, identifier)


I'm voting for (3), largely because the mechanics of downloading and unzipping are just a means to an end (the template).

Also, I like my victory unpacked 😉

michaeljoseph · 2017-07-03T21:19:34Z

cookiecutter/zipfile.py

+        # Build the name of the cached zipfile,
+        # and prompt to delete if it already exists.
+        identifier = zip_url.rsplit('/', 1)[1]
+        zip_path = os.path.join(clone_to_dir, identifier)


I think this would also make it a no-brainer to subsequently re-use prompt_and_delete (cookiecutter.repository is probably the natural destination of this refactor?)

michaeljoseph · 2017-07-03T21:19:40Z

cookiecutter/zipfile.py

+    # prompt for deletion. If we've previously OK'd deletion,
+    # don't ask again.
+    zip_file = ZipFile(zip_path)
+    unzip_name = zip_file.namelist()[0][:-1]


Perhaps we should also attempt to catch exceptions related to ZipFile errors to wrap in a custom exception?

michaeljoseph · 2017-07-03T21:21:16Z

tests/zipfile/test_prompt_and_delete.py

+        return_value=True,
+        autospec=True
+    )
+    dir = tmpdir.mkdir('repo')


Let's avoid shadowing builtin names?

michaeljoseph · 2017-07-03T21:24:00Z

tests/zipfile/test_unzip.py

+        clone_to_dir=str(clone_to_dir)
+    )
+
+    assert output_dir == os.path.join(str(clone_to_dir), 'fake-repo-tmpl')


Here and below, we can use the py.path.local api for consistency: clone_to_dir.join('fake-repo-tmpl')

theodesp · 2017-08-09T11:10:49Z

@freakboy3742 Can you review the requested changes?

freakboy3742 · 2017-08-09T14:35:57Z

@theodesp My apologies - I've been snowed under with other work. I'll take a look ASAP.

freakboy3742 · 2017-09-16T06:37:49Z

Apologies for the delay in updating this patch. I've now addressed the issues raised in the two reviews by @michaeljoseph and @hackebrot, and added a couple of extra improvements for good measure:

Updated docstrings as requested
Combined the vcs and zipfile versions of prompt_and_delete, and moved the result to the utils package (it can't be in repository as suggested because of circular dependencies)
Added an extra question to prompt_or_delete step that allows the user to reuse an existing template, instead of forcing a re-download. This is necessary for using cookiecutter on planes, on unreliable conference WiFi, etc.
Improved error handling for badly formed or invalid zip archives
Restructured some tests to make better use of pytest.mark.parametrize
Modify zipfile behavior to:
- Keep the original zipfile (if downloaded from a URL) in the .cookiecutter directory;
- When generating a template, unpack the zipfile into a temporary directory; and
- Delete the unpacked contents after use.
  This ensures that if the user retrieves a Zipfile repository, they won't have an unzipped version visible on their system for any longer than is necessary to generate the templated output.
Added ability to extract password-protected Zipfiles. Combined with the previous change, this means you can deliver a commercial template in a password-protected Zipfile. End users will only ever have access to generated output, not the raw template itself. This solution won't stop determined attackers, but it's strong enough to foil casual attempts at circumventing a commercial license.

freakboy3742 · 2017-09-16T06:56:49Z

Regarding the coverage results - there are 4 lines missed. Two of those lines are in cookiecutter/utils.py and were pre-existing coverage misses. The other two lines are in cookiecutter/zipfile.py, but aren't coverable - they're a backwards compatible import shim for the naming of the zipfile.BadZipFile exception (Lines 9-10) .

hackebrot

Great work @freakboy3742! 👏

Only a few minor changes and then this should be ready to go 🚀

hackebrot · 2017-09-22T07:52:45Z

cookiecutter/cli.py

@@ -116,6 +116,7 @@ def main(
            output_dir=output_dir,
            config_file=config_file,
            default_config=default_config,
+            password=os.environ.get('COOKIECUTTER_REPO_PASSWORD')
        )
    except (OutputDirExistsException,


We need to catch InvalidZipRepository exceptions here.

hackebrot · 2017-09-22T08:21:32Z

cookiecutter/repository.py

@@ -23,6 +24,11 @@ def is_repo_url(value):
    return bool(REPO_REGEX.match(value))


+def is_zip_file(value):
+    """Return True if value is a zip file."""
+    return value.endswith('.zip')


AFAIK file extensions on Windows are not case sensitive, so maybe we want to change this return value.lower().endswith('.zip').

hackebrot · 2017-09-22T08:33:31Z

cookiecutter/zipfile.py

+from zipfile import ZipFile
+try:
+    from zipfile import BadZipFile
+except ImportError:


Can you please add a comment mentioning that BadZipfile was deprecated in Python 3.2?

hackebrot · 2017-09-22T08:44:56Z

docs/usage.rst

@@ -69,6 +69,42 @@ type of repo that you want to use prepending `hg+` or `git+` to repo url::

    $ cookiecutter hg+https://example.com/repo

+Works with Zip files


I love that you always write great documentation for your changes, @freakboy3742!!! 📝 🙇

freakboy3742 · 2017-09-22T10:53:30Z

@hackebrot Thanks for the review - changes have been made!

hackebrot · 2017-09-22T11:29:08Z

Thank you, @freakboy3742! 🙇

hackebrot · 2017-10-14T15:12:27Z

Thank you for your work @freakboy3742! 🙇 🍪

freakboy3742 added 4 commits June 17, 2017 13:04

Added support for downloading and templating from Zip files.

6ca3f32

Added docs for zipfile templates.

9bf5215

Fixed flake8 problems.

9d22ecd

Fixes for Python 2.7 compatibility.

30c4e95

hackebrot added the enhancement This issue/PR relates to a feature request. label Jun 22, 2017

hackebrot requested changes Jul 2, 2017

View reviewed changes

michaeljoseph suggested changes Jul 3, 2017

View reviewed changes

freakboy3742 added 10 commits September 14, 2017 09:46

Merge branch 'master' into zipfile

6880882

Corrected docstring.

e139b29

Remove shadowing of builtins.

cc799c0

Add docstring for unzip method.

cbbff9e

Use pytest.mark.parametrize instead of a fixture.

cf5c1e7

Improved testing of zip file extraction problems.

5786603

After unrolling a zipfile template, delete the extracted files.

25008ab

Refactored prompt_and_delete, and added option to use old repo copy.

3cac20c

Added handling for password-protected repositories.

e72e8c8

Enable password to be passed in as a paraneter.

f637516

Fixed flake8 problems.

24420ad

freakboy3742 added 2 commits September 16, 2017 19:14

Minor change for Windows compatibility.

0e152ce

Another Windows compatibility fix.

508bc4b

hackebrot requested changes Sep 22, 2017

View reviewed changes

freakboy3742 added 2 commits September 22, 2017 18:51

Allow for capitalized file extensions (esp for Windows)

d2e2594

Catch invalid zip files at the CLI level.

8668961

Added note explaining the deprecation import.

3936a80

hackebrot approved these changes Sep 22, 2017

View reviewed changes

michaeljoseph approved these changes Oct 7, 2017

View reviewed changes

hackebrot merged commit a9d00ec into cookiecutter:master Oct 14, 2017

hackebrot added a commit that referenced this pull request Oct 14, 2017

Update HISTORY.rst for #961

5503196

hackebrot mentioned this pull request Oct 15, 2017

Release 1.6.0 #1001

Closed

		return ok_to_delete


		def unzip(zip_url, is_url, clone_to_dir='.', no_input=False):

		@@ -69,6 +69,42 @@ type of repo that you want to use prepending `hg+` or `git+` to repo url::

		$ cookiecutter hg+https://example.com/repo

		Works with Zip files

Add option to use templates from Zip files or Zip URLs #961

Add option to use templates from Zip files or Zip URLs #961

Uh oh!

Conversation

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!