Git: simplify submodule checks

readthedocs/projects/exceptions.py

@@ -39,10 +39,6 @@ class RepositoryError(BuildUserError):
         "ensure that your repository URL is correct and your repository is public. "
         "Private repositories are not supported.",
     )
-    INVALID_SUBMODULES = _(
-        "One or more submodule URLs are not valid: {}, "
-        "git/ssh URL schemas for submodules are not supported."
-    )
     DUPLICATED_RESERVED_VERSIONS = _(
         "You can not have two versions with the name latest or stable."
         " Ensure you don't have both a branch and a tag with this name."

readthedocs/projects/tests/mockers.py

@@ -149,18 +149,9 @@ def _mock_git_repository(self):
             'readthedocs.vcs_support.backends.git.Backend.submodules',
             new_callable=mock.PropertyMock,
             return_value=[
-                mock.Mock(
-                    path='one',
-                    url='https://github.com/submodule/one',
-                ),
-                mock.Mock(
-                    path='two',
-                    url='https://github.com/submodule/two',
-                ),
-                mock.Mock(
-                    path='three',
-                    url='https://github.com/submodule/three',
-                ),
+                "one",
+                "two",
+                "three",
             ],
         )
 

readthedocs/projects/tests/test_build_tasks.py

@@ -1611,7 +1611,7 @@ def test_python_install_pip_several_options(self, load_yaml_config):
     @pytest.mark.parametrize(
         "value,expected",
         [
-            (ALL, ["one", "two", "three"]),
+            (ALL, []),
             (["one", "two"], ["one", "two"]),
         ],
     )

readthedocs/projects/validators.py

@@ -79,27 +79,7 @@ def __call__(self, value):
         raise ValidationError(_("Invalid scheme for URL"))
 
 
-class SubmoduleURLValidator(RepositoryURLValidator):
-
-    """
-    A URL validator for repository submodules.
-
-    If a repository has a relative submodule, the URL path is effectively the
-    supermodule's remote ``origin`` URL with the relative path applied.
-
-    From the git docs::
-
-        ``<repository>`` is the URL of the new submodule's origin repository.
-        This may be either an absolute URL, or (if it begins with ``./`` or
-        ``../``), the location relative to the superproject's default remote
-        repository
-    """
-
-    disallow_relative_url = False
-
-
 validate_repository_url = RepositoryURLValidator()
-validate_submodule_url = SubmoduleURLValidator()
 
 
 def validate_build_config_file(path):

readthedocs/rtd_tests/tests/test_backend.py

@@ -181,24 +181,6 @@ def test_check_for_submodules(self):
         repo.checkout("submodule")
         self.assertTrue(repo.are_submodules_available(self.dummy_conf))
 
-    def test_check_invalid_submodule_urls(self):
-        """Test that a branch with an invalid submodule fails correctly."""
-        repo = self.project.vcs_repo(
-            environment=self.build_environment,
-            version_type=BRANCH,
-            version_identifier="invalidsubmodule",
-        )
-        repo.update()
-        repo.checkout("invalidsubmodule")
-        with self.assertRaises(RepositoryError) as e:
-            repo.update_submodules(self.dummy_conf)
-        # `invalid` is created in `make_test_git`
-        # it's a url in ssh form.
-        self.assertEqual(
-            str(e.exception),
-            RepositoryError.INVALID_SUBMODULES.format(["invalid"]),
-        )
-
     def test_check_submodule_urls(self):
         """Test that a valid submodule is found in the 'submodule' branch."""
         repo = self.project.vcs_repo(
@@ -208,7 +190,7 @@ def test_check_submodule_urls(self):
         )
         repo.update()
         repo.checkout("submodule")
-        valid, _ = repo.validate_submodules(self.dummy_conf)
+        valid, _ = repo.get_available_submodules(self.dummy_conf)
         self.assertTrue(valid)
 
     @patch("readthedocs.vcs_support.backends.git.Backend.fetch")
@@ -231,7 +213,7 @@ def test_git_update_with_external_version(self, fetch):
         repo.update()
         fetch.assert_called_once()
 
-    def test_invalid_submodule_is_ignored(self):
+    def test_submodule_without_url_is_included(self):
         """Test that an invalid submodule isn't listed."""
         repo = self.project.vcs_repo(
             environment=self.build_environment,
@@ -252,9 +234,8 @@ def test_invalid_submodule_is_ignored(self):
             )
             f.write(content)
 
-        valid, submodules = repo.validate_submodules(self.dummy_conf)
-        self.assertTrue(valid)
-        self.assertEqual(list(submodules), ["foobar"])
+        submodules = list(repo.submodules)
+        self.assertEqual(submodules, ["foobar", "not-valid-path"])
 
     def test_parse_submodules(self):
         repo = self.project.vcs_repo(
@@ -296,10 +277,16 @@ def test_parse_submodules(self):
             )
             f.write(content)
 
-        valid, submodules = repo.validate_submodules(self.dummy_conf)
-        self.assertTrue(valid)
+        submodules = list(repo.submodules)
         self.assertEqual(
-            list(submodules), ["foobar", "path with spaces", "another-submodule"]
+            submodules,
+            [
+                "foobar",
+                "not-valid-path",
+                "path with spaces",
+                "another-submodule",
+                "invalid-submodule-key",
+            ],
         )
 
     def test_skip_submodule_checkout(self):