Merge pull request #294 from rstudio/bcwu-excludefiles

make exclude environment files work on multiple OS

Author: bcwu

rsconnect/bundle.py

@@ -14,6 +14,7 @@
 from pprint import pformat
 from collections import defaultdict
 from mimetypes import guess_type
+from pathlib import Path
 import click
 
 
@@ -33,7 +34,7 @@
 
 # From https://github.com/rstudio/rsconnect/blob/485e05a26041ab8183a220da7a506c9d3a41f1ff/R/bundle.R#L85-L88
 # noinspection SpellCheckingInspection
-directories_to_ignore = [
+directories_ignore_list = [
     ".Rproj.user/",
     ".env/",
     ".git/",
@@ -47,6 +48,7 @@
     "rsconnect/",
     "venv/",
 ]
+directories_to_ignore = {Path(d) for d in directories_ignore_list}
 
 
 # noinspection SpellCheckingInspection
@@ -423,17 +425,20 @@ def make_notebook_html_bundle(
     return bundle_file
 
 
-def keep_manifest_specified_file(relative_path):
+def keep_manifest_specified_file(relative_path, ignore_path_set=directories_to_ignore):
     """
     A helper to see if the relative path given, which is assumed to have come
     from a manifest.json file, should be kept or ignored.
 
     :param relative_path: the relative path name to check.
     :return: True, if the path should kept or False, if it should be ignored.
     """
-    for ignore_me in directories_to_ignore:
-        if relative_path.startswith(ignore_me):
+    p = Path(relative_path)
+    for parent in p.parents:
+        if parent in ignore_path_set:
             return False
+    if p in ignore_path_set:
+        return False
     return True
 
 
@@ -550,56 +555,6 @@ def list_environment_dirs(directory):
     return envs
 
 
-def _create_api_file_list(
-    directory,  # type: str
-    requirements_file_name,  # type: str
-    extra_files=None,  # type: typing.Optional[typing.List[str]]
-    excludes=None,  # type: typing.Optional[typing.List[str]]
-):
-    # type: (...) -> typing.List[str]
-    """
-    Builds a full list of files under the given directory that should be included
-    in a manifest or bundle.  Extra files and excludes are relative to the given
-    directory and work as you'd expect.
-
-    :param directory: the directory to walk for files.
-    :param requirements_file_name: the name of the requirements file for the current
-    Python environment.
-    :param extra_files: a sequence of any extra files to include in the bundle.
-    :param excludes: a sequence of glob patterns that will exclude matched files.
-    :return: the list of relevant files, relative to the given directory.
-    """
-    # Don't let these top-level files be added via the extra files list.
-    extra_files = extra_files or []
-    skip = [requirements_file_name, "manifest.json"]
-    extra_files = sorted(list(set(extra_files) - set(skip)))
-
-    # Don't include these top-level files.
-    excludes = list(excludes) if excludes else []
-    excludes.append("manifest.json")
-    excludes.append(requirements_file_name)
-    excludes.extend(list_environment_dirs(directory))
-    glob_set = create_glob_set(directory, excludes)
-
-    file_list = []
-
-    for subdir, dirs, files in os.walk(directory):
-        for file in files:
-            abs_path = os.path.join(subdir, file)
-            rel_path = os.path.relpath(abs_path, directory)
-
-            if keep_manifest_specified_file(rel_path) and (rel_path in extra_files or not glob_set.matches(abs_path)):
-                file_list.append(rel_path)
-                # Don't add extra files more than once.
-                if rel_path in extra_files:
-                    extra_files.remove(rel_path)
-
-    for rel_path in extra_files:
-        file_list.append(rel_path)
-
-    return sorted(file_list)
-
-
 def make_api_manifest(
     directory: str,
     entry_point: str,
@@ -624,7 +579,17 @@ def make_api_manifest(
     if is_environment_dir(directory):
         excludes = list(excludes or []) + ["bin/", "lib/"]
 
-    relevant_files = _create_api_file_list(directory, environment.filename, extra_files, excludes)
+    extra_files = extra_files or []
+    skip = [environment.filename, "manifest.json"]
+    extra_files = sorted(list(set(extra_files) - set(skip)))
+
+    # Don't include these top-level files.
+    excludes = list(excludes) if excludes else []
+    excludes.append("manifest.json")
+    excludes.append(environment.filename)
+    excludes.extend(list_environment_dirs(directory))
+
+    relevant_files = create_file_list(directory, extra_files, excludes)
     manifest = make_source_manifest(app_mode, environment, entry_point, None, image)
 
     manifest_add_buffer(manifest, environment.filename, environment.contents)
@@ -667,37 +632,15 @@ def make_html_bundle_content(
 
     extra_files = extra_files or []
     skip = ["manifest.json"]
-    extra_files = sorted(list(set(extra_files) - set(skip)))
+    extra_files = sorted(set(extra_files) - set(skip))
 
     # Don't include these top-level files.
     excludes = list(excludes) if excludes else []
     excludes.append("manifest.json")
     if not isfile(path):
         excludes.extend(list_environment_dirs(path))
-    glob_set = create_glob_set(path, excludes)
-
-    file_list = []
 
-    for rel_path in extra_files:
-        file_list.append(rel_path)
-
-    if isfile(path):
-        file_list.append(path)
-    else:
-        for subdir, dirs, files in os.walk(path):
-            for file in files:
-                abs_path = os.path.join(subdir, file)
-                rel_path = os.path.relpath(abs_path, path)
-
-                if keep_manifest_specified_file(rel_path) and (
-                    rel_path in extra_files or not glob_set.matches(abs_path)
-                ):
-                    file_list.append(rel_path)
-                    # Don't add extra files more than once.
-                    if rel_path in extra_files:
-                        extra_files.remove(rel_path)
-
-    relevant_files = sorted(file_list)
+    relevant_files = create_file_list(path, extra_files, excludes)
     manifest = make_html_manifest(entrypoint, image)
 
     for rel_path in relevant_files:
@@ -706,6 +649,48 @@ def make_html_bundle_content(
     return manifest, relevant_files
 
 
+def create_file_list(
+    path: str,
+    extra_files: typing.List[str] = None,
+    excludes: typing.List[str] = None,
+) -> typing.List[str]:
+    """
+    Builds a full list of files under the given path that should be included
+    in a manifest or bundle.  Extra files and excludes are relative to the given
+    directory and work as you'd expect.
+
+    :param path: a file, or a directory to walk for files.
+    :param extra_files: a sequence of any extra files to include in the bundle.
+    :param excludes: a sequence of glob patterns that will exclude matched files.
+    :return: the list of relevant files, relative to the given directory.
+    """
+    extra_files = extra_files or []
+    excludes = excludes if excludes else []
+    glob_set = create_glob_set(path, excludes)
+    exclude_paths = {Path(p) for p in excludes}
+    file_set = set()  # type: typing.Set[str]
+    file_set.union(extra_files)
+
+    if isfile(path):
+        file_set.add(path)
+        return sorted(file_set)
+
+    for subdir, dirs, files in os.walk(path):
+        if Path(subdir) in exclude_paths:
+            continue
+        for file in files:
+            abs_path = os.path.join(subdir, file)
+            rel_path = os.path.relpath(abs_path, path)
+
+            if Path(abs_path) in exclude_paths:
+                continue
+            if keep_manifest_specified_file(rel_path, exclude_paths | directories_to_ignore) and (
+                rel_path in extra_files or not glob_set.matches(abs_path)
+            ):
+                file_set.add(rel_path)
+    return sorted(file_set)
+
+
 def infer_entrypoint(path, mimetype):
     if os.path.isfile(path):
         return path
@@ -823,25 +808,9 @@ def _create_quarto_file_list(
     excludes = list(excludes) if excludes else []
     excludes.append("manifest.json")
     excludes.extend(list_environment_dirs(directory))
-    glob_set = create_glob_set(directory, excludes)
-
-    file_list = []
-
-    for subdir, dirs, files in os.walk(directory):
-        for file in files:
-            abs_path = os.path.join(subdir, file)
-            rel_path = os.path.relpath(abs_path, directory)
-
-            if keep_manifest_specified_file(rel_path) and (rel_path in extra_files or not glob_set.matches(abs_path)):
-                file_list.append(rel_path)
-                # Don't add extra files more than once.
-                if rel_path in extra_files:
-                    extra_files.remove(rel_path)
-
-    for rel_path in extra_files:
-        file_list.append(rel_path)
 
-    return sorted(file_list)
+    file_list = create_file_list(directory, extra_files, excludes)
+    return file_list
 
 
 def make_quarto_manifest(

rsconnect/main.py

@@ -407,7 +407,7 @@ def bootstrap(
 def add(ctx, name, server, api_key, insecure, cacert, account, token, secret, verbose):
 
     set_verbosity(verbose)
-    if sys.version_info >= (3, 8):
+    if click.__version__ >= "8.0.0" and sys.version_info >= (3, 8):
         click.echo("Detected the following inputs:")
         for k, v in locals().items():
             if k in {"ctx", "verbose"}:

rsconnect/validation.py

@@ -25,9 +25,7 @@ def validate_connection_options(url, api_key, insecure, cacert, account_name, to
             )
         )
     if not name and not url and not shinyapps_options:
-        raise RSConnectException(
-            "You must specify one of -n/--name OR -s/--server OR  T/--token, -S/--secret."
-        )
+        raise RSConnectException("You must specify one of -n/--name OR -s/--server OR  T/--token, -S/--secret.")
 
     present_connect_options = _get_present_options(connect_options)
     present_shinyapps_options = _get_present_options(shinyapps_options)
@@ -40,13 +38,9 @@ def validate_connection_options(url, api_key, insecure, cacert, account_name, to
             )
         )
 
-    if url and ('posit.cloud' in url or 'rstudio.cloud' in url):
+    if url and ("posit.cloud" in url or "rstudio.cloud" in url):
         if len(present_cloud_options) != len(cloud_options):
-            raise RSConnectException(
-                "-T/--token and -S/--secret must be provided for Posit Cloud."
-            )
+            raise RSConnectException("-T/--token and -S/--secret must be provided for Posit Cloud.")
     elif present_shinyapps_options:
         if len(present_shinyapps_options) != len(shinyapps_options):
-            raise RSConnectException(
-                "-A/--account, -T/--token, and -S/--secret must all be provided for shinyapps.io."
-            )
+            raise RSConnectException("-A/--account, -T/--token, and -S/--secret must all be provided for shinyapps.io.")

tests/test_bundle.py

@@ -276,9 +276,9 @@ def do_test_html_bundle(self, directory):
     def test_keep_manifest_specified_file(self):
         self.assertTrue(keep_manifest_specified_file("app.R"))
         self.assertFalse(keep_manifest_specified_file("packrat/packrat.lock"))
-        self.assertTrue(keep_manifest_specified_file("rsconnect"))
+        self.assertFalse(keep_manifest_specified_file("rsconnect"))
         self.assertFalse(keep_manifest_specified_file("rsconnect/bogus.file"))
-        self.assertTrue(keep_manifest_specified_file("rsconnect-python"))
+        self.assertFalse(keep_manifest_specified_file("rsconnect-python"))
         self.assertFalse(keep_manifest_specified_file("rsconnect-python/bogus.file"))
         self.assertFalse(keep_manifest_specified_file(".svn/bogus.file"))
         self.assertFalse(keep_manifest_specified_file(".env/share/jupyter/kernels/python3/kernel.json"))