Fixed #22

 - Implement a new command named 'collect_redist_src'
 - Have option to copy with or without directory structure
 - Able to take a CSV/JSON inventory as an input
 - Able to output to a zip file
 - Add some test code

Author: chinyeungli

.gitignore

@@ -49,3 +49,4 @@ docs/_build
 /tcl/
 /.python-version
 /.tox/
+/.pytest_cache/

src/attributecode/cmd.py

@@ -38,12 +38,13 @@
 from attributecode.attrib import check_template
 from attributecode.attrib import DEFAULT_TEMPLATE_FILE
 from attributecode.attrib import generate_and_save as generate_attribution_doc
-from attributecode.gen import generate as generate_about_files
-from attributecode.model import collect_inventory
+from attributecode.gen import generate as generate_about_files, load_inventory
+from attributecode.model import collect_inventory, get_copy_list
 from attributecode.model import copy_redist_src
 from attributecode.model import write_output
 from attributecode.util import extract_zip
 from attributecode.util import filter_errors
+from attributecode.util import get_temp_dir
 
 
 __copyright__ = """
@@ -376,8 +377,21 @@ def attrib(location, output, template, vartext, quiet, verbose):
 
 @click.argument('output',
     required=True,
-    metavar='OUTPUT',
-    type=click.Path(exists=True, file_okay=False, writable=True, resolve_path=True))
+    metavar='OUTPUT')
+
+@click.option('--from-inventory',
+    metavar='FILE',
+    type=click.Path(exists=True, dir_okay=False, readable=True, resolve_path=True),
+    help='Path to an inventory CSV/JSON file as the base list for files/directories ' 
+         'that need to be copied which have the \'redistribute\' flagged.')
+
+@click.option('--with-structures',
+    is_flag=True,
+    help='Copy sources with directory structure.')
+
+@click.option('--zip',
+    is_flag=True,
+    help='Zip the copied sources to the output location.')
 
 @click.option('-q', '--quiet',
     is_flag=True,
@@ -389,14 +403,21 @@ def attrib(location, output, template, vartext, quiet, verbose):
 
 @click.help_option('-h', '--help')
 
-def collect_redist_src(location, output, quiet, verbose):
+def collect_redist_src(location, output, from_inventory, with_structures, zip, quiet, verbose):
     """
-Collect sources that have 'redistribute' flagged to the output location.
+Collect sources that have 'redistribute' flagged in .ABOUT files or inventory
+to the output location.
 
-LOCATION: Path to a file or directory containing .ABOUT files.
+LOCATION: Path to a directory containing sources that need to be copied
+(and containing ABOUT files if `inventory` is not provided)
 
-OUTPUT: Path to a directory where sources will be copied to.
+OUTPUT: Path to a directory or a zip file where sources will be copied to.
     """
+    if zip:
+        if not output.endswith('.zip'):
+            click.echo('The output needs to be a zip file.')
+            sys.exit()
+
     if not quiet:
         print_version()
         click.echo('Collecting inventory from ABOUT files...')
@@ -405,9 +426,25 @@ def collect_redist_src(location, output, quiet, verbose):
         # accept zipped ABOUT files as input
         location = extract_zip(location)
 
-    errors, abouts = collect_inventory(location)
-    copy_errors = copy_redist_src(abouts, location, output)
+    if from_inventory:
+        errors, abouts = load_inventory(from_inventory, location)
+    else:
+        errors, abouts = collect_inventory(location)
+
+    if zip:
+        # Copy to a temp location and the zip to the ouput location
+        output_location = get_temp_dir()
+    else:
+        output_location = output
+
+    copy_list, copy_list_errors = get_copy_list(abouts, location)
+    copy_errors = copy_redist_src(copy_list, location, output_location, with_structures)
+
+    if zip:
+        import shutil
+        shutil.make_archive(output, 'zip', output_location)
 
+    errors.extend(copy_list_errors)
     errors.extend(copy_errors)
     errors_count = report_errors(errors, quiet, verbose, log_file_loc=output + '-error.log')
     if not quiet:

src/attributecode/model.py

@@ -1271,12 +1271,45 @@ def get_field_names(abouts):
     return fields
 
 
-def copy_redist_src(abouts, location, output):
+def copy_redist_src(copy_list, location, output, with_structure):
     """
