threads / discuss / 7719

fetch: Auto-following tags should check connectivity, not existence

Subject: fetch: Auto-following tags should check connectivity, not existence

## tl;dr

3 messages between Apr 17, 2007 and Apr 20, 2007.

replies: 2people: 2as markdown or json

Johannes Sixt· Apr 17, 2007, 16:10 UTC · lore

git-fetch's auto-following of tags fetches all tags for which it finds objects in the local repository. I feel it were better if not object existence, but connectivity to the existing refs was checked, like this:

diff --git a/git-fetch.sh b/git-fetch.sh
index fd70696..1b3c459 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -314,11 +314,12 @@ case "$no_tags$tags" in
        taglist=$(IFS=' ' &&
        echo "$ls_remote_result" |
        git-show-ref --exclude-existing=refs/tags/ |
        while read sha1 name
        do
-           git-cat-file -t "$sha1" >/dev/null 2>&1 || continue
+           t=$(git-rev-list --max-count=1 "$sha1" --not --all 2>
/dev/null) &&
+           test -z "$t" || continue
            echo >&2 "Auto-following $name"
            echo ".${name}:${name}"
        done)
    esac
    case "$taglist" in

Is this considered in the shell-to-C rewrite that's currently going on?

The background is that I sometimes fetch a branch with tags, but then
decide to remove it (and the tags as well). git-gc and git-prune do not
guarantee that the objects go away because they could be reachable from
reflogs. Now the next git-fetch will fetch the tags again even though I
do not have any refs from which they would be reachable.

-- Hannes

Junio C Hamano· Apr 20, 2007, 06:39 UTC · re: Johannes Sixt · lore

Re: fetch: Auto-following tags should check connectivity, not existence

Johannes Sixt <J.Sixt@eudaptics.com> writes:

Show 23 quoted lines

> git-fetch's auto-following of tags fetches all tags for which it finds
> objects in the local repository. I feel it were better if not object
> existence, but connectivity to the existing refs was checked, like this:
>
> diff --git a/git-fetch.sh b/git-fetch.sh
> index fd70696..1b3c459 100755
> --- a/git-fetch.sh
> +++ b/git-fetch.sh
> @@ -314,11 +314,12 @@ case "$no_tags$tags" in
>         taglist=$(IFS=' ' &&
>         echo "$ls_remote_result" |
>         git-show-ref --exclude-existing=refs/tags/ |
>         while read sha1 name
>         do
> -           git-cat-file -t "$sha1" >/dev/null 2>&1 || continue
> +           t=$(git-rev-list --max-count=1 "$sha1" --not --all 2>
> /dev/null) &&
> +           test -z "$t" || continue
>             echo >&2 "Auto-following $name"
>             echo ".${name}:${name}"
>         done)
>     esac
>     case "$taglist" in

I think "something like this" is a sensible thing to do, on top of the updates to "rev-list --objects" to verify the blob we list actually exists.

However, I think --max-count=1 defeats what you are trying to do. Revision limiting will only look at commits, and if you have all commits that lead to the "$sha1" commit from some of the existing refs, but lack some blobs or trees that belong to some of the commits that are not the first commit that will be listed, their absense will not be noticed.

Junio C Hamano· Apr 20, 2007, 10:30 UTC · re: Junio C Hamano · lore

Re: fetch: Auto-following tags should check connectivity, not existence

Junio C Hamano <junkio@cox.net> writes:

Show 12 quoted lines

> Johannes Sixt <J.Sixt@eudaptics.com> writes:
>
>> git-fetch's auto-following of tags fetches all tags for which it finds
>> objects in the local repository. I feel it were better if not object
>> existence, but connectivity to the existing refs was checked, like this:
> ...
> However, I think --max-count=1 defeats what you are trying to
> do.  Revision limiting will only look at commits, and if you
> have all commits that lead to the "$sha1" commit from some of
> the existing refs, but lack some blobs or trees that belong to
> some of the commits that are not the first commit that will be
> listed, their absense will not be noticed.

Sorry, but I have to take this back, after looking at your patch once again. In this case, all you are interested in is to see if the commit ancestry is connected, and you do not mind if the chain is somewhat incomplete in blobs and trees, as you will re-fetch the chain in a safe manner in the later round. In other words, the code does not have to be as strict as my quickfetch series, which tries to *omit* re-fetching altogether.

So --max-count=1 is fine, although it may not be much of an optimization in practice, it does not harm correctness in any way, as my previous message suggested.