[PATCH] Fix parsing submodule URL (passes tests) (#7)

* Fix parsing submodule URL.

* Fallback to dummy values when Subproject is not available.

* update tests - wip

* fixes

* lint test files too

* add string utils

* subproject objects can have a None project attribute

* submodule_to_project can return None

* more robust url parsing

* fix lint

* fix tests

* fix tests (2)

* add `self_managed_gitlab_host` optional arg, clean code a bit

* run tests for python 3.7 too

* update README.md

* update user link

* add README.md in test

Co-authored-by: Darkdragon-001 <darkdragon-001@web.de>

Author: ValentinFrancois

Makefile

@@ -2,6 +2,7 @@ PROJECT = gitlab_submodule
 
 lint:
 	flake8 $(PROJECT) --count --show-source --statistics
+	flake8 tests --count --show-source --statistics
 
 test:
 	PYTHON_VERSION=$$(python3 --version) && \

README.md

@@ -5,22 +5,38 @@ List project submodules and get the commits they point to with python-gitlab.
 
 The [Gitlab REST API V4](https://docs.gitlab.com/ee/api/api_resources.html) 
 doesn't implement anything for submodule inspection yet. The only thing we can 
-currently do is [updating a submodule to a new commit id](https://docs.gitlab.com/ee/api/repository_submodules.html),
+currently do is [updating a submodule to a new commit id](
+https://docs.gitlab.com/ee/api/repository_submodules.html),
 but how can we trigger such an update if we don't know the current commit id 
 of our submodule?
 
 If you're using `python-gitlab` and you're distributing shared code among 
 your projects with submodules, you've probably run into this issue already.
 
 This package provides minimal utils to list the submodules present in a 
-Gitlab project, and more importantly to get the commits they're pointing to 
-(when the submodules are Gitlab projects themselves, otherwise we cannot 
-access the project via their URLs using `python-gitlab` only).
+Gitlab project, and more importantly to get the commits they're pointing to.
 
 Internally, it reads and parses the `.gitmodules` file at the root of the 
 Project. To get the commit id of a submodule, it finds the last commit that 
 updated the submodule and parses its diff.
 
+---
+**About the future of this package**
+
+I don't plan to make PRs to `python-gitlab` for now. 
+
+In my opinion this problem should ideally be fixed in the Gitlab REST API, 
+and then `python-gitlab` could wrap around the new endpoints.
+
+So I see this package as a temporary solution until the API gets extended 
+with more submodule functionalities.
+
+[@darkdragon-001](https://github.com/darkdragon-001) created an issue on
+GitLab about the lack of support for submodules, feel free to support it with
+a thumb up: https://gitlab.com/gitlab-org/gitlab/-/issues/352836
+
+---
+
 ## Requirements
 - Python >= __3.7__ (required by `python-gitlab` since version `3.0.0`)
 
@@ -100,14 +116,13 @@ Output:
 ### `iterate_subprojects(...)`
 What you'll probably use most of the time.<br/>
 - Yields [`Subproject`](#class-subproject) objects that describe the submodules.
-- Ignores submodules that are not hosted on Gitlab. If you want to list all 
-  modules present in the `.gitmodules` file but without mapping them to 
-  `gitlab.v4.objects.Project` objects, use [`list_submodules(...)`](#list_submodules) instead.
 ```python
 iterate_subprojects(
     project: Project,
     gl: Union[Gitlab, ProjectManager],
     ref: Optional[str] = None,
+    only_gitlab_subprojects: bool = False,
+    self_managed_gitlab_host: Optional[str] = None,
     get_latest_commit_possible_if_not_found: bool = False,
     get_latest_commit_possible_ref: Optional[str] = None
 ) -> Generator[Subproject, None, None]
@@ -118,13 +133,20 @@ Parameters:
   `projects: gitlab.v4.objects.ProjectManager` attribute
 - `ref`: (optional) a ref to a branch, commit, tag etc. Defaults to the 
   HEAD of the project default branch.
-- `get_latest_commit_possible_if_not_found`: in some rare cases, there 
-  won't be any `Subproject commit ...` info in the diff of the last commit 
-  that updated the submodules. Set this option to `True` if you want to get 
-  instead the most recent commit in the subproject that is anterior to the 
+- `only_gitlab_subprojects`: (optional) if set to `True`, will ignore the 
+  submodules not hosted on GitLab. If set to `False` (default), it will yield
+  [`Subproject`](#class-subproject) objects with `self.project = None`
+  for submodules not hosted on GitLab.
+- `self_managed_gitlab_host`: (optional) if some submodules are hosted on a 
+  self-managed GitLab instance, you should pass its url here otherwise it 
+  may be impossible to know from the URL that it's a GitLab project.
+- `get_latest_commit_possible_if_not_found`: (optional) in some rare cases, 
+  there won't be any `Subproject commit ...` info in the diff of the last 
+  commit that updated the submodules. Set this option to `True` if you want to 
+  get instead the most recent commit in the subproject that is anterior to the 
   commit that updated the submodules of the project. If your goal is to 
   check that your submodules are up-to-date, you might want to use this.
-- `get_latest_commit_possible_ref`: in case you set 
+- `get_latest_commit_possible_ref`: (optional) in case you set 
   `get_latest_commit_possible_if_not_found` to `True`, you can specify a ref for the 
   subproject (for instance your submodule could point to a different branch 
   than the main one). By default, the main branch of the subproject will be 
@@ -140,10 +162,13 @@ returns a `list` of [`Subproject`](#class-subproject) objects.
 Basic objects that contain the info about a Gitlab subproject.
 
 Attributes:
-- `project: gitlab.v4.objects.Project`: the Gitlab project that the submodule links to
+- `project: Optional[gitlab.v4.objects.Project]`: the Gitlab project that the 
+  submodule links to (can be `None` if the submodule is not hosted on GitLab)
 - `submodule: `[`Submodule`](#class-submodule): a basic object that contains 
   the info found in the `.gitmodules` file (name, path, url).
-- `commit: gitlab.v4.objects.ProjectCommit`: the commit that the submodule points to
+- `commit: Union[gitlab.v4.objects.ProjectCommit, Commit]`: the commit that 
+  the submodule points to (if the submodule is not hosted on GitLab, it will 
+  be a dummy `Commit` object with a single attribute `id`)
 - `commit_is_exact: bool`: `True` most of the time, `False` only if the commit 
   had to be guessed via the `get_latest_commit_possible_if_not_found` option
 
@@ -195,6 +220,7 @@ hosted on Gitlab.
 submodule_to_subproject(
     gitmodules_submodule: Submodule,
     gl: Union[Gitlab, ProjectManager],
+    self_managed_gitlab_host: Optional[str] = None,
     get_latest_commit_possible_if_not_found: bool = False,
     get_latest_commit_possible_ref: Optional[str] = None
 ) -> Subproject

gitlab_submodule/__init__.py

@@ -7,8 +7,7 @@
 
 __all__ = [
     'Submodule', 'Subproject',
-    'list_submodules',
-    'iterate_submodules',
+    'list_submodules', 'iterate_submodules',
     'submodule_to_subproject',
     'iterate_subprojects', 'list_subprojects'
 ]

gitlab_submodule/gitlab_submodule.py

@@ -23,16 +23,21 @@ def _get_project_manager(
 def submodule_to_subproject(
         gitmodules_submodule: Submodule,
         gl: Union[Gitlab, ProjectManager],
+        self_managed_gitlab_host: Optional[str] = None,
         get_latest_commit_possible_if_not_found: bool = False,
         get_latest_commit_possible_ref: Optional[str] = None
 ) -> Subproject:
-    submodule_project = submodule_to_project(gitmodules_submodule,
-                                             _get_project_manager(gl))
+    submodule_project = submodule_to_project(
+        gitmodules_submodule,
+        _get_project_manager(gl),
+        self_managed_gitlab_host
+    )
     submodule_commit, commit_is_exact = get_submodule_commit(
         gitmodules_submodule,
         submodule_project,
         get_latest_commit_possible_if_not_found,
-        get_latest_commit_possible_ref)
+        get_latest_commit_possible_ref
+    )
     return Subproject(
         gitmodules_submodule,
         submodule_project,
@@ -45,20 +50,21 @@ def iterate_subprojects(
         project: Project,
         gl: Union[Gitlab, ProjectManager],
         ref: Optional[str] = None,
+        only_gitlab_subprojects: bool = False,
+        self_managed_gitlab_host: Optional[str] = None,
         get_latest_commit_possible_if_not_found: bool = False,
         get_latest_commit_possible_ref: Optional[str] = None
 ) -> Generator[Subproject, None, None]:
     for gitmodules_submodule in iterate_submodules(project, ref):
-        try:
-            yield submodule_to_subproject(
-                gitmodules_submodule,
-                _get_project_manager(gl),
-                get_latest_commit_possible_if_not_found,
-                get_latest_commit_possible_ref)
-        except ValueError:
-            continue
-        except Exception:
-            raise
+        subproject: Subproject = submodule_to_subproject(
+            gitmodules_submodule,
+            _get_project_manager(gl),
+            self_managed_gitlab_host,
+            get_latest_commit_possible_if_not_found,
+            get_latest_commit_possible_ref
+        )
+        if not (only_gitlab_subprojects and not subproject.project):
+            yield subproject
 
 
 def list_subprojects(*args, **kwargs) -> List[Subproject]:

gitlab_submodule/objects.py

@@ -1,5 +1,9 @@
+from typing import Union, Optional
+
 from gitlab.v4.objects import Project, ProjectCommit
 
+from gitlab_submodule.string_utils import lstrip
+
 
 class Submodule:
 
@@ -48,18 +52,16 @@ def __repr__(self):
         )
 
 
-def lstrip(string: str, pattern: str) -> str:
-    if string[:len(pattern)] == pattern:
-        return string[len(pattern):]
-    else:
-        return string
+class Commit:
+    def __init__(self, _id) -> None:
+        self.id = id
 
 
 class Subproject:
     def __init__(self,
                  submodule: Submodule,
-                 project: Project,
-                 commit: ProjectCommit,
+                 project: Optional[Project],
+                 commit: Union[ProjectCommit, Commit],
                  commit_is_exact: bool):
         self.submodule = submodule
         self.project = project

gitlab_submodule/string_utils.py

@@ -0,0 +1,12 @@
+def lstrip(string: str, pattern: str) -> str:
+    if string[:len(pattern)] == pattern:
+        return string[len(pattern):]
+    else:
+        return string
+
+
+def rstrip(string: str, pattern: str) -> str:
+    if string[-len(pattern):] == pattern:
+        return string[:-len(pattern)]
+    else:
+        return string

gitlab_submodule/submodule_commit.py

@@ -1,18 +1,18 @@
-from typing import Optional, Tuple
+from typing import Optional, Tuple, Union
 
 import re
 
 from gitlab.v4.objects import Project, ProjectCommit
 
-from gitlab_submodule.objects import Submodule
+from gitlab_submodule.objects import Submodule, Commit
 
 
 def get_submodule_commit(
         submodule: Submodule,
-        submodule_project: Project,
+        submodule_project: Optional[Project] = None,
         *args,
         **kwargs
- ) -> Tuple[ProjectCommit, bool]:
+ ) -> Tuple[Union[ProjectCommit, Commit], bool]:
     commit_id, is_exact = _get_submodule_commit_id(
         submodule.parent_project,
         submodule.path,
@@ -21,7 +21,10 @@ def get_submodule_commit(
         *args,
         **kwargs
     )
-    commit = submodule_project.commits.get(commit_id)
+    if submodule_project is not None:
+        commit = submodule_project.commits.get(commit_id)
+    else:
+        commit = Commit(commit_id)
     return commit, is_exact
 
 
@@ -76,15 +79,15 @@ def _get_submodule_commit_id(
     # was created before this date.
     # This requires a Project object for the submodule so if it wasn't
     # passed we cannot guess anything.
-    if not get_latest_commit_possible_if_not_found:
+    if not (get_latest_commit_possible_if_not_found
+            and submodule_project is not None):
         raise ValueError(
             f'Could not find commit id for submodule {submodule_path} of '
             f'project {project.path_with_namespace}.')
-    else:
-        last_subproject_commits = submodule_project.commits.list(
-            ref_name=(get_latest_commit_possible_ref
-                      if get_latest_commit_possible_ref
-                      else submodule_project.default_branch),
-            until=update_submodule_commit.created_at
-        )
+
+    last_subproject_commits = submodule_project.commits.list(
+        ref_name=(get_latest_commit_possible_ref
+                  if get_latest_commit_possible_ref
+                  else submodule_project.default_branch),
+        until=update_submodule_commit.created_at)
     return last_subproject_commits[0].id, False

gitlab_submodule/submodule_to_project.py

@@ -1,49 +1,73 @@
 from typing import Optional
+import logging
 
 from posixpath import join, normpath
-from giturlparse import parse
+from giturlparse import parse, GitUrlParsed
 
 from gitlab.v4.objects import Project, ProjectManager
 
 from gitlab_submodule.objects import Submodule
+from gitlab_submodule.string_utils import lstrip, rstrip
 
+logger = logging.getLogger(__name__)
 
-def submodule_to_project(submodule: Submodule,
-                         project_manager: ProjectManager) -> Project:
+
+def submodule_to_project(
+        submodule: Submodule,
+        project_manager: ProjectManager,
+        self_managed_gitlab_host: Optional[str] = None) -> Optional[Project]:
     submodule_project_path_with_namespace = \
         _submodule_url_to_path_with_namespace(submodule.url,
-                                              submodule.parent_project)
+                                              submodule.parent_project,
+                                              self_managed_gitlab_host)
     if not submodule_project_path_with_namespace:
-        raise ValueError(
-            f'submodule at {submodule.url} is not hosted on Gitlab')
+        return None
     submodule_project = project_manager.get(
         submodule_project_path_with_namespace)
     return submodule_project
 
 
 def _submodule_url_to_path_with_namespace(
         url: str,
-        parent_project: Project
-) -> Optional[str]:
+        parent_project: Project,
+        self_managed_gitlab_host: Optional[str] = None) -> Optional[str]:
     """Returns a path pointing to a Gitlab project, or None if the submodule
     is hosted elsewhere
     """
-    try:
-        parsed = parse(url)
-        if parsed.platform != 'gitlab':
-            return None
-        if parsed.groups:
-            to_join = [parsed.owner, join(*parsed.groups), parsed.repo]
-        else:
-            to_join = [parsed.owner, parsed.repo]
-        path_with_namespace = join(*to_join)
+    # check if the submodule url is a relative path to the project path
+    if url.startswith('./') or url.startswith('../'):
+        # we build the path of the submodule project using the path of
+        # the current project
+        url = rstrip(url, '.git')
+        path_with_namespace = normpath(
+            join(parent_project.path_with_namespace, url))
         return path_with_namespace
-    except Exception:
-        # check if the submodule url is a relative path to the project path
-        if url.startswith('./') or url.startswith('../'):
-            # we build the path of the submodule project using the path of
-            # the current project
-            path_with_namespace = normpath(
-                join(parent_project.path_with_namespace, url))
-            return path_with_namespace
-    return None
+
+    parsed: GitUrlParsed = parse(url)
+    if not parsed.valid:
+        logger.warning(f'submodule git url does not seem to be valid: {url}')
+        return None
+
+    # even if the parent project is hosted on a self-managed gitlab host,
+    # it can still use submodules hosted on gitlab.com
+    gitlab_hosts = ['gitlab']
+    if self_managed_gitlab_host:
+        gitlab_hosts.append(self_managed_gitlab_host)
+
+    # giturlparse.GitUrlParsed.platform is too permissive and will be set to
+    # 'gitlab' for some non-gitlab urls, for instance:
+    # https://opensource.ncsa.illinois.edu/bitbucket/scm/u3d/3dutilities.git
+    if (parsed.platform != 'gitlab'
+            or all([host not in parsed.host for host in gitlab_hosts])):
+        logger.warning(f'submodule git url is not hosted on gitlab: {url}')
+        return None
+
+    # Format to python-gitlab path_with_namespace:
+    # rewrite to https format then split by host and keep & cut the right part.
+    # I find it more robust than trying to rebuild the path from the different
+    # attributes of giturlparse.GitUrlParsed objects
+    https_url = parsed.url2https
+    path_with_namespace = https_url.split(parsed.host)[1]
+    path_with_namespace = lstrip(path_with_namespace, '/')
+    path_with_namespace = rstrip(path_with_namespace, '.git')
+    return path_with_namespace