-    Given a list of About objects, copy the referenced source (file or directory)
-    to the output location if the 'redistribute' field is set to True. 
+    Given a list of files/directories and copy to the destination
     """
     errors = []
+    for from_path in copy_list:
+        norm_from_path = norm(from_path)
+        relative_from_path = norm_from_path.partition(util.norm(location))[2]
+        # Need to strip the '/' to use the join
+        if relative_from_path.startswith('/'):
+            relative_from_path = relative_from_path.partition('/')[2]
+        # Get the directory name of the output path
+        if with_structure:
+            output_dir = os.path.dirname(os.path.join(output, util.norm(relative_from_path)))
+        else:
+            output_dir = output
+        err = copy_file(from_path, output_dir)
+        if err:
+            errors.extend(err)
+    return errors
+
+
+def get_copy_list(abouts, location):
+    """
+    Return a list of files/directories that need to be copied (and error if any)
+    This is a summary list in a sense that if a directory is already in the list,
+    its children directories/files will not be included in the list regardless if
+    they have 'redistribute' flagged. The reason for this is we want to capture 
+    the error/warning if existence files/directories already exist. However, if
+    we don't have this "summarized" list, and we've copied a file (with directory structure)
+    and then later on this file's parent directory also need to be copied, then
+    it will prompt warning as the directory that need to be copied is already exist.
+    Technically, this is correct, but it leads to confusion. Therefore, we want to
+    create a summarized list to avoid this kind of confusion.
+    """
+    errors = []
+    copy_list = []
+    dir_list = []
+    file_list = []
     for about in abouts:
         if about.redistribute.value:
             file_exist = True
@@ -1290,17 +1323,57 @@ def copy_redist_src(abouts, location, output):
                 for k in about.about_resource.value:
                     from_path = about.about_resource.value.get(k)
                     norm_from_path = norm(from_path)
+                    # Get the relative path
                     relative_from_path = norm_from_path.partition(util.norm(location))[2]
-                    # Need to strip the '/' to use the join
-                    if relative_from_path.startswith('/'):
-                        relative_from_path = relative_from_path.partition('/')[2]
-                    # Get the directory name of the output path
-                    output_dir = os.path.dirname(os.path.join(output, util.norm(relative_from_path)))
-                    err = copy_file(from_path, output_dir)
-                    if err:
-                        errors.extend(err)
-    return errors
+                    if os.path.isdir(from_path):
+                        if not dir_list:
+                            dir_list.append(relative_from_path)
+                        else:
+                            handled = False
+                            for dir in dir_list:
+                                # The dir is a parent of the relative_from_path
+                                if dir in relative_from_path:
+                                    handled = True
+                                    continue
+                                # The relative_from_path is the parent of the dir
+                                # We need to update the dir_list
+                                if relative_from_path in dir:
+                                    dir_list.remove(dir)
+                                    dir_list.append(relative_from_path)
+                                    handled = True
+                                    continue
+                            if not handled:
+                                dir_list.append(relative_from_path)
+                    else:
+                        # Check if the file is from "root"
+                        # If the file is at root level, it'll add to the copy_list
+                        if not os.path.dirname(relative_from_path) == '/':
+                            file_list.append(relative_from_path)
+                        else:
+                            copy_list.append(from_path)
+
+    for dir in dir_list:
+        for f in file_list:
+            # The file is already in one of copied directories
+            if dir in f:
+                file_list.remove(f)
+                continue
+        if dir.startswith('/'):
+            dir = dir.partition('/')[2]
+        absolute_path = os.path.join(location, dir)
+        if on_windows:
+            absolute_path = add_unc(absolute_path)
+        copy_list.append(absolute_path)
+
+    for f in file_list:
+        if f.startswith('/'):
+            f = f.partition('/')[2]
+        absolute_path = os.path.join(location, f)
+        if on_windows:
+            absolute_path = add_unc(absolute_path)
+        copy_list.append(absolute_path)
 
+    return copy_list, errors
 
 def about_object_to_list_of_dictionary(abouts):
     """

src/attributecode/util.py

@@ -28,6 +28,8 @@
 import string
 import sys
 
+from distutils.dir_util import copy_tree
+
 from attributecode import CRITICAL
 from attributecode import WARNING
 from attributecode import Error
@@ -478,6 +480,7 @@ def copy_license_notice_files(fields, base_dir, reference_dir, afp):
 
 
 def copy_file(from_path, to_path):
+    error = []
     # Return if the from_path is empty or None.
     if not from_path:
         return []
@@ -489,11 +492,10 @@ def copy_file(from_path, to_path):
             to_path = add_unc(to_path)
 
     # Strip the white spaces
-    from_path = from_path.strip()
+    from_path = norm(from_path.strip())
     to_path = to_path.strip()
-
     # Errors will be captured when doing the validation
-    if not posixpath.exists(from_path):
+    if not os.path.exists(from_path):
         return []
 
     if not posixpath.exists(to_path):
@@ -503,18 +505,21 @@ def copy_file(from_path, to_path):
             # Copy the whole directory structure
             folder_name = os.path.basename(from_path)
             to_path = os.path.join(to_path, folder_name)
-            # Since we need to copy everything along with the directory structure,
-            # making sure the directory does not exist will not hurt.
             if os.path.exists(to_path):
-                shutil.rmtree(to_path)
-            # Copy the directory recursively along with its structure 
-            shutil.copytree(from_path, to_path)
+                msg = norm(to_path)+ ' is already existed and is replaced by ' + norm(from_path)
+                error.append(Error(WARNING, msg))
+            copy_tree(from_path, to_path)
         else:
