FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

review · gitpython-developers/GitPython@a83262b · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

Commit a83262b

Browse files
Byron
and
codex
committed
review
- Add tests - [P2] Disable lazy fetching during no-fetch updates — git/objects/submodule/base.py:865-866 For a partial-cloned submodule (for example, using `--filter=blob:none`), the target commit can be cached while its blobs are missing. Skipping `Remote.fetch` does not prevent the later checkout/reset from implicitly fetching those blobs. I reproduced `update(no_fetch=True)` spawning `git fetch origin ... --stdin` despite this guard. This violates the documented local-only behavior. Disable Git's lazy fetching for commands executed in this mode, including checkout and restoration. - [P2] Propagate no_fetch to recursive root-module updates — git/objects/submodule/root.py:442-442 For repositories with nested submodules, `Repo.submodule_update(no_fetch=True)` still fetches nested remotes. The flag reaches this immediate `sm.update(recursive=False, ...)`, but the subsequent `type(self)(sm.module()).update(...)` call omits it and defaults to `False`. Since root updates recurse by default, offline updates fail even when all required objects are cached. Forward the flag to the recursive call too. - [P2] Skip the branch-change fetch loop when no_fetch is set — git/objects/submodule/root.py:90-90 When `.gitmodules` changes a submodule's configured branch, the branch-change handler still unconditionally calls `remote.fetch(...)` for every remote. Consequently, even `submodule_update(recursive=False, no_fetch=True)` accesses remotes and fails offline, including when the target remote-tracking branch already exists locally. Guard that fetch loop with this flag as well. - [P2] Handle URL changes without requiring freshly fetched refs — git/objects/submodule/root.py:282-283 When a submodule's URL changes with `no_fetch=True`, the newly created `__new_origin__` remote has no refs, so the following `smr.refs` branch check raises `ValueError`. This occurs even when switching to an identical mirror with all required history cached locally, and leaves the temporary remote behind. The no-fetch path must handle the fetch-dependent validation and remote replacement, not merely skip this fetch. - [P2] Allow restoring retained repositories without fetching — git/objects/submodule/base.py:893-896 After `git submodule deinit`, the retained repository under `.git/modules/<name>` can already contain the requested commit. This branch has validated that repository but now rejects `update(init=True, no_fetch=True)` merely because its checkout is empty. Reconnecting the repository and restoring cached contents requires no network access; native `git submodule update --init --no-fetch` succeeds in this case. Allow restoration and conditionally skip the later `fetch_remotes(mrepo)` instead. Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
1 parent 738e940 commit a83262b

3 files changed

Lines changed: 517 additions & 98 deletions

File tree

‎git/objects/submodule/base.py‎

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,32 @@
44
__all__ = ["Submodule", "UpdateProgress"]
55

66
import gc
7-
from io import BytesIO
87
import logging
98
import ntpath
109
import os
1110
import os.path as osp
12-
from pathlib import Path
1311
import shlex
1412
import stat
1513
import sys
16-
import uuid
1714
import urllib.parse
15+
import uuid
16+
from io import BytesIO
17+
from pathlib import Path
18+
19+
# typing ----------------------------------------------------------------------
20+
from typing import (
21+
TYPE_CHECKING,
22+
Any,
23+
Callable,
24+
Dict,
25+
Iterator,
26+
List,
27+
Literal,
28+
Mapping,
29+
Sequence,
30+
Union,
31+
cast,
32+
)
1833

1934
import git
2035
from git.cmd import Git
@@ -46,23 +61,7 @@
4661
sm_section,
4762
)
4863

49-
# typing ----------------------------------------------------------------------
50-
51-
from typing import (
52-
Any,
53-
Callable,
54-
Dict,
55-
Iterator,
56-
List,
57-
Literal,
58-
Mapping,
59-
Sequence,
60-
TYPE_CHECKING,
61-
Union,
62-
cast,
63-
)
64-
65-
from git.types import Commit_ish, PathLike, TBD
64+
from git.types import TBD, Commit_ish, PathLike
6665

