diff --git a/properdocs/commands/serve.py b/properdocs/commands/serve.py index afe3f847..f3f9cbce 100644 --- a/properdocs/commands/serve.py +++ b/properdocs/commands/serve.py @@ -58,6 +58,7 @@ def get_config(): site_dir=site_dir, **kwargs, ) + config.site_dir = site_dir # Overwrite it afterwards, so normal validation still kicks in. config.watch.extend(watch) return config diff --git a/properdocs/config/base.py b/properdocs/config/base.py index 5242c769..76947724 100644 --- a/properdocs/config/base.py +++ b/properdocs/config/base.py @@ -155,7 +155,10 @@ def __init__(self, config_file_path: str | bytes | None = None): config_file_path = config_file_path.decode(encoding=sys.getfilesystemencoding()) except UnicodeDecodeError: raise ValidationError("config_file_path is not a Unicode string.") - self.config_file_path = config_file_path or '' + if config_file_path: + self.config_file_path = os.path.abspath(config_file_path) + else: + self.config_file_path = '' def set_defaults(self) -> None: """ diff --git a/properdocs/config/config_options.py b/properdocs/config/config_options.py index 7a9a82a6..979ca7ba 100644 --- a/properdocs/config/config_options.py +++ b/properdocs/config/config_options.py @@ -4,6 +4,7 @@ import ipaddress import logging import os +import pathlib import string import sys import traceback @@ -708,13 +709,18 @@ def post_validation(self, config: Config, key_name: str): if not config.config_file_path: return - # Validate that the dir is not the parent dir of the config file. - if os.path.dirname(config.config_file_path) == config[key_name]: - raise ValidationError( - f"The '{key_name}' should not be the parent directory of the" - f" config file. Use a child directory instead so that the" - f" '{key_name}' is a sibling of the config file." - ) + # Validate that the docs dir does not contain the config file. + config_rel_path = _relative_path(config.config_file_path, to=config[key_name]) + if config_rel_path is None: + return # Good, it's not inside the docs_dir. + exclude_docs: pathspec.gitignore.GitIgnoreSpec | None = config.get('exclude_docs') + if exclude_docs and exclude_docs.match_file(config_rel_path): + return # Good, the appropriate config is present. + raise ValidationError( + f"The '{config_rel_path.name}' config file should not be inside the {key_name}.\n" + f"To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration:\n\n" + f"exclude_docs: |\n /{config_rel_path.as_posix()}" + ) class File(FilesystemObject): @@ -765,19 +771,21 @@ def post_validation(self, config: Config, key_name: str): # Validate that the docs_dir and site_dir don't contain the # other as this will lead to copying back and forth on each # and eventually make a deep nested mess. - if (docs_dir + os.sep).startswith(site_dir.rstrip(os.sep) + os.sep): + if _relative_path(docs_dir, to=site_dir): raise ValidationError( - f"The 'docs_dir' should not be within the 'site_dir' as this " - f"can mean the source files are overwritten by the output or " - f"it will be deleted if --clean is passed to properdocs build. " - f"(site_dir: '{site_dir}', docs_dir: '{docs_dir}')" + "The docs_dir should not be inside the site_dir as this " + "can mean the source files are overwritten by the output or " + "deleted entirely." ) - elif (site_dir + os.sep).startswith(docs_dir.rstrip(os.sep) + os.sep): + if site_rel_path := _relative_path(site_dir, to=docs_dir): + exclude_docs: pathspec.gitignore.GitIgnoreSpec | None = config.get('exclude_docs') + if exclude_docs and exclude_docs.match_file(site_rel_path): + return # Good, the appropriate config is present. raise ValidationError( - f"The 'site_dir' should not be within the 'docs_dir' as this " - f"leads to the build directory being copied into itself and " - f"duplicate nested files in the 'site_dir'. " - f"(site_dir: '{site_dir}', docs_dir: '{docs_dir}')" + f"The site_dir should not be inside the docs_dir as this " + f"leads to the build directory being copied into itself.\n" + f"To allow this arrangement, please exclude the directory (and other files as applicable) by adding the following configuration:\n\n" + f"exclude_docs: |\n /{site_rel_path.as_posix()}" ) @@ -1224,3 +1232,10 @@ def run_validation(self, value: object) -> pathspec.gitignore.GitIgnoreSpec: return pathspec.gitignore.GitIgnoreSpec.from_lines(lines=value.splitlines()) except ValueError as e: raise ValidationError(str(e)) + + +def _relative_path(main: str | os.PathLike, to: str | os.PathLike) -> pathlib.Path | None: + try: + return pathlib.Path(main).relative_to(to) + except ValueError: + return None diff --git a/properdocs/tests/config/config_options_legacy_tests.py b/properdocs/tests/config/config_options_legacy_tests.py index afe49e16..18e4f319 100644 --- a/properdocs/tests/config/config_options_legacy_tests.py +++ b/properdocs/tests/config/config_options_legacy_tests.py @@ -759,17 +759,20 @@ class Schema: ) self.assertEqual(conf['dir'], os.path.join(base_path, 'foo')) - def test_site_dir_is_config_dir_fails(self): + def test_docs_dir_is_config_dir_fails(self): class Schema: - dir = c.DocsDir() + docs_dir = c.DocsDir() with self.expect_error( - dir="The 'dir' should not be the parent directory of the config file. " - "Use a child directory instead so that the 'dir' is a sibling of the config file." + docs_dir="""The 'properdocs.yml' config file should not be inside the docs_dir. +To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /properdocs.yml""" ): self.get_config( Schema, - {'dir': '.'}, + {'docs_dir': '.'}, config_file_path=os.path.join(os.path.abspath('.'), 'properdocs.yml'), ) @@ -851,7 +854,7 @@ class Schema: site_dir = c.SiteDir() docs_dir = c.Dir() - def test_doc_dir_in_site_dir(self): + def test_doc_dir_in_site_dir_fails(self): j = os.path.join # The parent dir is not the same on every system, so use the actual dir name parent_dir = properdocs.__file__.split(os.sep)[-3] @@ -869,24 +872,27 @@ def test_doc_dir_in_site_dir(self): for test_config in test_configs: with self.subTest(test_config): with self.expect_error( - site_dir=re.compile(r"The 'docs_dir' should not be within the 'site_dir'.*") + site_dir="The docs_dir should not be inside the site_dir as this can mean the source files are overwritten by the output or deleted entirely." ): self.get_config(self.Schema, test_config) - def test_site_dir_in_docs_dir(self): + def test_site_dir_in_docs_dir_fails(self): j = os.path.join test_configs = ( {'docs_dir': 'docs', 'site_dir': j('docs', 'site')}, {'docs_dir': '.', 'site_dir': 'site'}, {'docs_dir': '', 'site_dir': 'site'}, - {'docs_dir': '/', 'site_dir': 'site'}, ) for test_config in test_configs: with self.subTest(test_config): with self.expect_error( - site_dir=re.compile(r"The 'site_dir' should not be within the 'docs_dir'.*") + site_dir="""The site_dir should not be inside the docs_dir as this leads to the build directory being copied into itself. +To allow this arrangement, please exclude the directory (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /site""" ): self.get_config(self.Schema, test_config) @@ -1266,7 +1272,7 @@ class Schema: config_path = "foo/properdocs.yaml" self.get_config(Schema, {"sub": {"opt": "bar"}}, config_file_path=config_path) - self.assertEqual(passed_config_path, config_path) + self.assertEqual(passed_config_path, os.path.abspath(config_path)) class ConfigItemsTest(TestCase): diff --git a/properdocs/tests/config/config_options_tests.py b/properdocs/tests/config/config_options_tests.py index 258d3c0d..94c1fd15 100644 --- a/properdocs/tests/config/config_options_tests.py +++ b/properdocs/tests/config/config_options_tests.py @@ -973,20 +973,54 @@ class Schema(Config): ) self.assertEqual(conf.dir, os.path.join(base_path, 'foo')) - def test_site_dir_is_config_dir_fails(self) -> None: + def test_docs_dir_is_config_dir_fails(self) -> None: class Schema(Config): - dir = c.DocsDir() + docs_dir = c.DocsDir() with self.expect_error( - dir="The 'dir' should not be the parent directory of the config file. " - "Use a child directory instead so that the 'dir' is a sibling of the config file." + docs_dir="""The 'properdocs.yml' config file should not be inside the docs_dir. +To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /properdocs.yml""" ): self.get_config( Schema, - {'dir': '.'}, + {'docs_dir': '.'}, config_file_path=os.path.join(os.path.abspath('.'), 'properdocs.yml'), ) + def test_docs_dir_is_config_dir_nested_fails(self) -> None: + class Schema(Config): + docs_dir = c.DocsDir() + + with self.expect_error( + docs_dir="""The 'properdocos.yaml' config file should not be inside the docs_dir. +To allow this arrangement, please exclude the file (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /config/properdocos.yaml""" + ): + self.get_config( + Schema, + {'docs_dir': '..'}, + config_file_path=os.path.join( + os.path.abspath('.'), 'docs', 'config', 'properdocos.yaml' + ), + ) + + def test_docs_dir_is_config_dir_succeeds(self) -> None: + class Schema(Config): + docs_dir = c.DocsDir() + exclude_docs = c.Optional(c.PathSpec()) + + conf = self.get_config( + Schema, + {'docs_dir': '.', 'exclude_docs': '/properdocs.y*l'}, + config_file_path=os.path.join(os.path.abspath('.'), 'properdocs.yml'), + ) + self.assertEqual(conf.docs_dir, os.path.abspath('.')) + class ListOfPathsTest(TestCase): def test_valid_path(self) -> None: @@ -1073,8 +1107,9 @@ class SiteDirTest(TestCase): class Schema(Config): site_dir = c.SiteDir() docs_dir = c.Dir() + exclude_docs = c.Optional(c.PathSpec()) - def test_doc_dir_in_site_dir(self) -> None: + def test_doc_dir_in_site_dir_fails(self) -> None: j = os.path.join # The parent dir is not the same on every system, so use the actual dir name parent_dir = properdocs.__file__.split(os.sep)[-3] @@ -1092,27 +1127,43 @@ def test_doc_dir_in_site_dir(self) -> None: for test_config in test_configs: with self.subTest(test_config): with self.expect_error( - site_dir=re.compile(r"The 'docs_dir' should not be within the 'site_dir'.*") + site_dir="The docs_dir should not be inside the site_dir as this can mean the source files are overwritten by the output or deleted entirely." ): self.get_config(self.Schema, test_config) - def test_site_dir_in_docs_dir(self) -> None: + def test_site_dir_in_docs_dir_fails(self) -> None: j = os.path.join test_configs = ( {'docs_dir': 'docs', 'site_dir': j('docs', 'site')}, {'docs_dir': '.', 'site_dir': 'site'}, {'docs_dir': '', 'site_dir': 'site'}, - {'docs_dir': '/', 'site_dir': 'site'}, ) for test_config in test_configs: with self.subTest(test_config): with self.expect_error( - site_dir=re.compile(r"The 'site_dir' should not be within the 'docs_dir'.*") + site_dir="""The site_dir should not be inside the docs_dir as this leads to the build directory being copied into itself. +To allow this arrangement, please exclude the directory (and other files as applicable) by adding the following configuration: + +exclude_docs: | + /site""" ): self.get_config(self.Schema, test_config) + def test_site_dir_in_docs_dir_succeeds(self) -> None: + j = os.path.join + + test_configs = ( + {'docs_dir': 'docs', 'site_dir': j('docs', 'site'), 'exclude_docs': 'site'}, + {'docs_dir': '.', 'site_dir': 'site', 'exclude_docs': '/site'}, + {'docs_dir': '', 'site_dir': 'site', 'exclude_docs': '/si*e'}, + ) + + for test_config in test_configs: + with self.subTest(test_config): + self.get_config(self.Schema, test_config) + def test_common_prefix(self) -> None: """Legitimate settings with common prefixes should not fail validation.""" test_configs = ( @@ -1572,7 +1623,7 @@ class Schema(Config): config_path = "foo/properdocs.yaml" self.get_config(Schema, {"sub": [{"opt": "bar"}]}, config_file_path=config_path) - self.assertEqual(passed_config_path, config_path) + self.assertEqual(passed_config_path, os.path.abspath(config_path)) class NestedSubConfigTest(TestCase):