+            file_name = os.path.basename(from_path)
+            to_file_path = os.path.join(to_path, file_name)
+            if os.path.exists(to_file_path):
+                msg = norm(to_file_path)+ ' is already existed and is replaced by ' + norm(from_path)
+                error.append(Error(WARNING, msg))
             shutil.copy2(from_path, to_path)
-        return []
+        return error
     except Exception as e:
         msg = 'Cannot copy file at %(from_path)r.' % locals()
-        error = [Error(CRITICAL, msg)]
+        error.append(Error(CRITICAL, msg))
         return error
 
 
@@ -628,6 +633,44 @@ def filter_errors(errors, minimum_severity=WARNING):
     return unique([e for e in errors if e.severity >= minimum_severity])
 
 
+def create_dir(location):
+    """
+    Create directory or directory tree at location, ensuring it is readable
+    and writeable.
+    """
+    import stat
+    if not os.path.exists(location):
+        os.makedirs(location)
+        os.chmod(location, stat.S_IRWXU | stat.S_IRWXG
+                 | stat.S_IROTH | stat.S_IXOTH)
+
+
+def get_temp_dir(sub_dir_path=None):
+    """
+    Create a unique new temporary directory location. Create directories
+    identified by sub_dir_path if provided in this temporary directory.
+    Return the location for this unique directory joined with the
+    sub_dir_path if any.
+    """
+    new_temp_dir = build_temp_dir()
+
+    if sub_dir_path:
+        # create a sub directory hierarchy if requested
+        new_temp_dir = os.path.join(new_temp_dir, sub_dir_path)
+        create_dir(new_temp_dir)
+    return new_temp_dir
+
+
+def build_temp_dir(prefix='attributecode-'):
+    """
+    Create and return a new unique empty directory created in base_dir.
+    """
+    import tempfile
+    location = tempfile.mkdtemp(prefix=prefix)
+    create_dir(location)
+    return location
+
+
 """
 Return True if a string s  name is safe to use as an attribute name.
 """

tests/test_model.py

@@ -35,7 +35,7 @@
 from attributecode import WARNING
 from attributecode import Error
 from attributecode import model
-from attributecode.util import add_unc
+from attributecode.util import add_unc, norm, on_windows
 from attributecode.util import load_csv
 from attributecode.util import to_posix
 from attributecode.util import replace_tab_with_spaces
@@ -1210,13 +1210,16 @@ def test_collect_inventory_does_not_convert_lf_to_crlf_from_directory(self):
         expected = get_test_loc('test_model/crlf/expected.csv')
         check_csv(expected, result, fix_cell_linesep=True, regen=False)
 
-    def test_copy_redist_src(self):
-        test_loc = get_test_loc('test_model/redistribution')
+    def test_copy_redist_src_no_structure(self):
+        test_loc = get_test_loc('test_model/redistribution/')
+        copy_list = [get_test_loc('test_model/redistribution/this.c'), get_test_loc('test_model/redistribution/test/subdir')]
         output = get_temp_dir()
-        errors, abouts = model.collect_inventory(test_loc)
-        expected_file = ['this.c']
 
-        err = model.copy_redist_src(abouts, test_loc, output)
+        expected_file = ['this.c', 'subdir']
+
+        with_structure = False
+        err = model.copy_redist_src(copy_list, test_loc, output, with_structure)
+        
         assert err == []
 
         from os import listdir
@@ -1226,6 +1229,37 @@ def test_copy_redist_src(self):
         for file in expected_file:
             assert file in copied_files
 
+    def test_copy_redist_src_with_structure(self):
+        test_loc = get_test_loc('test_model/redistribution/')
+        copy_list = [get_test_loc('test_model/redistribution/this.c'), get_test_loc('test_model/redistribution/test/subdir')]
+        output = get_temp_dir()
+
+        expected_file = ['this.c', 'test']
+
+        with_structure = True
+        err = model.copy_redist_src(copy_list, test_loc, output, with_structure)
+
+        assert err == []
+
+        from os import listdir
+        copied_files = listdir(output)
+        assert len(expected_file) == len(copied_files)
+        assert err == []
+        for file in expected_file:
+            assert file in copied_files
+
+    def test_get_copy_list(self):
+        location = get_test_loc('test_model/redistribution/')
+        result = get_temp_file()
+        errors, abouts = model.collect_inventory(location)
+        copy_list, err = model.get_copy_list(abouts, location)
+        assert err == []
+        norm_list = []
+        for c in copy_list:
+            norm_list.append(norm(c))
+        expected = [os.path.join(location, 'this.c'), os.path.join(location, 'test/subdir')]
+        assert norm_list == expected
+
 class FetchLicenseTest(unittest.TestCase):
 
     @mock.patch.object(model, 'urlopen')

tests/testdata/test_model/redistribution/test/subdir/test.ABOUT

@@ -0,0 +1,3 @@
+about_resource: .
+name: test
+redistribute: x