From: Junio C Hamano <hidden> Date: 2016-06-15 22:48:29
Chris Packham [off-list ref] writes:
git repack -a did the correct thing.
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).
That's an understatement. Thanks for starting the effort.
I will likely to have quite a few style issues with the script
implementation, and am undecided if this should be a new command
or an option to some existing command, but it's time we have a
management facility for alternates.
You probably need one entry in the command-list.txt to classify where in
the main manual page git(1) for this command to appear. I would suggest
imitating "git config".
...
+#
+# 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
We want to use this on platforms with ksh and dash, so please refrain from
bash-ism features. "local" does not mix well with "#!/bin/sh".
+ shift
+
+ if [ -e $alternates ]; then
We tend to avoid '[' and write it like this:
if test -f "$alternates"
then
...
Also notice that an indent is one tab and one tabstop is 8 places, and
alternates need to be quoted in case $GIT_DIR has IFS whitespace in it.
+ while read line
+ do
+ $callback $line $*
As the path to an alternate object store can contain IFS whitespace, line
needs to be quoted. Also you call walk_alternates with things like "$dir"
that could also be a path with IFS whitespace, it needs to be quoted
properly. I suspect callback is only your own shell function, so it does
not have to be quoted, but it is Ok to quote it for consistency. I.e.
"$callback" "$line" "$@"
You are loooooose in quoting throughout your script, so I won't bother to
point all of them out in the rest of the message. You also are loose in
checking error returns (what you failed to write $newalternates file, for
example), which needs to be fixed before the final version.
+ done < $alternates
This is correct per POSIX even if $alternates itself has IFS whitespace in
it, but newer bash on some platforms have an obnoxious "safety feature"
(see 3fa7c3d work around an obnoxious bash "safety feature" on OpenBSD,
2010-01-26) that can cause it to barf on this. It is unfortunate but we
need to quote it like this:
done <"$alternates"
Also notice that there is no SP after < or > redirection (it is just a
style thing).
+# 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}
+ if [ "$recursive" == "true" -a "$GIT_DIR" != "$new_git_dir" ]
+ then
if test "$recursive" = true && test "$GIT_DIR" != "$new_git_dir"
then
...
+ GIT_DIR=$new_git_dir show_alternates
This is probably depending on a bug in bash and is not portable. See
76c9c0d (rebase -i: Export GIT_AUTHOR_* variables explicitly, 2010-01-22)
+add_alternate()
+{
+ if test ! -d $dir; then
+ die "fatal: $dir is not a directory"
+ fi
This will not work with relative alternates. In two repositories A and B,
where B borrows from A:
A/.git/objects/
B/.git/objects/info/alternates
the alternates file in B can point at A's .git/objects, relative to its
own .git/objects/.
+ walk_alternates check_current_alternate_walk $dir
+
+ # At this point we know that $dir is a directory that exists
+ # and that its not already being used as an alternate. We could
+ # go further and verify that $dir has valid objects.
+
+ # if we're still going we can safely add the alternate
+ touch $GIT_DIR/objects/info/alternates
+ echo "$(readlink -f $dir)" >> $GIT_DIR/objects/info/alternates
What is this touch for?
Is readlink(1) portable enough across platforms? A more fundamental
question is if resolving symbolic link like this behind user is a sensible
thing to do, especially as you are ...
+ say "$dir added as an alternate"
... lying to the user here, if $dir indeed is a symbolic link.
+rewrite_alternates()
+{
+ if test "$1" != "$2"; then
+ echo $2 >> $3
+ fi
+}
That's a misleading name for this helper function.
+del_alternate()
+{
+ if test ! $force = "true"; then
+ say "Not forced, use"
+ say " 'git repack -a' to fetch missing objects, then "
+ say " '$dashless -f -d $dir' to remove the alternate"
+ die
+ fi
+
+ local alternates=$GIT_DIR/objects/info/alternates
+
+ new_alts_file=$(mktemp $alternates-XXXXXX)
+ touch $new_alts_file
+
+ walk_alternates rewrite_alternates $dir $new_alts_file
I think this is a more expensive way to say:
grep -v -F "$dir" <"$alternates" >"$new_alternates"
+ mv $new_alts_file $alternates
+
+ # save the git from repeatedly reading a 0 length file
+ if test $(stat -c "%s" $alternates) -eq 0; then
+ rm $alternates
+ fi
Good point, but it would be better to do something like this:
grep -v -F "$dir" <"$alternates" >"$new_alternates"
if test -s "$new_alternates"
then
mv "$new_alternates" "$alternates"
else
rm -f "$alternates"
fi
without making it less portable by using "stat".
From: Chris Packham <hidden> Date: 2016-06-15 22:48:29
On Wed, Mar 24, 2010 at 12:58 PM, Junio C Hamano [off-list ref] wrote:
Chris Packham [off-list ref] writes:
quoted
git repack -a did the correct thing.
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).
That's an understatement. Thanks for starting the effort.
Thanks for the comments (and lessons in portable shell scripting).
I'll re-roll shortly. Some responses to specific questions below.
I will likely to have quite a few style issues with the script
implementation, and am undecided if this should be a new command
or an option to some existing command, but it's time we have a
management facility for alternates.
Fair enough about style issues, I tend to code everything like its C I
guess some habits are hard to break. I was wondering about the
justification for having a separate command but I couldn't think of
anywhere else it'd fit. Also calling the command alternates when we
create them with git clone --reference seems a bit funny.
You probably need one entry in the command-list.txt to classify where in
the main manual page git(1) for this command to appear. I would suggest
imitating "git config".
...
+#
+# 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
We want to use this on platforms with ksh and dash, so please refrain from
bash-ism features. "local" does not mix well with "#!/bin/sh".
quoted
+ shift
+
+ if [ -e $alternates ]; then
We tend to avoid '[' and write it like this:
if test -f "$alternates"
then
...
I think there were a few of these I missed before submitting. Easy to fix.
Also notice that an indent is one tab and one tabstop is 8 places, and
alternates need to be quoted in case $GIT_DIR has IFS whitespace in it.
I must have used the wrong thing as an example. I actually started
with tabs and converted it to spaces after looking at
git-mergetool.sh.
quoted
+ while read line
+ do
+ $callback $line $*
As the path to an alternate object store can contain IFS whitespace, line
needs to be quoted. Also you call walk_alternates with things like "$dir"
that could also be a path with IFS whitespace, it needs to be quoted
properly. I suspect callback is only your own shell function, so it does
not have to be quoted, but it is Ok to quote it for consistency. I.e.
"$callback" "$line" "$@"
You are loooooose in quoting throughout your script, so I won't bother to
point all of them out in the rest of the message. You also are loose in
checking error returns (what you failed to write $newalternates file, for
example), which needs to be fixed before the final version.
I'll fix these.
quoted
+ done < $alternates
This is correct per POSIX even if $alternates itself has IFS whitespace in
it, but newer bash on some platforms have an obnoxious "safety feature"
(see 3fa7c3d work around an obnoxious bash "safety feature" on OpenBSD,
2010-01-26) that can cause it to barf on this. It is unfortunate but we
need to quote it like this:
done <"$alternates"
Also notice that there is no SP after < or > redirection (it is just a
style thing).
quoted
+# 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}
+ if [ "$recursive" == "true" -a "$GIT_DIR" != "$new_git_dir" ]
+ then
if test "$recursive" = true && test "$GIT_DIR" != "$new_git_dir"
then
...
quoted
+ GIT_DIR=$new_git_dir show_alternates
This is probably depending on a bug in bash and is not portable. See
76c9c0d (rebase -i: Export GIT_AUTHOR_* variables explicitly, 2010-01-22)
OK I thought that was a standard thing. Exporting could play havoc
with recursion, will have to look for a solution for that.
quoted
+add_alternate()
+{
+ if test ! -d $dir; then
+ die "fatal: $dir is not a directory"
+ fi
This will not work with relative alternates. In two repositories A and B,
where B borrows from A:
A/.git/objects/
B/.git/objects/info/alternates
the alternates file in B can point at A's .git/objects, relative to its
own .git/objects/.
So would the best approach be not to validate the input or to validate
it relative to $PWD and .git/objects/. I think the answer ties into my
use of readlink.
quoted
+ walk_alternates check_current_alternate_walk $dir
+
+ # At this point we know that $dir is a directory that exists
+ # and that its not already being used as an alternate. We could
+ # go further and verify that $dir has valid objects.
+
+ # if we're still going we can safely add the alternate
+ touch $GIT_DIR/objects/info/alternates
+ echo "$(readlink -f $dir)" >> $GIT_DIR/objects/info/alternates
What is this touch for?
I wasn't 100% sure >> would work if the file didn't exist. I just
tried on a bash shell and it works. Does anyone know of any supported
shells that behave differently w.r.t the >> operator?
Is readlink(1) portable enough across platforms? A more fundamental
question is if resolving symbolic link like this behind user is a sensible
thing to do, especially as you are ...
quoted
+ say "$dir added as an alternate"
... lying to the user here, if $dir indeed is a symbolic link.
Using readlink was my hack around converting a user specified relative
path to an absolute one. I actually would prefer it if it didn't
interpret a symlink. I also had my doubts about portability its
probably not going to exist on platforms that don't have real symlinks
(windows). What I really wanted to do was something like "abspath
$dir" but I was bitterly disappointed to find that was a gnu make-ism.
Any suggestions?
quoted
+rewrite_alternates()
+{
+ if test "$1" != "$2"; then
+ echo $2 >> $3
+ fi
+}
That's a misleading name for this helper function.
I think this is made redundant by your grep suggestion below.
quoted
+del_alternate()
+{
+ if test ! $force = "true"; then
+ say "Not forced, use"
+ say " 'git repack -a' to fetch missing objects, then "
+ say " '$dashless -f -d $dir' to remove the alternate"
+ die
+ fi
+
+ local alternates=$GIT_DIR/objects/info/alternates
+
+ new_alts_file=$(mktemp $alternates-XXXXXX)
+ touch $new_alts_file
+
+ walk_alternates rewrite_alternates $dir $new_alts_file
I think this is a more expensive way to say:
grep -v -F "$dir" <"$alternates" >"$new_alternates"
Much easier. I'll do that.
quoted
+ mv $new_alts_file $alternates
+
+ # save the git from repeatedly reading a 0 length file
+ if test $(stat -c "%s" $alternates) -eq 0; then
+ rm $alternates
+ fi
Good point, but it would be better to do something like this:
grep -v -F "$dir" <"$alternates" >"$new_alternates"
if test -s "$new_alternates"
then
mv "$new_alternates" "$alternates"
else
rm -f "$alternates"
fi
without making it less portable by using "stat".
From: Chris Packham <hidden> Date: 2016-06-15 22:48:29
Heres my updated patch. I think I've addressed all the comments except
Stephen's.
I've also added some tests in the 2nd patch but I'm running into a problem
where all the tests pass but the overall test fails:
FATAL: Unexpected exit with code 0
make: *** [t9800-git-alternate.sh] Error 1
My current TODO list is
- fix the test
- add tests for git alternate --delete
- add documentation
- refine the display output, its a bit jumbled at the moment it could be better
I'm travelling for the next couple of weeks so I may not be that quick to
respond to comments.
From: Chris Packham <hidden> Date: 2016-06-15 22:48:29
The current UI around alternates is lacking. There are commands to add
an alternate at the time of a clone (e.g. 'git clone --reference') but
no commands to see what alternates have been configured or to add an
alternate after a repository has been cloned. This patch adds a
friendlier UI for displaying and configuring alternates.
Signed-off-by: Chris Packham <redacted>
---
Hopefully I've addressed the comments from Junio and Jonathan. I haven't tried
to re-work the commands into show|add|delete as per Stephens suggestion. I have
subtly changed from 'alternates' to 'alternate' i.e. ditched the plural in line
with commands like 'git remote'
Makefile | 1 +
git-alternate.sh | 155 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 156 insertions(+), 0 deletions(-)
create mode 100755 git-alternate.sh
@@ -0,0 +1,155 @@+#!/bin/sh+#+# This file is licensed under the GPL v2+#++OPTIONS_KEEPDASHDASH=+OPTIONS_SPEC="\+gitalternate[-r|--recursive]+gitalternate[-a|--adddir]+gitalternate[-f|--force][-d|deletedir]+--+r,recursiverecursivelyfollowalternates+a,add=adddirasanalternate+d,del=deletedirasanalternate+f,forceforceadeleteoperation+"+.git-sh-setup++#+# Given the path in $1, which may or may not be a relative path,+# convert it to an absolute path+#+abspath()+{+cd"$1"+pwd+cd->/dev/null+}++#+# 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()+{+alternates=$GIT_DIR/objects/info/alternates+callback=$1+shift++iftest-f"$alternates"+then+whilereadline+do+$callback"$line""$@"+done<"$alternates"+fi+}++#+# 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"++new_git_dir=${line%%/objects}+iftest"$recursive"="true"&&test"$GIT_DIR"!="$new_git_dir"+then+(+exportGIT_DIR=$new_git_dir+show_alternates+)+fi+}++# Display alternates currently configured+show_alternates()+{+walk_alternatesshow_alternates_walk+}++#+# Walk function to check that the specified alternate does not+# already exist.+#+check_current_alternate_walk()+{+iftest"$1"="$2";then+die"fatal: Object store $2 is already used by $GIT_DIR"+fi+}++# Add a new alternate+add_alternate()+{+iftest!-d"$dir";then+die"fatal: $dir is not a directory"+fi++abs_dir=$(abspath"$dir")+walk_alternatescheck_current_alternate_walk"$abs_dir"++# At this point we know that $dir is a directory that exists+# and that its not already being used as an alternate. We could+# go further and verify that $dir has valid objects.++# if we're still going we can safely add the alternate+echo"$abs_dir">>$GIT_DIR/objects/info/alternates+say"$abs_dir added as an alternate"+say" use 'git repack -adl' to remove duplicate objects"+}++# Deletes the name alternate from the alternates file.+# If there are no more alternates the alternates file will be removed+del_alternate()+{+iftest!$force="true";then+say"Not forced, use"+say" 'git repack -a' to fetch missing objects, then "+say" '$dashless -f -d $dir' to remove the alternate"+die+fi++alternates=$GIT_DIR/objects/info/alternates+new_alternates=$alternates.tmp++grep-v-F"$dir"<"$alternates">"$new_alternates"+iftest-s"$new_alternates"+then+mv"$new_alternates""$alternates"+else+# save the git from repeatedly reading a 0 length file+rm-f"$alternates"+fi+}++dir=""+oper=""+force="false"++# Option parsing+whiletest$#!=0+do+case"$1"in+-r|--recursive)recursive="true";;+-a|--add)oper="add";dir="$2";shift;;+-d|--delete)oper="del";dir="$2";shift;;+-f|--force)force="true";;+--)shift;break;;+*)usage;;+esac+shift+done++# Now go and do it+case"$oper"in+add)add_alternate;;+del)del_alternate;;+*)show_alternates;;+esac+
From: Chris Packham <hidden> Date: 2016-06-15 22:48:30
Signed-off-by: Chris Packham <redacted>
---
I wasn't sure about the test numbering so I just grabbed the highest one. Still
need to add tests for the deletion use case.
t/t9800-git-alternate.sh | 95 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 95 insertions(+), 0 deletions(-)
create mode 100644 t/t9800-git-alternate.sh
@@ -0,0 +1,95 @@+#!/bin/sh++test_description='Test of git alternate command'++../test-lib.sh++test_expect_success\+'Setup for rest of the test''+mkdir-pbase&&+cdbase&&+gitinit&&+echotest>a.txt&&+echotest>b.txt&&+echotest>c.txt&&+gitadd*.txt&&+gitcommit-a-m"Initial Commit"&&+cd..&&+gitclonebaseA&&+gitcloneAB&&+gitcloneAC&&+gitcloneAD+'++test_expect_success\+'Add alternate after clone''+(cdB&&+gitalternate-a../base/.git/objects+)+'++test_expect_success\+'add same alternate fails adding existing abs path''+(cdB&&+test_must_failgitalternate-a$PWD/base/.git/objects+)+'++test_expect_success\+'add same alternate fails adding existing relative path''+(cdB&&+test_must_failgitalternate-a../base/.git/objects+)+'++test_expect_success\+'Add multiple alternates''+(cdC&&+gitalternate-a../base/.git/objects&&+gitalternate-a../B/.git/objects+)+'++test_expect_success\+'Add recursive alternate''+(cdD&&+gitalternate-a../C/.git/objects+)+'++test_expect_success\+'test git alternate display''+testbase=$PWD+(cdB&&+gitalternate>actual&&+{+echo"Object store $testbase/base/.git/objects"+echo" referenced via $testbase/B/.git"+}>expect&&+test_cmpexpectactual+)+'++test_expect_success\+'test git alternate recursive display''+testbase=$PWD+(cdD&&++gitalternate-r>actual&&++{+echo"Object store $testbase/C/.git/objects"+echo" referenced via $testbase/D/.git"+echo"Object store $testbase/base/.git/objects"+echo" referenced via $testbase/C/.git"+echo"Object store $testbase/B/.git/objects"+echo" referenced via $testbase/C/.git"+echo"Object store $testbase/base/.git/objects"+echo" referenced via $testbase/B/.git"+}>expect&&++test_cmpexpectactual+)+'++#rm -rf A B C D base
+test_expect_success \
+ 'Setup for rest of the test' '
The modern style for test headers is
test_expect_success 'Setup for rest of the test' '
test goes here
'
It may improve readability if you insert a blank line after the headline.
+ mkdir -p base &&
+ cd base &&
+ git init &&
+ echo test > a.txt &&
+ echo test > b.txt &&
+ echo test > c.txt &&
+ git add *.txt &&
+ git commit -a -m "Initial Commit" &&
+ cd .. &&
Do not use 'cd dir && ... && cd ..', use (cd dir && ...) like you did in
the rest of the tests.
+test_expect_success \
+ 'Add alternate after clone' '
+ (cd B &&
+ git alternate -a ../base/.git/objects
+ )
We saw tests like this written with more whitespace like:
(
cd B &&
git alternate -a ../base/.git/objects
)
+test_expect_success \
+ 'add same alternate fails adding existing abs path' '
+ (cd B &&
+ test_must_fail git alternate -a $PWD/base/.git/objects
This use of $PWD is OK, but for consistency it should be $(pwd) like
below. Moreover, it needs double-quotes.
You must write this as (d-quotes not needed here)
testbase=$(pwd) &&
for the benefit of Windows. The difference is that $PWD returns /c/path,
but $(pwd) returns c:/path.
When the alternate was set up using the command, git has only ever seen a
c:/path style path (regardless of whether you used $PWD or $(pwd), because
the path is converted to c:/path by the shell before it invokes git), and
therefore the alternates file contains this style.
But when the expected result is constructed, the /c/path style is *not*
converted to c:/path; and a mismatch would be detected.
From: Junio C Hamano <hidden> Date: 2016-06-15 22:48:31
Chris Packham [off-list ref] writes:
+abspath()
+{
+ cd "$1"
+ pwd
+ cd - > /dev/null
+}
I do not think "cd -" is all that portable. As you will be always using this in
this form:
somevariable=$(abspath "$it")
you are in a subshell and won't affect the caller anyway. Why not drop
the "go back to where we came from"?
Also a shell built-in "pwd" tends to be fooled by $PWD especially since
you are running "cd" without -P.
+#
+# 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()
This is more like "for_each_alternates"; "walk" can be mistaken as if you
are recursively looking at alternates defined in alternate repositories
of the repository you start from.
+{
+ alternates=$GIT_DIR/objects/info/alternates
+ callback=$1
+ shift
+
+ if test -f "$alternates"
+ then
+ while read line
+ do
+ $callback "$line" "$@"
+ done< "$alternates"
done <"$alternates"
How well does this handle relative alternate object stores? Shouldn't it
be more like this?
while read altdir
do
case "$altdir" in
/*) ;; # full path
*) altdir="$GIT_DIR/objects/$altdir" ;;
esac &&
$callback "$altdir" "$@"
done <"$alternates"
+# 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"
+
+ new_git_dir=${line%%/objects}
Do you need double-% here, not a single one?
+# Add a new alternate
+add_alternate()
+{
+ if test ! -d "$dir"; then
+ die "fatal: $dir is not a directory"
+ fi
+
+ abs_dir=$(abspath "$dir")
+ walk_alternates check_current_alternate_walk "$abs_dir"
+
+ # At this point we know that $dir is a directory that exists
+ # and that its not already being used as an alternate. We could
s/its/(it's|it is)/;
But I don't think it is true that you have verified that it is not used.
You are running abspath on the input from the end user, but you are using
existing entries in the alternates file that may not be absolute. They
can be relative to $GIT_DIR/objects/.
+ say " use 'git repack -adl' to remove duplicate objects"
Good.
+# Deletes the name alternate from the alternates file.
+# If there are no more alternates the alternates file will be removed
+del_alternate()
+{
+ if test ! $force = "true"; then
+ say "Not forced, use"
+ say " 'git repack -a' to fetch missing objects, then "
+ say " '$dashless -f -d $dir' to remove the alternate"
+ die
Hmm, I am afraid that this will end up training users to always say -f
without even thinking. Shouldn't this code be doing whatever necessary
steps to make sure this repository has all the necessary objects without
the named alternates and then removing the file? An easiest might be to
temporarily remove the entry and run fsck, perhaps?
From: Chris Packham <hidden> Date: 2016-06-15 22:48:32
Sorry, just realized I missed this email before sending my last update.
On Mon, Mar 29, 2010 at 12:32 AM, Junio C Hamano [off-list ref] wrote:
Chris Packham [off-list ref] writes:
quoted
+abspath()
+{
+ cd "$1"
+ pwd
+ cd - > /dev/null
+}
I do not think "cd -" is all that portable. As you will be always using this in
this form:
somevariable=$(abspath "$it")
you are in a subshell and won't affect the caller anyway. Why not drop
the "go back to where we came from"?
Fair enough. Should be safe.
Also a shell built-in "pwd" tends to be fooled by $PWD especially since
you are running "cd" without -P.
Are you saying I should switch to "echo $PWD"?
quoted
+#
+# 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()
This is more like "for_each_alternates"; "walk" can be mistaken as if you
are recursively looking at alternates defined in alternate repositories
of the repository you start from.
Easy change.
quoted
+{
+ alternates=$GIT_DIR/objects/info/alternates
+ callback=$1
+ shift
+
+ if test -f "$alternates"
+ then
+ while read line
+ do
+ $callback "$line" "$@"
+ done< "$alternates"
done <"$alternates"
How well does this handle relative alternate object stores?
Not well at all.
Shouldn't it be more like this?
while read altdir
do
case "$altdir" in
/*) ;; # full path
*) altdir="$GIT_DIR/objects/$altdir" ;;
esac &&
$callback "$altdir" "$@"
done <"$alternates"
Thanks, I was wondering how to handle relative paths.
quoted
+# 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"
+
+ new_git_dir=${line%%/objects}
Do you need double-% here, not a single one?
Not if I'm not using a regex. I'll change it.
quoted
+# Add a new alternate
+add_alternate()
+{
+ if test ! -d "$dir"; then
+ die "fatal: $dir is not a directory"
+ fi
+
+ abs_dir=$(abspath "$dir")
+ walk_alternates check_current_alternate_walk "$abs_dir"
+
+ # At this point we know that $dir is a directory that exists
+ # and that its not already being used as an alternate. We could
s/its/(it's|it is)/;
But I don't think it is true that you have verified that it is not used.
You are running abspath on the input from the end user, but you are using
existing entries in the alternates file that may not be absolute. They
can be relative to $GIT_DIR/objects/.
I'll add some actual verification
quoted
+ say " use 'git repack -adl' to remove duplicate objects"
Good.
quoted
+# Deletes the name alternate from the alternates file.
+# If there are no more alternates the alternates file will be removed
+del_alternate()
+{
+ if test ! $force = "true"; then
+ say "Not forced, use"
+ say " 'git repack -a' to fetch missing objects, then "
+ say " '$dashless -f -d $dir' to remove the alternate"
+ die
Hmm, I am afraid that this will end up training users to always say -f
without even thinking. Shouldn't this code be doing whatever necessary
steps to make sure this repository has all the necessary objects without
the named alternates and then removing the file? An easiest might be to
temporarily remove the entry and run fsck, perhaps?
Yeah I had the same fear. I was wondering how to proceed with this part.
I take it your suggestion is to update the alternates file and run git
fsck to see if any objects are missing then, if there are missing
objects, switch to the original alternates file run git repack -a to
fetch the missing objects and finally switch to the new alternates
file. I'll code something up for us to discuss.