Re: [PATCH] Add git-bundle - pack objects and references for disconnected transfer

From: Junio C Hamano <junkio@cox.net>
Date: Feb 16, 2007, 02:11 UTC
Message-ID: <7vodnuc0me.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <11715851972838-git-send-email-mdl123@verizon.net>

Mark Levedahl <mdl123@verizon.net> writes:

Show 7 quoted lines

> +# find the refs to carry along and get sha1s for each.
> +refs=
> +fullrevargs=
> +for arg in $gitrevargs ; do
> +	#ignore options and basis refs, get unambiguous ref name for things
> +	# we will transport (e.g., user gives master, have heads/master and
> +	# remotes/origin/master, we keep the former).

I am not sure if that is a good behaviour. What if user gives xyzzy and there are tags/xyzzy and remotes/origin/xyzzy?

I suspect it would be much safer to error out in ambiguous cases.

> +# we do things a slow way if max-age or min-age are given
> +fast=
> +[ "${fullrevargs##*--max-age}" == "$fullrevargs" ] && \
> +[ "${fullrevargs##*--min-age}" == "$fullrevargs" ] && fast=1

Style.

Our scripts tend to spell the test command "test" and equality test operator as single '='. Also they tend to say:

        case "$fullrevargs" in
        *--max-age* | *--min-age*) ... 
        esac

Show 5 quoted lines

> +	# get immediate parents of each commit to include
> +	parents=
> +	for c in $commits ; do
> +		parents="$parents $(git-rev-list --parents $c | head -1 | cut -b42-)"
> +	done

Nicely done, but you seemed to have used "head -n 1" elsewhere, which is more portable.

Show 7 quoted lines

> +	# factor out what will be in this bundle, the remainder are the bundle's pre-requisites.
> +	# double up commits in this as we only want things that are only in parents to appear once
> +	prereqs=$(printf "%s\n" $parents $commits $commits | \
> +		sort | \
> +		uniq -c | \
> +		grep ' 1 ' \
> +		| sed 's/ *1 //')

Don't pipe output of grep to sed ;-).

> +else
> +	prereqs=$(git-rev-list --objects-edge $fullrevargs | \
> +		grep '^-' | sed 's/-//')
> +fi

Likewise.  The latter is:

	... | sed -ne 's/^-//p'

> +# create refs and pack
> +[ -e "$bfile" ] && rm -f "$bfile" 2>/dev/null
> +printf "%s\n" $prereqs > .gitBundleReferences

Our scripts assume $GIT_DIR/ is writable but not necessarily the working directory when they have to use temporary files. Also please clean temporaries even when you error out with traps.

	tmp=$GIT_DIR/bundle_tmp$$
	references="$tmp-references"
        pack="$tmp-pack"
        trap 'rm -f "$tmp-*"' 0 1 2 3 15

        echo "-" >>"$references" &&
        git-show-ref $refs >>"$references" &&
        (git-rev-list ... >"$pack") || exit