From 63c62ed801380be88ecdf7d686ddcc7cc13cdb29 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 26 Feb 2024 19:43:59 -0500 Subject: [PATCH] Revise docstrings within git.objects.submodule --- git/objects/submodule/base.py | 82 ++++++++++++------- git/objects/submodule/root.py | 148 +++++++++++++++++++--------------- git/objects/submodule/util.py | 15 ++-- 3 files changed, 147 insertions(+), 98 deletions(-) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 497e5a06a..58b474fdd 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -60,7 +60,8 @@ class UpdateProgress(RemoteProgress): """Class providing detailed progress information to the caller who should - derive from it and implement the ``update(...)`` message.""" + derive from it and implement the + :meth:`update(...) ` message.""" CLONE, FETCH, UPDWKTREE = [1 << x for x in range(RemoteProgress._num_op_codes, RemoteProgress._num_op_codes + 3)] _num_op_codes: int = RemoteProgress._num_op_codes + 3 @@ -76,8 +77,8 @@ class UpdateProgress(RemoteProgress): # IndexObject comes via the util module. It's a 'hacky' fix thanks to Python's import -# mechanism, which causes plenty of trouble if the only reason for packages and -# modules is refactoring - subpackages shouldn't depend on parent packages. +# mechanism, which causes plenty of trouble if the only reason for packages and modules +# is refactoring - subpackages shouldn't depend on parent packages. class Submodule(IndexObject, TraversableIterableObj): """Implements access to a git submodule. They are special in that their sha represents a commit in the submodule's repository which is to be checked out @@ -119,11 +120,19 @@ def __init__( We only document the parameters that differ from :class:`~git.objects.base.IndexObject`. - :param repo: Our parent repository. - :param binsha: Binary sha referring to a commit in the remote repository. See - the ``url`` parameter. - :param parent_commit: See :meth:`set_parent_commit`. - :param url: The URL to the remote repository which is the submodule. + :param repo: + Our parent repository. + + :param binsha: + Binary sha referring to a commit in the remote repository. + See the `url` parameter. + + :param parent_commit: + See :meth:`set_parent_commit`. + + :param url: + The URL to the remote repository which is the submodule. + :param branch_path: Complete relative path to ref to checkout when cloning the remote repository. """ @@ -1313,9 +1322,11 @@ def exists(self) -> bool: @property def branch(self) -> "Head": """ - :return: The branch instance that we are to checkout + :return: + The branch instance that we are to checkout - :raise InvalidGitRepositoryError: If our module is not yet checked out + :raise InvalidGitRepositoryError: + If our module is not yet checked out. """ return mkhead(self.module(), self._branch_path) @@ -1329,7 +1340,10 @@ def branch_path(self) -> PathLike: @property def branch_name(self) -> str: - """:return: The name of the branch, which is the shortest possible branch name""" + """ + :return: + The name of the branch, which is the shortest possible branch name + """ # Use an instance method, for this we create a temporary Head instance # which uses a repository that is available at least (it makes no difference). return git.Head(self.repo, self._branch_path).name @@ -1342,9 +1356,11 @@ def url(self) -> str: @property def parent_commit(self) -> "Commit_ish": """ - :return: Commit instance with the tree containing the .gitmodules file + :return: + Commit instance with the tree containing the ``.gitmodules`` file - :note: Will always point to the current head's commit if it was not set explicitly. + :note: + Will always point to the current head's commit if it was not set explicitly. """ if self._parent_commit is None: return self.repo.commit() @@ -1353,33 +1369,40 @@ def parent_commit(self) -> "Commit_ish": @property def name(self) -> str: """ - :return: The name of this submodule. It is used to identify it within the - .gitmodules file. + :return: + The name of this submodule. It is used to identify it within the + ``.gitmodules`` file. - :note: By default, this is the name is the path at which to find the submodule, - but in GitPython it should be a unique identifier similar to the identifiers + :note: + By default, this is the name is the path at which to find the submodule, but + in GitPython it should be a unique identifier similar to the identifiers used for remotes, which allows to change the path of the submodule easily. """ return self._name def config_reader(self) -> SectionConstraint[SubmoduleConfigParser]: """ - :return: ConfigReader instance which allows you to query the configuration - values of this submodule, as provided by the .gitmodules file. + :return: + ConfigReader instance which allows you to query the configuration values of + this submodule, as provided by the ``.gitmodules`` file. - :note: The config reader will actually read the data directly from the - repository and thus does not need nor care about your working tree. + :note: + The config reader will actually read the data directly from the repository + and thus does not need nor care about your working tree. - :note: Should be cached by the caller and only kept as long as needed. + :note: + Should be cached by the caller and only kept as long as needed. - :raise IOError: If the .gitmodules file/blob could not be read. + :raise IOError: + If the ``.gitmodules`` file/blob could not be read. """ return self._config_parser_constrained(read_only=True) def children(self) -> IterableList["Submodule"]: """ - :return: IterableList(Submodule, ...) an iterable list of submodules instances - which are children of this submodule or 0 if the submodule is not checked out. + :return: + IterableList(Submodule, ...) An iterable list of submodules instances which + are children of this submodule or 0 if the submodule is not checked out. """ return self._get_intermediate_items(self) @@ -1395,7 +1418,10 @@ def iter_items( *Args: Any, **kwargs: Any, ) -> Iterator["Submodule"]: - """:return: Iterator yielding Submodule instances available in the given repository""" + """ + :return: + Iterator yielding Submodule instances available in the given repository + """ try: pc = repo.commit(parent_commit) # Parent commit instance parser = cls._config_parser(repo, pc, read_only=True) @@ -1423,8 +1449,8 @@ def iter_items( entry = index.entries[index.entry_key(p, 0)] sm = Submodule(repo, entry.binsha, entry.mode, entry.path) except KeyError: - # The submodule doesn't exist, probably it wasn't - # removed from the .gitmodules file. + # The submodule doesn't exist, probably it wasn't removed from the + # .gitmodules file. continue # END handle keyerror # END handle critical error diff --git a/git/objects/submodule/root.py b/git/objects/submodule/root.py index dde384bbe..8ef874f90 100644 --- a/git/objects/submodule/root.py +++ b/git/objects/submodule/root.py @@ -26,7 +26,8 @@ class RootUpdateProgress(UpdateProgress): - """Utility class which adds more opcodes to the UpdateProgress.""" + """Utility class which adds more opcodes to + :class:`~git.objects.submodule.base.UpdateProgress`.""" REMOVE, PATHCHANGE, BRANCHCHANGE, URLCHANGE = [ 1 << x for x in range(UpdateProgress._num_op_codes, UpdateProgress._num_op_codes + 4) @@ -91,43 +92,59 @@ def update( This method behaves smartly by determining changes of the path of a submodules repository, next to changes to the to-be-checked-out commit or the branch to be checked out. This works if the submodules ID does not change. + Additionally it will detect addition and removal of submodules, which will be handled gracefully. - :param previous_commit: If set to a commit-ish, the commit we should use as the - previous commit the HEAD pointed to before it was set to the commit it - points to now. - If None, it defaults to ``HEAD@{1}`` otherwise - :param recursive: if True, the children of submodules will be updated as well - using the same technique. - :param force_remove: If submodules have been deleted, they will be forcibly - removed. Otherwise the update may fail if a submodule's repository cannot be - deleted as changes have been made to it. + :param previous_commit: + If set to a commit-ish, the commit we should use as the previous commit the + HEAD pointed to before it was set to the commit it points to now. + If None, it defaults to ``HEAD@{1}`` otherwise. + + :param recursive: + If True, the children of submodules will be updated as well using the same + technique. + + :param force_remove: + If submodules have been deleted, they will be forcibly removed. Otherwise + the update may fail if a submodule's repository cannot be deleted as changes + have been made to it. (See :meth:`Submodule.update ` for more information.) - :param init: If we encounter a new module which would need to be initialized, - then do it. - :param to_latest_revision: If True, instead of checking out the revision pointed - to by this submodule's sha, the checked out tracking branch will be merged - with the latest remote branch fetched from the repository's origin. + + :param init: + If we encounter a new module which would need to be initialized, then do it. + + :param to_latest_revision: + If True, instead of checking out the revision pointed to by this submodule's + sha, the checked out tracking branch will be merged with the latest remote + branch fetched from the repository's origin. Unless `force_reset` is specified, a local tracking branch will never be reset into its past, therefore the remote branch must be in the future for this to have an effect. - :param force_reset: If True, submodules may checkout or reset their branch even - if the repository has pending changes that would be overwritten, or if the - local tracking branch is in the future of the remote tracking branch and - would be reset into its past. - :param progress: :class:`RootUpdateProgress` instance or None if no progress - should be sent. - :param dry_run: If True, operations will not actually be performed. Progress - messages will change accordingly to indicate the WOULD DO state of the - operation. - :param keep_going: If True, we will ignore but log all errors, and keep going - recursively. Unless `dry_run` is set as well, `keep_going` could cause + + :param force_reset: + If True, submodules may checkout or reset their branch even if the + repository has pending changes that would be overwritten, or if the local + tracking branch is in the future of the remote tracking branch and would be + reset into its past. + + :param progress: + :class:`RootUpdateProgress` instance or None if no progress should be sent. + + :param dry_run: + If True, operations will not actually be performed. Progress messages will + change accordingly to indicate the WOULD DO state of the operation. + + :param keep_going: + If True, we will ignore but log all errors, and keep going recursively. + Unless `dry_run` is set as well, `keep_going` could cause subsequent/inherited errors you wouldn't see otherwise. In conjunction with `dry_run`, this can be useful to anticipate all errors when updating submodules. - :return: self + + :return: + self """ if self.repo.bare: raise InvalidGitRepositoryError("Cannot update submodules in bare repositories") @@ -234,13 +251,14 @@ def update( ################### if sm.url != psm.url: # Add the new remote, remove the old one. - # This way, if the url just changes, the commits will not - # have to be re-retrieved. + # This way, if the url just changes, the commits will not have + # to be re-retrieved. nn = "__new_origin__" smm = sm.module() rmts = smm.remotes - # Don't do anything if we already have the url we search in place. + # Don't do anything if we already have the url we search in + # place. if len([r for r in rmts if r.url == sm.url]) == 0: progress.update( BEGIN | URLCHANGE, @@ -272,17 +290,17 @@ def update( # END if urls match # END for each remote - # If we didn't find a matching remote, but have exactly one, - # we can safely use this one. + # If we didn't find a matching remote, but have exactly + # one, we can safely use this one. if rmt_for_deletion is None: if len(rmts) == 1: rmt_for_deletion = rmts[0] else: - # If we have not found any remote with the original URL - # we may not have a name. This is a special case, - # and its okay to fail here. - # Alternatively we could just generate a unique name and - # leave all existing ones in place. + # If we have not found any remote with the + # original URL we may not have a name. This is a + # special case, and its okay to fail here. + # Alternatively we could just generate a unique + # name and leave all existing ones in place. raise InvalidGitRepositoryError( "Couldn't find original remote-repo at url %r" % psm.url ) @@ -292,19 +310,19 @@ def update( orig_name = rmt_for_deletion.name smm.delete_remote(rmt_for_deletion) # NOTE: Currently we leave tags from the deleted remotes - # as well as separate tracking branches in the possibly totally - # changed repository (someone could have changed the url to - # another project). At some point, one might want to clean - # it up, but the danger is high to remove stuff the user - # has added explicitly. + # as well as separate tracking branches in the possibly + # totally changed repository (someone could have changed + # the url to another project). At some point, one might + # want to clean it up, but the danger is high to remove + # stuff the user has added explicitly. # Rename the new remote back to what it was. smr.rename(orig_name) - # Early on, we verified that the our current tracking branch - # exists in the remote. Now we have to ensure that the - # sha we point to is still contained in the new remote - # tracking branch. + # Early on, we verified that the our current tracking + # branch exists in the remote. Now we have to ensure + # that the sha we point to is still contained in the new + # remote tracking branch. smsha = sm.binsha found = False rref = smr.refs[self.branch_name] @@ -316,10 +334,11 @@ def update( # END for each commit if not found: - # Adjust our internal binsha to use the one of the remote - # this way, it will be checked out in the next step. - # This will change the submodule relative to us, so - # the user will be able to commit the change easily. + # Adjust our internal binsha to use the one of the + # remote this way, it will be checked out in the + # next step. This will change the submodule relative + # to us, so the user will be able to commit the + # change easily. _logger.warning( "Current sha %s was not contained in the tracking\ branch at the new remote, setting it the the remote's tracking branch", @@ -328,7 +347,8 @@ def update( sm.binsha = rref.commit.binsha # END reset binsha - # NOTE: All checkout is performed by the base implementation of update. + # NOTE: All checkout is performed by the base + # implementation of update. # END handle dry_run progress.update( END | URLCHANGE, @@ -342,7 +362,8 @@ def update( # HANDLE PATH CHANGES ##################### if sm.branch_path != psm.branch_path: - # Finally, create a new tracking branch which tracks the new remote branch. + # Finally, create a new tracking branch which tracks the new + # remote branch. progress.update( BEGIN | BRANCHCHANGE, i, @@ -354,8 +375,8 @@ def update( if not dry_run: smm = sm.module() smmr = smm.remotes - # As the branch might not exist yet, we will have to fetch all remotes - # to be sure... + # As the branch might not exist yet, we will have to fetch + # all remotes to be sure... for remote in smmr: remote.fetch(progress=progress) # END for each remote @@ -372,11 +393,12 @@ def update( # END ensure tracking branch exists tbr.set_tracking_branch(find_first_remote_branch(smmr, sm.branch_name)) - # NOTE: All head-resetting is done in the base implementation of update - # but we will have to checkout the new branch here. As it still points - # to the currently checked out commit, we don't do any harm. - # As we don't want to update working-tree or index, changing the ref is - # all there is to do. + # NOTE: All head-resetting is done in the base + # implementation of update but we will have to checkout the + # new branch here. As it still points to the currently + # checked out commit, we don't do any harm. + # As we don't want to update working-tree or index, changing + # the ref is all there is to do. smm.head.reference = tbr # END handle dry_run @@ -409,10 +431,10 @@ def update( keep_going=keep_going, ) - # Update recursively depth first - question is which inconsistent - # state will be better in case it fails somewhere. Defective branch - # or defective depth. The RootSubmodule type will never process itself, - # which was done in the previous expression. + # Update recursively depth first - question is which inconsistent state will + # be better in case it fails somewhere. Defective branch or defective depth. + # The RootSubmodule type will never process itself, which was done in the + # previous expression. if recursive: # The module would exist by now if we are not in dry_run mode. if sm.module_exists(): diff --git a/git/objects/submodule/util.py b/git/objects/submodule/util.py index f8265798d..91e18a26b 100644 --- a/git/objects/submodule/util.py +++ b/git/objects/submodule/util.py @@ -35,7 +35,7 @@ def sm_section(name: str) -> str: - """:return: Section title used in .gitmodules configuration file""" + """:return: Section title used in ``.gitmodules`` configuration file""" return f'submodule "{name}"' @@ -51,7 +51,8 @@ def mkhead(repo: "Repo", path: PathLike) -> "Head": def find_first_remote_branch(remotes: Sequence["Remote"], branch_name: str) -> "RemoteReference": - """Find the remote branch matching the name of the given branch or raise InvalidGitRepositoryError.""" + """Find the remote branch matching the name of the given branch or raise + :class:`git.exc.InvalidGitRepositoryError`.""" for remote in remotes: try: return remote.refs[branch_name] @@ -69,11 +70,11 @@ def find_first_remote_branch(remotes: Sequence["Remote"], branch_name: str) -> " class SubmoduleConfigParser(GitConfigParser): - """Catches calls to _write, and updates the .gitmodules blob in the index + """Catches calls to _write, and updates the ``.gitmodules`` blob in the index with the new data, if we have written into a stream. - Otherwise it would add the local file to the index to make it correspond - with the working tree. Additionally, the cache must be cleared. + Otherwise it would add the local file to the index to make it correspond with the + working tree. Additionally, the cache must be cleared. Please note that no mutating method will work in bare mode. """ @@ -86,8 +87,8 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: # { Interface def set_submodule(self, submodule: "Submodule") -> None: - """Set this instance's submodule. It must be called before - the first write operation begins.""" + """Set this instance's submodule. It must be called before the first write + operation begins.""" self._smref = weakref.ref(submodule) def flush_to_index(self) -> None: