Do not codebase walk outside input paths

For multiple inputs, do not walk outside the input paths, from their
common prefix. Instead create only the directory relationships between
the common prefix and input paths and start the codebase walk from
every input paths.

Also deprecate --include options to only support ignoring paths
through path patterns.

Signed-off-by: Ayan Sinha Mahapatra <asmahapatra@aboutcode.org>

Author: AyanSinhaMahapatra

src/commoncode/resource.py

@@ -63,7 +63,7 @@
 
 # Tracing flags
 TRACE = False
-TRACE_DEEP = True
+TRACE_DEEP = False
 
 
 def logger_debug(*args):
@@ -110,7 +110,7 @@ def skip_ignored(location):
     return is_special(location) or ignored(location)
 
 
-def is_ignored(location,  includes=None, excludes=None):
+def is_ignored(location,  includes=tuple(), excludes=tuple()):
 
     excludes = {
         pattern: 'User ignore: Supplied by --ignore' for pattern in excludes
@@ -339,8 +339,8 @@ def __init__(
 
         # finally populate
         self.paths = self._prepare_clean_paths(paths)
+        self.includes = self._prepare_clean_paths(includes)
         self.ignores = ignores
-        self.includes = includes
         self._populate()
 
     def _prepare_clean_paths(self, paths=tuple()):
@@ -593,30 +593,48 @@ def err(_error):
                 f"ERROR: cannot populate codebase: {_error}\n{traceback.format_exc()}"
             )
 
-        skip_ignored = partial(is_ignored, includes=includes, excludes=ignores)
+        # ignore creating resources based on path patterns
+        skip_ignored = partial(is_ignored, excludes=ignores)
 
         if TRACE_DEEP:
             logger_debug(f"parents_by_loc: {parents_by_loc}, ignores: {ignores}, includes: {includes}")
 
-        # Walk over the directory and build the resource tree
-        for top, dirs, files in depth_walk(
-            root_location=root.location,
-            skip_ignored=skip_ignored,
-            max_depth=self.max_depth,
-            error_handler=err,
-        ):
-            parent = parents_by_loc.pop(top)
-            for created in self._create_resources(
-                parent=parent,
-                top=top,
-                dirs=dirs,
-                files=files,
-                skip_ignored=skip_ignored,
+        # in the case of a single input location, walking starts from
+        # the root and only the root location
+        if not includes:
+            includes = [root.location]
+        else:
+            # create the directory resources between the common
+            # prefix and the included locations so that they are
+            # connected to the root
+            for created in self._create_resources_common_prefix_to_inputs(
+                root=root,
+                includes=includes,
             ):
-                # on the plain, bare FS, files cannot be parents
                 if not created.is_file:
                     parents_by_loc[created.location] = created
 
+        # we start walking through all the input locations
+        for included_location in includes:
+            # Walk over the directory and build the resource tree
+            for top, dirs, files in depth_walk(
+                root_location=included_location,
+                skip_ignored=skip_ignored,
+                max_depth=self.max_depth,
+                error_handler=err,
+            ):
+                parent = parents_by_loc.pop(top)
+                for created in self._create_resources(
+                    parent=parent,
+                    top=top,
+                    dirs=dirs,
+                    files=files,
+                    skip_ignored=skip_ignored,
+                ):
+                    # on the plain, bare FS, files cannot be parents
+                    if not created.is_file:
+                        parents_by_loc[created.location] = created
+
     def _create_resources(self, parent, top, dirs, files, skip_ignored=skip_ignored):
         """
         Create and yield ``files`` and ``dirs`` children Resources of a
@@ -641,6 +659,28 @@ def _create_resources(self, parent, top, dirs, files, skip_ignored=skip_ignored)
                     logger_debug("Codebase.create_resources:", res)
                 yield res
 
+    def _create_resources_common_prefix_to_inputs(self, root, includes):
+
+        if TRACE_DEEP:
+            logger_debug(f"_create_resources_common_prefix_to_inputs: root:{root.location}, includes: {includes}")
+
+        for included_path in includes:
+            _, _, extra_dir_path = included_path.rpartition(root.location)
+            extra_dirs = extra_dir_path.strip("/").split("/")
+            if TRACE_DEEP:
+                    logger_debug(f"_create_resources_common_prefix_to_inputs: root:{root.location}, includes: {includes}")
+
+            dir_resource = root
+            for dir_segment in extra_dirs:
+                dir_resource = self._get_or_create_resource(
+                    name=dir_segment,
+                    parent=dir_resource,
+                    is_file=False,
+                )
+                if TRACE:
+                    logger_debug("Codebase.create_resources:", dir_resource)
+                yield dir_resource
+
     def _create_root_resource(self):
         """
         Create and return the root Resource of this codebase.
@@ -1606,8 +1646,8 @@ def clean_path(path):
     Return a cleaned and normalized POSIX ``path``.
     """
     path = path or ""
-    # convert to posix and ensure we have no slash at both ends
-    path = posixpath_normpath(path.replace("\\", "/").strip("/"))
+    # convert to posix and ensure we have no slash at the end
+    path = posixpath_normpath(path.replace("\\", "/").rstrip("/"))
     if path == ".":
         path = ""
     return path

src/commoncode/testcase.py

@@ -92,7 +92,7 @@ class FileDrivenTesting(object):
 
     test_data_dir = None
 
-    def get_test_loc(self, test_path, copy=False, debug=False, must_exist=True):
+    def get_test_loc(self, test_path, copy=False, debug=False, must_exist=True, relative=False):
         """
         Given a `test_path` relative to the self.test_data_dir directory, return the
         location to a test file or directory for this path. Copy to a temp
@@ -128,6 +128,11 @@ def get_test_loc(self, test_path, copy=False, debug=False, must_exist=True):
                 # cleanup of VCS that could be left over from checkouts
                 self.remove_vcs(target_dir)
                 test_loc = target_dir
+        
+        if relative:
+            _, _, rel_test_loc = test_loc.rpartition(os.getcwd())
+            return rel_test_loc.strip("/")
+
         return test_loc
 
     def get_temp_file(self, extension=None, dir_name="td", file_name="tf"):

src/scancode/cli.py

@@ -221,16 +221,6 @@ def default_processes():
     callback=validate_input_path,
     type=click.Path(exists=True, readable=True, path_type=str))
 
-@click.option('--include',
-    multiple=True,
-    default=None,
-    metavar='<pattern>',
-    help='Include files matching <pattern>.',
-    sort_order=11,
-    help_group=cliutils.CORE_GROUP,
-    cls=PluggableCommandLineOption,
-)
-
 @click.option('--ignore',
     multiple=True,
     default=None,
@@ -415,7 +405,6 @@ def default_processes():
 def scancode(
     ctx,
     input,  # NOQA
-    include,
     ignore,
     strip_root,
     full_root,
@@ -527,7 +516,6 @@ def scancode(
         # run proper
         success, _results = run_scan(
             input=input,
-            include=include,
             ignore=ignore,
             from_json=from_json,
             strip_root=strip_root,
@@ -570,7 +558,6 @@ def scancode(
 
 def run_scan(
     input,  # 
-    include=[],
     ignore=[],
     from_json=False,
     strip_root=False,
@@ -623,6 +610,9 @@ def echo_func(*_args, **_kwargs):
         msg = 'At least one input path is required.'
         raise ScancodeError(msg)
 
+    # To support multiple path inputs
+    include = []
+
     if not isinstance(input, (list, tuple)):
         if not isinstance(input, str):
             msg = 'Unknown <input> format: "{}".'.format(repr(input))
@@ -637,8 +627,6 @@ def echo_func(*_args, **_kwargs):
     # VirtualCodebase; otherwise we have to process `input` to make it a single
     # root with excludes.
     elif not from_json:
-        # FIXME: support the multiple root better. This is quirky at best
-
         # This is the case where we have a list of input path and the
         # `from_json` option is not selected: we can handle this IFF they share
         # a common root directory and none is an absolute path
@@ -650,30 +638,33 @@ def echo_func(*_args, **_kwargs):
             )
             raise ScancodeError(msg)
 
+        abs_input = [os.path.abspath(i) for i in input]
+
         # find the common prefix directory (note that this is a pre string
         # operation hence it may return non-existing paths
-        common_prefix = os.path.commonprefix(input)
+        common_prefix = os.path.commonprefix(abs_input)
 
         if not common_prefix:
             # we have no common prefix, but all relative. therefore the
-            # parent/root is the current ddirectory
+            # parent/root is the current directory
             common_prefix = str('.')
+        elif not common_prefix.endswith("/"):
+            # common prefix has trailing incomplete dirname
+            # for example the common prefix of "/temp/scancode"
+            # and "/temp/scans" is "/temp/scan"
+            common_prefix, _, _ = common_prefix.rpartition("/")
 
         elif not os.path.isdir(common_prefix):
             msg = (
                 'Invalid inputs: all input paths must share a '
-                'common single parent directory.'
+                f'common single parent directory. common part: {common_prefix}'
             )
 
             raise ScancodeError(msg)
 
-        # and we craft a list of synthetic --include path pattern options from
-        # the input list of paths
-        included_paths = [as_posixpath(path).rstrip('/') for path in input]
-        include.extend(included_paths)
-
-        # ... and use the common prefix as our new input
-        # FIXME: we should not walk outside inputs
+        # and we craft a list of include paths where the codebase walks
+        # will start from, even though the root is the common prefix
+        include = [as_posixpath(path).rstrip('/') for path in abs_input]
         input = common_prefix  # NOQA
 
     # build mappings of all options to pass down to plugins

tests/commoncode/test_resource.py

@@ -353,7 +353,7 @@ def test_get_resource_for_multiple_resource_codebase(self):
 
         codebase = Codebase(test_codebase)
         assert codebase.get_resource("resource/a").path == "resource/a"
-        assert codebase.get_resource("/resource/c").path == "resource/c"
+        assert codebase.get_resource("resource/c").path == "resource/c"
         assert codebase.get_resource("resource/dsasda/../b/").path == "resource/b"
 
     def test_Resource_build_path(self):

tests/scancode/data/help/help_linux.txt

@@ -144,7 +144,6 @@ Options:
     -n, --processes INT      Set the number of parallel processes to use. Disable
                              parallel processing if 0. Also disable threading if
                              -1. [default: (number of CPUs)-1]
-    --include <pattern>      Include files matching <pattern>.
     -q, --quiet              Do not print summary or progress.
     -v, --verbose            Print progress as file-by-file path instead of a
                              progress bar. Print verbose scan counters.

tests/scancode/test_cli.py

@@ -168,7 +168,7 @@ def test_scan_info_returns_full_root():
     file_paths = [f['path'] for f in result_data['files']]
     assert len(file_paths) == 12
     # note that we strip paths from leading and trailing slashes
-    root = fileutils.as_posixpath(test_dir).strip('/')
+    root = fileutils.as_posixpath(test_dir)
     assert all(p.startswith(root) for p in file_paths)
 
 
@@ -184,7 +184,7 @@ def test_scan_info_returns_correct_full_root_with_single_file():
     scanned_file = files[0]
     # and we check that the path is the full path without repeating the file name
     # note that the path never contain leading and trailing slashes
-    assert scanned_file['path'] == fileutils.as_posixpath(test_file).strip('/')
+    assert scanned_file['path'] == fileutils.as_posixpath(test_file)
 
 
 def test_scan_info_returns_does_not_strip_root_with_single_file():
@@ -837,6 +837,15 @@ def test_scan_should_not_fail_with_low_max_in_memory_setting_when_ignoring_files
     run_scan_click(args, expected_rc=0)
 
 
+def test_scan_supports_multiple_input_paths():
+    test_file_1 = test_env.get_test_loc('summaries/client', relative=True)
+    test_file_2 = test_env.get_test_loc('summaries/counts', relative=True)
+    result_file = test_env.get_temp_file('json')
+    args = ['--info', '-n', '1', test_file_1, test_file_2, '--json', result_file]
+    run_scan_click(args, expected_rc=0)
+
+
+
 def test_get_displayable_summary():
     from scancode.cli import get_displayable_summary
     from commoncode.resource import Codebase