Chris Packham wrote:
> It occurs to me that the UI around alternates is a bit lacking i.e.
> there isn't a git command to display the alternates in use or to add
> them to an existing repository (or at least I couldn't find one
> skimming the docs or googling). So here's my attempt to add a 'git
> alternates' command which can display, add or remove an alternate.
Quite welcome!
Something like this explanation probably belongs in the commit
message. That way, years down the line, people don’t need to trawl
the list archives to see what your goal was.
Comments:
> From a5c64de20937da132376d717f19a1d52b54701d2 Mon Sep 17 00:00:00 2001
> From: Chris Packham <judge.packham@xxxxxxxxx>
> Date: Wed, 24 Mar 2010 11:34:11 -0700
Redundant next to the email header. It is useful to be able to include
fields that differ from the e-mail header (often Subject:,
sometimes Date:, sometimes From:), but aside from that, this metadata
should be omitted when sending patches to the git mailing list.
> +#
> +# Runs through the alternates file calling the callback function $1
> +# with the name of the alternate as the first argument to the callback
> +# any additional arguments are passed to the callback function.
> +#
> +walk_alternates()
> +{
> + local alternates=$GIT_DIR/objects/info/alternates
> + local callback=$1
I couldn’t find any other uses of “local” in-tree. I assume old shells
don’t support it.
Will this be a problem for recursion? Maybe the callback should be
called in a subshell.
> + shift
> +
> + if [ -e $alternates ]; then
> + while read line
> + do
> + $callback $line $*
Probably "$line" "$@" instead of $line $* would be more flexible.
> +#
> +# Walk function to display one alternate object store and, if the user
> +# has specified -r, recursively call show_alternates on the git
> +# repository that the object store belongs to.
> +#
> +show_alternates_walk()
> +{
> + say "Object store $1"
> + say " referenced via $GIT_DIR"
> +
> + local new_git_dir=${line%%/objects}
use of local.
> + if [ "$recursive" == "true" -a "$GIT_DIR" != "$new_git_dir" ]
Should use = instead of == (portability). Also git scripts tend to
spell out 'test' and avoid the -a and -o operators:
if test "$recursive" = true && test "$GIT_DIR" != "$new_git-dir"
though that is not a hard and fast rule.
> +add_alternate()
> +{
[...]
> + touch $GIT_DIR/objects/info/alternates
Necessary?
> + echo "$(readlink -f $dir)" >> $GIT_DIR/objects/info/alternates
Maybe
readlink -f "$dir" >> $GIT_DIR/objects/info/alternates
would be simpler.
> +rewrite_alternates()
> +{
> + if test "$1" != "$2"; then
> + echo $2 >> $3
> + fi
> +}
What does this function do? (Could use a comment.)
> +del_alternate()
> +{
[...]
> + local alternates=$GIT_DIR/objects/info/alternates
use of local.
> +
> + new_alts_file=$(mktemp $alternates-XXXXXX)
Not used elsewhere in git. Is this needed? Maybe a single
$GIT_DIR/objects/info/new-alternates.tmp or similar would be good
enough.
> + # save the git from repeatedly reading a 0 length file
> + if test $(stat -c "%s" $alternates) -eq 0; then
Not used elsewhere in core git. test -s can help.
> +# Option parsing
See OPTIONS_SPEC in git-repack.sh for an example of how to simplify
this.
> +# Now go and do it
> +case $oper in
> + add) add_alternate ;;
> + del) del_alternate ;;
> + *) show_alternates ;;
> +esac
Thank you for your excellent work! Looks very useful.
Regards,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html