6766
if TYPE_CHECKING:
6867
from git.index import IndexFile
@@ -793,8 +792,9 @@ def update(
793792
Allow unsafe options to be used, like ``--upload-pack``.
794793
795794
:param no_fetch:
796-
If ``True``, submodule updating will be attempted without fetching
797-
new changes from remotes.
795+
If ``True``, update using locally available objects and remote-tracking
796+
refs without fetching or cloning. Repositories retained after
797+
:meth:`deinit` can be restored without fetching.
798798
799799
:note:
800800
Does nothing in bare repositories.
@@ -886,15 +886,12 @@ def fetch_remotes(module_repo: "Repo") -> None:
886886
raise OSError(
887887
"Module directory at %r does already exist and is non-empty" % checkout_module_abspath
888888
)
889-
elif no_fetch:
890-
raise ValueError(
891-
"Module directory at %r is empty but fetching is disabled" % checkout_module_abspath
892-
)
893889
os.makedirs(checkout_module_abspath, exist_ok=True)
894890
self._write_git_file_and_module_config(checkout_module_abspath, module_abspath)
895891
mrepo = git.Repo(checkout_module_abspath)
896892
mrepo.head.reset(mrepo.head.commit, index=True, working_tree=True)
897-
fetch_remotes(mrepo)
893+
if not no_fetch:
894+
fetch_remotes(mrepo)
898895
with self.repo.config_writer() as writer:
899896
writer.set_value(sm_section(self.name), "url", self.url)
900897

‎git/objects/submodule/root.py‎

Lines changed: 76 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,17 @@
55

66
import logging
77

8+
# typing -------------------------------------------------------------------
9+
from typing import TYPE_CHECKING, Union
10+
811
import git
912
from git.exc import InvalidGitRepositoryError
13+
from git.types import Commit_ish
1014
from git.util import IterableList
1115

1216
from .base import Submodule, UpdateProgress
1317
from .util import find_first_remote_branch
1418

15-
# typing -------------------------------------------------------------------
16-
17-
from typing import TYPE_CHECKING, Union
18-
19-
from git.types import Commit_ish
20-
2119
if TYPE_CHECKING:
2220
from git.repo import Repo
2321

@@ -148,8 +146,9 @@ def update( # type: ignore[override]
148146
when updating submodules.
149147
150148
:param no_fetch:
151-
If ``True``, submodule updating will be attempted without fetching
152-
new changes from remotes.
149+
If ``True``, update using locally available objects and remote-tracking
150+
refs without fetching or cloning. Cached refs are preserved and used even
151+
when a submodule's URL changes.
153152
154153
:return:
155154
self
@@ -259,7 +258,7 @@ def update( # type: ignore[override]
259258
# HANDLE URL CHANGE
260259
###################
261260
if sm.url != psm.url:
262-
# Add the new remote, remove the old one.
261+
# When fetching, add the new remote and remove the old one.
263262
# This way, if the url just changes, the commits will not have
264263
# to be re-retrieved.
265264
nn = "__new_origin__"
@@ -277,34 +276,19 @@ def update( # type: ignore[override]
277276
)
278277

279278
if not dry_run:
280-
assert nn not in [r.name for r in rmts]
281-
smr = smm.create_remote(nn, sm.url)
282-
if not no_fetch:
283-
smr.fetch(progress=progress)
284-
285-
# If we have a tracking branch, it should be available
286-
# in the new remote as well.
287-
if len([r for r in smr.refs if r.remote_head == sm.branch_name]) == 0:
288-
raise ValueError(
289-
"Submodule branch named %r was not available in new submodule remote at %r"
290-
% (sm.branch_name, sm.url)
291-
)
292-
# END head is not detached
293-
294-
# Now delete the changed one.
295-
rmt_for_deletion = None
279+
previous_remote = None
296280
for remote in rmts:
297281
if remote.url == psm.url:
298-
rmt_for_deletion = remote
282+
previous_remote = remote
299283
break
300284
# END if urls match
301285
# END for each remote
302286

303287
# If we didn't find a matching remote, but have exactly
304288
# one, we can safely use this one.
305-
if rmt_for_deletion is None:
289+
if previous_remote is None:
306290
if len(rmts) == 1:
307-
rmt_for_deletion = rmts[0]
291+
previous_remote = rmts[0]
308292
else:
309293
# If we have not found any remote with the
310294
# original URL we may not have a name. This is a
@@ -317,45 +301,64 @@ def update( # type: ignore[override]
317301
# END handle one single remote
318302
# END handle check we found a remote
319303

320-
orig_name = rmt_for_deletion.name
321-
smm.delete_remote(rmt_for_deletion)
322-
# NOTE: Currently we leave tags from the deleted remotes
323-
# as well as separate tracking branches in the possibly
324-
# totally changed repository (someone could have changed
325-
# the url to another project). At some point, one might
326-
# want to clean it up, but the danger is high to remove
327-
# stuff the user has added explicitly.
328-
329-
# Rename the new remote back to what it was.
330-
smr.rename(orig_name)
331-
332-
# Early on, we verified that the our current tracking
333-
# branch exists in the remote. Now we have to ensure
334-
# that the sha we point to is still contained in the new
335-
# remote tracking branch.
336-
smsha = sm.binsha
337-
found = False
338-
rref = smr.refs[self.branch_name]
339-
for c in rref.commit.traverse():
340-
if c.binsha == smsha:
341-
found = True
342-
break
343-
# END traverse all commits in search for sha
344-
# END for each commit
345-
346-
if not found:
347-
# Adjust our internal binsha to use the one of the
348-
# remote this way, it will be checked out in the
349-
# next step. This will change the submodule relative
350-
# to us, so the user will be able to commit the
351-
# change easily.
352-
_logger.warning(
353-
"Current sha %s was not contained in the tracking\
304+
if no_fetch:
305+
# A new remote would have no cached refs. Preserve
306+
# the existing refs and tracking configuration for
307+
# offline updates instead of replacing the remote.
308+
previous_remote.set_url(git.Git.polish_url(sm.url, expand_vars=False))
309+
else:
310+
assert nn not in [r.name for r in rmts]
311+
smr = smm.create_remote(nn, sm.url)
312+
smr.fetch(progress=progress)
313+
314+
# If we have a tracking branch, it should be available
315+
# in the new remote as well.
316+
if len([r for r in smr.refs if r.remote_head == sm.branch_name]) == 0:
317+
raise ValueError(
318+
"Submodule branch named %r was not available in new submodule remote at %r"
319+
% (sm.branch_name, sm.url)
320+
)
321+
# END head is not detached
322+
323+
orig_name = previous_remote.name
324+
smm.delete_remote(previous_remote)
325+
# NOTE: Currently we leave tags from the deleted remotes
326+
# as well as separate tracking branches in the possibly
327+
# totally changed repository (someone could have changed
328+
# the url to another project). At some point, one might
329+
# want to clean it up, but the danger is high to remove
330+
# stuff the user has added explicitly.
331+
332+
# Rename the new remote back to what it was.
333+
smr.rename(orig_name)
334+
335+
# Early on, we verified that the our current tracking
336+
# branch exists in the remote. Now we have to ensure
337+
# that the sha we point to is still contained in the new
338+
# remote tracking branch.
339+
smsha = sm.binsha
340+
found = False
341+
rref = smr.refs[self.branch_name]
342+
for c in rref.commit.traverse():
343+
if c.binsha == smsha:
344+
found = True
345+
break
346+
# END traverse all commits in search for sha
347+
# END for each commit
348+
349+
if not found:
350+
# Adjust our internal binsha to use the one of the
351+
# remote this way, it will be checked out in the
352+
# next step. This will change the submodule relative
353+
# to us, so the user will be able to commit the
354+
# change easily.
355+
_logger.warning(
356+
"Current sha %s was not contained in the tracking\
354357
branch at the new remote, setting it the the remote's tracking branch",
355-
sm.hexsha,
356-
)
357-
sm.binsha = rref.commit.binsha
358-
# END reset binsha
358+
sm.hexsha,
359+
)
360+
sm.binsha = rref.commit.binsha
361+
# END reset binsha
359362

360363
# NOTE: All checkout is performed by the base
361364
# implementation of update.
@@ -385,11 +388,12 @@ def update( # type: ignore[override]
385388
if not dry_run:
386389
smm = sm.module()
387390
smmr = smm.remotes
388-
# As the branch might not exist yet, we will have to fetch
389-
# all remotes to be sure...
390-
for remote in smmr:
391-
remote.fetch(progress=progress)
392-
# END for each remote
391+
# As the branch might not exist yet, fetch all remotes
392+
# unless restricted to locally cached refs.
393+
if not no_fetch:
394+
for remote in smmr:
395+
remote.fetch(progress=progress)
396+
# END for each remote
393397

394398
try:
395399
tbr = git.Head.create(
@@ -458,6 +462,7 @@ def update( # type: ignore[override]
458462
dry_run=dry_run,
459463
force_reset=force_reset,
460464
keep_going=keep_going,
465+
no_fetch=no_fetch,
461466
)
462467
# END handle dry_run
463468
# END handle recursive

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL