* Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype
From: Stephan Beyer @ 2008-07-03 21:09 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin
Cc: git, Daniel Barkalow, Christian Couder
In-Reply-To: <7vbq1f68rh.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, Jul 03, 2008 at 12:03:49PM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 2 Jul 2008, Junio C Hamano wrote:
> > Stephan Beyer <s-beyer@gmx.net> writes:
> >
> > > git sequencer is planned as a backend for user scripts
> > > that execute a sequence of git instructions and perhaps
> > > need manual intervention, for example git-rebase or git-am.
> >
> > ...
> > > +output () {
> > > + case "$VERBOSE" in
> > > + 0)
> > > + "$@" >/dev/null
> > > + ;;
> > > + 1)
> > > + output=$("$@" 2>&1 )
> > > + status=$?
> > > + test $status -ne 0 && printf '%s\n' "$output"
> > > + return $status
> > > + ;;
> > > + 2)
> > > + "$@"
> > > + ;;
> > > + esac
> > > +}
> >
> > Perhaps misnamed? This feels more like "do" or "perform" or "run".
>
> My fault. I like "perform".
Right, the "output" name puts a wrong behavior in mind.
"perform" (or "do" or "run") won't exactly say that this function
"adjusts" the output somehow, but I'm nonetheless fine with taking
"perform".
(I inline-attach patches so that you can incrementally review what I did
with your feedback.)
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -75,7 +75,7 @@ quit () {
exit 0
}
-output () {
+perform () {
case "$VERBOSE" in
0)
"$@" >/dev/null
@@ -165,7 +165,7 @@ restore () {
git symbolic-ref HEAD "$HEADNAME"
;;
esac &&
- output git reset --hard "$HEAD"
+ perform git reset --hard "$HEAD"
}
has_action () {
@@ -208,14 +208,14 @@ pick_one () {
then
test "$(git rev-parse --verify "$1^")" = \
"$(git rev-parse --verify HEAD)" &&
- output git reset --hard "$1" &&
+ perform git reset --hard "$1" &&
return
fi
test "$1" != '--edit' -a "$what" = 'revert' &&
what='revert --no-edit'
test -n "$SIGNOFF" &&
what="$what -s"
- $use_output git $what "$@"
+ $use_perform git $what "$@"
}
nth_string () {
@@ -945,15 +945,15 @@ insn_pick () {
# Be kind to users and ignore --mainline=1 on non-merge commits
test -n "$mainline" -a 2 -gt $(count_parents "$sha1") && mainline=
- use_output=
+ use_perform=
test -n "$edit_msg" ||
- use_output=output
+ use_perform=perform
pick_one "$op" $edit_msg $mainline $sha1 ||
die_with_patch $sha1 "Could not apply $sha1"
test -n "$EDIT" ||
- use_output=output
+ use_perform=perform
get_current_author
signoff=
@@ -961,18 +961,18 @@ insn_pick () {
if test -n "$AUTHOR" -a -n "$MESSAGE"
then
# this is just because we only want to do ONE amending commit
- $use_output git commit --amend $EDIT $signoff --no-verify \
+ $use_perform git commit --amend $EDIT $signoff --no-verify \
--author "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" \
--message="$MESSAGE"
else
# correct author if AUTHOR is set
test -n "$AUTHOR" &&
- $use_output git commit --amend $EDIT --no-verify -C HEAD \
+ $use_perform git commit --amend $EDIT --no-verify -C HEAD \
--author "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
# XXX: a git-cherry-pick --author could be nicer here
# correct commit message if MESSAGE is set
test -n "$MESSAGE" &&
- $use_output git commit --amend $EDIT $signoff --no-verify \
+ $use_perform git commit --amend $EDIT $signoff --no-verify \
-C HEAD --message="$MESSAGE"
fi
@@ -1128,12 +1128,12 @@ insn_squash () {
case "$(peek_next_command)" in
squash)
edit_commit=
- use_output=output
+ use_perform=perform
cp "$MSG" "$squash_msg"
;;
*)
edit_commit=-e
- use_output=
+ use_perform=
rm -f "$squash_msg" || exit
;;
esac
@@ -1157,9 +1157,9 @@ insn_squash () {
failed=
if test -n "$from"
then
- output git reset --soft "$sha1"
+ perform git reset --soft "$sha1"
else
- output git reset --soft HEAD^
+ perform git reset --soft HEAD^
pick_one cherry-pick -n "$sha1" || failed=t
fi
@@ -1169,7 +1169,7 @@ insn_squash () {
if test -z "$failed"
then
# This is like --amend, but with a different message
- with_author $use_output git commit --no-verify \
+ with_author $use_perform git commit --no-verify \
-F "$MSG" $edit_commit || failed=t
else
cp "$MSG" "$GIT_DIR/MERGE_MSG"
@@ -1320,13 +1320,13 @@ insn_merge () {
fi
mark_action_done
- if ! with_author output git merge $strategy -m junk $new_parents
+ if ! with_author perform git merge $strategy -m junk $new_parents
then
git rerere
cat "$MSG" >"$GIT_DIR/MERGE_MSG"
die_to_continue 'Error merging'
fi
- with_author output git commit --amend -F "$MSG" --no-verify
+ with_author perform git commit --amend -F "$MSG" --no-verify
return 0
}
@@ -1352,7 +1352,7 @@ insn_reset () {
comment_for_reflog reset
mark_action_done
- output git reset --hard "$(mark_to_commit "$1")"
+ perform git reset --hard "$(mark_to_commit "$1")"
}
@@ -1375,7 +1375,7 @@ insn_ref () {
comment_for_reflog ref
mark_action_done
- output git update-ref "$1" HEAD
+ perform git update-ref "$1" HEAD
}
@@ -1547,10 +1547,10 @@ do_startup () {
then
ONTO="refs/heads/${ONTO##*/}"
echo "$ONTO" >"$SEQ_DIR/onto"
- output git checkout "$(git rev-parse "$ONTO")" ||
+ perform git checkout "$(git rev-parse "$ONTO")" ||
die_abort "Could not checkout branch $ONTO"
else
- output git checkout "$ONTO" ||
+ perform git checkout "$ONTO" ||
die_abort "Could not checkout commit $ONTO"
ONTO=
fi
@@ -1653,7 +1653,7 @@ Fix with git sequencer --edit or abort with $(print_caller --abort)."
comment_for_reflog skip
git rerere clear
- output git reset --hard "$(cat "$SEQ_DIR/skiphead")" &&
+ perform git reset --hard "$(cat "$SEQ_DIR/skiphead")" &&
rm -f "$WHY_FILE" &&
echo '# SKIPPED the last instruction' >>"$DONE" &&
execute_rest
###
On Wed, Jul 02, 2008 at 06:45:06PM -0700, Junio C Hamano wrote:
> > +LAST_COUNT=
> > +mark_action_done () {
> > + sed -e 1q <"$TODO" >>"$DONE"
> > + sed -e 1d <"$TODO" >"$TODO.new"
> > + mv -f "$TODO.new" "$TODO"
> > + if test "$VERBOSE" -gt 0
> > + then
> > + count=$(grep -c '^[^#]' <"$DONE")
> > + total=$(expr "$count" + "$(grep -c '^[^#]' <"$TODO")")
>
> Here we are not counting lines that are comments as insns (I am not
> complaining; just making a mental note).
Add to your note, that empty lines are also skipped, because at least
one character (which is not #) must be there.
And add to your note, that leading whitespace is not ignored on that
checks, which can lead to wrong numbers, but: (quoting Dscho)
> "count" and "total" are only used for the progress output
(The builtin handles that better because it just does proper parsing
and not such hacks.)
Junio C Hamano wrote:
> > + if test "$LAST_COUNT" != "$count"
> > + then
> > + LAST_COUNT="$count"
> > + test "$VERBOSE" -lt 1 ||
> > + printf 'Sequencing (%d/%d)\r' "$count" "$total"
> > + test "$VERBOSE" -lt 2 || echo
> > + fi
> > + fi
> > +}
> > +
> > +# Generate message, patch and author script files
> > +make_patch () {
> > + parent_sha1=$(git rev-parse --verify "$1"^) ||
> > + die "Cannot get patch for $1^"
> > + git diff-tree -p "$parent_sha1..$1" >"$PATCH"
>
> Could there be a case where we need/want to deal with a root commit
> without parents?
Yes, and this is not handled correctly.
I've taken Jakub's idea:
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -139,9 +139,9 @@ mark_action_done () {
# Generate message, patch and author script files
make_patch () {
- parent_sha1=$(git rev-parse --verify "$1"^) ||
- die "Cannot get patch for $1^"
- git diff-tree -p "$parent_sha1..$1" >"$PATCH"
+ parent_sha1=$(git rev-parse --verify "$1^" 2>/dev/null ||
+ echo '--root')
+ git diff-tree -p "$parent_sha1" "$1" >"$PATCH"
test -f "$MSG" ||
commit_message "$1" >"$MSG"
test -f "$AUTHOR_SCRIPT" ||
###
Btw, another root commit problem is btw that it's not possible
to cherry-pick root commits.
Junio C Hamano wrote:
> > + test -f "$MSG" ||
> > + commit_message "$1" >"$MSG"
> > + test -f "$AUTHOR_SCRIPT" ||
> > + get_author_ident_from_commit "$1" >"$AUTHOR_SCRIPT"
> > +}
> > +
> > +# Generate a patch and die with "conflict" status code
> > +die_with_patch () {
> > + make_patch "$1"
> > + git rerere
> > + die_to_continue "$2"
> > +}
> > +
> > +restore () {
> > + git rerere clear
> > +
> > + HEADNAME=$(cat "$SEQ_DIR/head-name")
> > + HEAD=$(cat "$SEQ_DIR/head")
>
> Perhaps
>
> read HEADNAME <"$SEQ_DIR/head-name"
>
> provided if these values are $IFS safe?
They are IFS-safe. I don't really have an opinion if
read FOO <foo
is better than
FOO=$(cat foo)
or vice versa, so I have no problem to change this.
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -158,8 +158,8 @@ die_with_patch () {
restore () {
git rerere clear
- HEADNAME=$(cat "$SEQ_DIR/head-name")
- HEAD=$(cat "$SEQ_DIR/head")
+ read HEADNAME <"$SEQ_DIR/head-name"
+ read HEAD <"$SEQ_DIR/head"
case $HEADNAME in
refs/*)
git symbolic-ref HEAD "$HEADNAME"
@@ -1496,9 +1496,10 @@ prepare_editable_todo () {
}
get_saved_options () {
- VERBOSE=$(cat "$SEQ_DIR/verbose")
- ONTO=$(cat "$SEQ_DIR/onto")
- WHY=$(cat "$WHY_FILE" 2>/dev/null)
+ read VERBOSE <"$SEQ_DIR/verbose"
+ read ONTO <"$SEQ_DIR/onto"
+ test -f "$WHY_FILE" &&
+ read WHY <"$WHY_FILE"
return 0
}
###
Johannes Schindelin wrote:
> > > +# Usage: pick_one (cherry-pick|revert) [-*|--edit] sha1
> > > +pick_one () {
> > > + what="$1"
> > > + # we just assume that this is either cherry-pick or revert
> > > + shift
> > > +
> > > + # check for fast-forward if no options are given
> > > + if expr "x$1" : 'x[^-]' >/dev/null
> > > + then
> > > + test "$(git rev-parse --verify "$1^")" = \
> > > + "$(git rev-parse --verify HEAD)" &&
> > > + output git reset --hard "$1" &&
> > > + return
> > > + fi
> > > + test "$1" != '--edit' -a "$what" = 'revert' &&
> > > + what='revert --no-edit'
> >
> > This looks somewhat wrong.
> >
> > When the history looks like ---A---B and we are at A, cherry-picking B can
> > be optimized to just advancing to B, but that optimization has a slight
> > difference (or two) in the semantics.
> >
> > (1) The committer information would not record the user and time of the
> > sequencer operation, which actually may be a good thing.
>
> This is debatable. But I think you are correct, for all the same reasons
> why a merge can result in a fast-forward.
Dscho, you mean me by referring to 'you' here, right?
Otherwise I'm a bit confused: "For the same reasons why a merge can result
in a fast-forward we should not do fast forward here" ;-)
I think the ff is more useful than real picking and I think the rebase-i
test suite even has a test case for this.
(Checked: "* FAIL 17: merge redoes merges" if it is removed, so it's
implicitly checked for redone merges.)
> > (2) When $what is revert, this codepath shouldn't be exercised, should
> > it?
>
> Yes.
I haven't done a check intentionally, but there was a stupid thinko.
So you're right.
But: this will only be a bug if the commit that _comes next in the
original history_ is to be reverted. This is usually user-nonsense
(I can't imagine a useful application for that), but nonetheless a
user can try it for fun and such a case is not worth a sanity check.
So it's perhaps good to add a test here and just do the revert (and no
ff) as the user requests.
> > (3) If B is a merge, even if $what is pick, this codepath shouldn't be
> > exercised, should it?
>
> I think it should, again for the same reason why a merge can result in a
> fast-forward.
Right.
Junio C Hamano wrote:
> As to the syntax I tend to prefer
>
> case "$1" in
> -*) ... do option thing ... ;;
> *) ... do other thing... ;;
> esac
Christian has mentioned this, but I didn't listen to him ;)
Because I think, if you put the cases, the do_thing and the ;;
on separate lines,
1. it is just consistent compared to the cases where you
do several things ... you don't do them on one line and
you put the ;; on an extra line
2. the diffs will look better on extensions
> So how about...
>
> case "$what,$1" in
> revert,--edit)
> what='revert --no-edit' ;;
> revert,* | cherry-pick,-* )
> ;;
> *)
> if ! git rev-parse --verify "$1^2" &&
> test "$(git rev-parse --verify "$1^") = \
> "$(git rev-parse --verify HEAD)"
> then
> output git reset --hard "$1"
> return
> fi
> ;;
> esac
I've done this:
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -200,19 +200,25 @@ with_author () {
# Usage: pick_one (cherry-pick|revert) [-*|--edit] sha1
pick_one () {
what="$1"
- # we just assume that this is either cherry-pick or revert
shift
- # check for fast-forward if no options are given
- if expr "x$1" : 'x[^-]' >/dev/null
- then
- test "$(git rev-parse --verify "$1^")" = \
- "$(git rev-parse --verify HEAD)" &&
- perform git reset --hard "$1" &&
+ case "$what,$1" in
+ revert,*)
+ test "$1" != '--edit' &&
+ what='revert --no-edit'
+ ;;
+ cherry-pick,-*)
+ ;;
+ cherry-pick,*)
+ # fast forward
+ if test "$(git rev-parse --verify "$1^")" = \
+ "$(git rev-parse --verify HEAD)"
+ then
+ perform git reset --hard "$1"
return
- fi
- test "$1" != '--edit' -a "$what" = 'revert' &&
- what='revert --no-edit'
+ fi
+ ;;
+ esac
test -n "$SIGNOFF" &&
what="$what -s"
$use_perform git $what "$@"
###
Johannes Schindelin wrote:
> > > +make_squash_message () {
> > > + if test -f "$squash_msg"
> > > + then
> > > + count=$(($(sed -n -e 's/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p' \
> > > + <"$squash_msg" | sed -n -e '$p')+1))
> > > + echo "# This is a combination of $count commits."
> > > + sed -e '1d' -e '2,/^./{
> > > + /^$/d
> > > + }' <"$squash_msg"
> > > + else
> > > + count=2
> > > + echo '# This is a combination of 2 commits.'
> > > + echo '# The first commit message is:'
> > > + echo
> > > + commit_message HEAD
> > > + fi
> > > + echo
> > > + echo "# This is the $(nth_string "$count") commit message:"
> > > + echo
> > > + commit_message "$1"
> > > +}
> > > +
> > > +make_squash_message_multiple () {
> > > + echo '# This is a dummy to get the 0.' >"$squash_msg"
> > > + for cur_sha1 in $(git rev-list --reverse "$sha1..HEAD")
> > > + do
> > > + make_squash_message "$cur_sha1" >"$MSG"
> > > + cp "$MSG" "$squash_msg"
> > > + done
> > > +}
> >
> > Hmm, I know this is how rebase-i is written, but we should be able to do
> > better than writing and flipping temporary times N times, shouldn't we?
>
> Right, again my fault.
No, not your fault.
I think Junio is referring to make_squash_message_multiple which is for
squash --from <mark>
and just calls your make_squash_message several times.
When I did this, I knew that this is not most efficient, but I had in
mind: this is just a prototype. The builtin will solve this in a
different way.
Nonetheless, purely tested:
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -264,11 +264,26 @@ make_squash_message () {
}
make_squash_message_multiple () {
- echo '# This is a dummy to get the 0.' >"$squash_msg"
- for cur_sha1 in $(git rev-list --reverse "$sha1..HEAD")
+ revlist=$(git rev-list --reverse "$sha1..HEAD")
+ count=$(echo "$revlist" | wc -l)
+ squash_i=0
+ echo "# This is a combination of $count commits."
+ for cur_sha1 in $revlist
do
- make_squash_message "$cur_sha1" >"$MSG"
- cp "$MSG" "$squash_msg"
+ squash_i=$(($squash_i+1))
+ if test -f "$squash_msg"
+ then
+ count=$(($count + $(sed -n -e \
+ 's/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p' \
+ <"$squash_msg" | sed -n -e '$p')+1))
+ sed -e '1d' -e '2,/^./{
+ /^$/d
+ }' <"$squash_msg"
+ fi
+ echo
+ echo "# This is the $(nth_string "$squash_i") commit message:"
+ echo
+ commit_message "$cur_sha1"
done
}
@@ -1128,7 +1143,7 @@ insn_squash () {
else
if test -n "$from"
then
- make_squash_message_multiple "$sha1"
+ make_squash_message_multiple "$sha1" >"$MSG"
else
make_squash_message "$sha1" >"$MSG"
fi
###
Johannes Schindelin wrote:
> > > +peek_next_command () {
> > > + sed -n -e '1s/ .*$//p' <"$TODO"
> > > +}
> >
> > ... which could respond "the next command is '#' (comment)", so we are
> > actively counting a comment as a step here. Does this contradict with the
> > mental note we made earlier, and if so, does the discrepancy hurt us
> > somewhere in this program?
Yes it does hurt ;-)
> Yes, this is wrong. it must be
>
> sed -n -e '/^#/d' -e '1s .*$//p' < "$TODO"
Thanks ;)
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -273,7 +273,7 @@ make_squash_message_multiple () {
}
peek_next_command () {
- sed -n -e '1s/ .*$//p' <"$TODO"
+ sed -n -e '/^#/d' -e '1s/ .*$//p' <"$TODO"
}
# If $1 is a mark, make a ref from it; otherwise keep it.
###
Junio C Hamano wrote:
> > +# If $1 is a mark, make a ref from it; otherwise keep it
> > +mark_to_ref () {
> > + arg="$1"
> > + ref=$(expr "x$arg" : 'x:0*\([0-9][0-9]*\)$')
>
> You might want to leave comments to describe constraints that led to this
> slightly awkward regexp:
>
> * :0 is allowed
> * :01 is the same as :1
Right.
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -270,7 +270,10 @@ peek_next_command () {
sed -n -e '1s/ .*$//p' <"$TODO"
}
-# If $1 is a mark, make a ref from it; otherwise keep it
+# If $1 is a mark, make a ref from it; otherwise keep it.
+# Note on marks:
+# * :0 is allowed
+# * :01 is the same as :1
mark_to_ref () {
arg="$1"
ref=$(expr "x$arg" : 'x:0*\([0-9][0-9]*\)$')
###
Junio C Hamano wrote:
> > +strategy_check () {
> > + case "$1" in
> > + resolve|recursive|octopus|ours|subtree|theirs)
> > + return
> > + ;;
> > + esac
> > + todo_warn "Strategy '$1' not known."
> > +}
>
> Hmm. Do we need to maintain list of available strategies here and then in
> git-merge separately?
No, this is potentially-error-prone at large.
But I want a check for this, so I'd vote for a
--list-strategies
feature in the builtin-merge.
(Before writing the quoted code, I was looking for such a feature, but
haven't found. But since builtin-merge is in work I didn't want send
patches on git-merge.sh)
Johannes Schindelin wrote:
> I'd not check in sequencer for the strategy. Especially given that we
> want to support user-written strategies in the future.
I don't know how this is planned to look like, but perhaps --list-strategies
may make sense here, too.
Also, the shell completion scripts could use that.
Junio C Hamano wrote:
> > +### Author script functions
> > +
> > +clean_author_script () {
> > + cat "$ORIG_AUTHOR_SCRIPT" >"$AUTHOR_SCRIPT"
> > +}
> > +
> > +# Take "Name <e-mail>" in stdin and outputs author script
> > +make_author_script_from_string () {
> > + sed -e 's/^\(.*\) <\(.*\)>.*$/GIT_AUTHOR_NAME="\1"\
> > +GIT_AUTHOR_EMAIL="\2"\
> > +GIT_AUTHOR_DATE=/'
> > +}
>
> If you are going to "."-source or eval the output from this, you would
> need to quote the values a lot more robustly, wouldn't you? Is this safe
> against shell metacharacters in names, mails and/or space between unixtime
> and the timezone information?
Time is set empty here.
It is not save according to shell metacharacters in name or e-mail and I
knew that when writing.
Of course, a
First "nick" Sure
author will get problems here. ;-)
And as long as nobody is named $(rm -rf "$HOME") [1], I thought this is
sufficient for the prototype.
But...
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -520,7 +520,7 @@ clean_author_script () {
# Take "Name <e-mail>" in stdin and outputs author script
make_author_script_from_string () {
- sed -e 's/^\(.*\) <\(.*\)>.*$/GIT_AUTHOR_NAME="\1"\
+ sed -e 's/[$"\\]/\\&/g' -e 's/^\(.*\) <\(.*\)>.*$/GIT_AUTHOR_NAME="\1"\
GIT_AUTHOR_EMAIL="\2"\
GIT_AUTHOR_DATE=/'
}
@@ -779,7 +779,7 @@ insn_patch () {
if test -z "$AUTHOR"
then
- sed -n -e '
+ sed -e 's/[$"\\]/\\&/g' -n -e '
s/^Author: \(.*\)$/GIT_AUTHOR_NAME="\1"/p;
s/^Email: \(.*\)$/GIT_AUTHOR_EMAIL="\1"/p;
s/^Date: \(.*\)$/GIT_AUTHOR_DATE="\1"/p
###
Is escaping $, " and \ enough?
> > + if test -z "$AUTHOR"
> > + then
> > + sed -n -e '
> > + s/^Author: \(.*\)$/GIT_AUTHOR_NAME="\1"/p;
> > + s/^Email: \(.*\)$/GIT_AUTHOR_EMAIL="\1"/p;
> > + s/^Date: \(.*\)$/GIT_AUTHOR_DATE="\1"/p
> > + ' <"$infofile" >>"$AUTHOR_SCRIPT"
>
> The same comment on quoting applies here, I think.
Right.
> > + if test -n "$failed"
> > + then
> > + # XXX: This is just a stupid hack:
> > + with_author git apply $apply_opts --reject --index "$PATCH"
>
> Please don't do this without being asked, if you are planning to use this
> in "am" when 3-way fallback was not asked. It _may_ make sense to give an
> option to the users to ask for .rej if they prefer to work that way better
> than working with 3-way merge fallback, but doing this without being asked
> is not acceptable.
The --reject was just a mind marker for that what I actually think is
useful and less annoying than the current behavior:
> > + die_to_continue 'Patch failed. See the .rej files.'
> > + # XXX: We actually needed a git-apply flag that creates
> > + # conflict markers and sets the DIFF_STATUS_UNMERGED flag.
>
> That is what -3way is all about, and this codepath is when the user did
> not ask for it, isn't it?
Now imagine you apply a patch that cannot be applied 100% cleanly and
you don't have the 3-way base in the repo. You know what happens?
Yes, the patch is completly rejected, because apply is atomic.
And I think a git-apply option that results in a non-atomic behavior,
that creates conflict markers (and no .rej files), would be a great
usability feature for the "patch" insn in sequencer.
I'd even suggest to make that the default, but that's debatable.
Btw, I removed the mind marker:
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -836,9 +836,7 @@ insn_patch () {
if test -n "$failed"
then
- # XXX: This is just a stupid hack:
- with_author git apply $apply_opts --reject --index "$PATCH"
- die_to_continue 'Patch failed. See the .rej files.'
+ die_to_continue 'Patch failed.'
# XXX: We actually needed a git-apply flag that creates
# conflict markers and sets the DIFF_STATUS_UNMERGED flag.
fi
###
> > +# Check the "pick" instruction
> > +check_pick () {
> > + revert=
> > + mainline=
> > + while test $# -gt 1
> > + do
> > ...
> > + done
> > +
> > + if test -n "$mainline"
> > + then
> > + test -z "$revert" ||
> > + todo_error "Cannot use $revert together with --mainline."
>
> Why not? If you have this...
>
> ---A---C---D
> /
> ---B
>
> and you are at D, you may want to undo the merge you made at C and go back
> to either A or B, which essentially is same as cherry-picking diff between
> C and D on top of either A or B. Both are valid operations aren't they?
Jakub already asked. I told him, that git-revert doesn't know --mainline,
but it seems that this was wrong. So:
--- a/git-sequencer.sh
+++ b/git-sequencer.sh
@@ -879,13 +879,11 @@ $OPTIONS_GENERAL
# Check the "pick" instruction
check_pick () {
- revert=
mainline=
while test $# -gt 1
do
case "$1" in
-R)
- revert="$1"
;;
--mainline)
shift
@@ -913,9 +911,6 @@ check_pick () {
if test -n "$mainline"
then
- test -z "$revert" ||
- todo_error "Cannot use $revert together with --mainline."
-
parents=$(count_parents "$1")
test "$parents" -lt "$mainline" &&
todo_error "Commit has only $parents (less than $mainline) parents."
###
Regards and big thanks for the fast reply,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: Cherry picking instead of merges.
From: David Brown @ 2008-07-03 20:53 UTC (permalink / raw)
To: Avery Pennarun; +Cc: git
In-Reply-To: <32541b130807031315j3d9b7d77y277e3cb994ab0964@mail.gmail.com>
On Thu, Jul 03, 2008 at 04:15:22PM -0400, Avery Pennarun wrote:
>Unfortunately, since your mismerged branches are already published,
>rewriting history would cause a lot of pain for everyone. It would be
>better to avoid doing that entirely. However, I can see why you'd
>want to do that in order to make future git-bisect easier.
It's only sitting in a private developer's branch. I want to do the merge
properly, but I'm just trying to figure out how to get the conflict
resolution out of his work.
>Basically, if you're going to try to fix the git-bisect intermediate
>versions, you're going to have to rewrite history anyway; in which
>case, why not just make your developer's cherry-picked branch the
>official one? Then your problems are solved, other than getting all
>your developers onto the new history.
Once we start cherry picking the changes from Company B, we have a
different set of changes from them, and future merges will get harder and
harder.
>This will apply the correct conflict resolution to the tip of your
>newest branch. All the revisions between X and HEAD will still be
>broken, but that's usually better than trying to rewrite history and
>pretend the broken revisions never existed. You can always use "git
>bisect skip" for cases like that.
Except we already know that the broken change is inside of the broken
revisions.
It turns out that things are more messed up than I thought. This developer
had done a 'git push' with some manual refs and pushed what was supposed to
be a merge into an unmerged branch.
I've spoken with all of the developers who use this tree, and everyone
agrees that rewinding the tree is the best way to go. Now just time to
trudge forward and learn.
But, it makes for a good new rule: no cherry-picking other people's
changes.
David
^ permalink raw reply
* Re: finding deleted file names
From: Shawn O. Pearce @ 2008-07-03 20:32 UTC (permalink / raw)
To: Jeff King; +Cc: Geoff Russell, git
In-Reply-To: <20080703103658.GA11571@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Thu, Jul 03, 2008 at 09:31:55AM +0930, Geoff Russell wrote:
>
> > git diff --diff-filter=D --name-only HEAD@{'7 days ago'}
> >
> > finds files deleted during the last 7 days, but if my repository is
> > only 6 days old I get a
> > fatal error.
> >
> > fatal: bad object HEAD@{7 days ago}
>
> This should issue a warning, but use the oldest reflog entry. However,
> there is a slight problem with that. See below for details.
>
> Shawn, does this seem sane to you?
Yea, this is the only reasonable thing to do here. Ack.
--
Shawn.
^ permalink raw reply
* Re: [RFC/PATCH (WIP)] Git.pm: Add get_config() method and related subroutines
From: Lea Wiemann @ 2008-07-03 20:30 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Petr Baudis
In-Reply-To: <200807031824.55958.jnareb@gmail.com>
Jakub Narebski wrote:
> Add get_config([PREFIX]) method [...]
>
> I hope I am not repeating [Lea's] work.
No, you're not. (You could've checked my repo at
<http://repo.or.cz/w/git/gitweb-caching.git> ;-).)
FWIW, I don't think it'll make much of a difference for gitweb, since
the 'git config -l' output is cached anyway, but it's good someone's
extracting this. Do you have any user for that function besides gitweb?
> * Should config_val_to_bool and config_val_to_int throw error or
> just return 'undef' on invalid values?
I suspect that if you have, say, command line tools, throwing an error
is better UI behavior than silently ignoring the entry. And developers
can always catch errors if they want to.
> * Is "return wantarray ? %config : \%config;" DWIM-mery good style?
Gitweb uses it as well, and it seems reasonable IMVHO.
> * Should ->get_config() use ->command_output_pipe, or simpler
> ->command() method, reading whole config into array?
Does it make a difference? If you're worried about performance, config
files are so short that it won't matter; use the easier path.
> * What should ->get_config() method be named? ->get_config()
> or perhaps ->config_hash(), or ->config_hashref()?
Regarding the method naming, how about making this an object oriented
interface? Bless the hash, and allow calls like
$config_hash->get('key'). I'm not sure how to name the constructor, but
if you can wait a week or so, you could maybe integrate this into the
Git::Repo interface (under Git/Config.pm), so you'd end up with ...
Git::Repo->new(directory => 'repo.git')->config->get('key')
... where config() transparently parses the config file if it hasn't
been read already. (The Git.pm API admittedly seems a little messy --
I'll post about that later -- so adding it to Git::Repo might be better
indeed.)
> * What should ->get_config() have as an optional parameter:
> PREFIX (/^$prefix/o), or simply SECTION (/^(?:$section)\./o)?
Off the top of my head, I don't see much need for a prefix parameter, so
I'd go for 'section'.
I haven't been able to answer all of the questions, but I hope this helps.
-- Lea
^ permalink raw reply
* Re: "make test" works again (sort-of) on cygwin.
From: Alex Riesen @ 2008-07-03 20:26 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list, Junio C Hamano
In-Reply-To: <486D0FFC.5090308@ramsay1.demon.co.uk>
Ramsay Jones, Thu, Jul 03, 2008 19:44:28 +0200:
> I spent many hours (not recently) trying to determine the reason for
> the crash, but it seems to be essentially "random gremlins" :-)
> However, since I mostly use Linux, I've not been very motivated to
> find a solution, particularly when git works fine (touch wood) when
> used for real work. (i.e. only "make test" crashes my machine).
> Also, as Alex and Shawn have not reported problems, I have assumed
> it is something specific to my environment. Dunno.
I haven't tried to run the test on XP recently (it is a production
workstation, after all), but the last time I risked it was locked up
hard. Win2k runs (and I even can work, well, browse, on it)
> Anyhow, the "sort-of" in the subject line, relates to the fact that
> I am seeing some test failures. In particular, all tests in
> t0004-unwritable.sh and tests 21->24 in t3700-add.sh. All of these
> tests involve chmod/permissions ...
Don't run "make test" as root (or "backup operator" on windows).
OTOH, a windows machine is almost useless, unless you're a member of
local administrators group (which includes "backup" permission).
^ permalink raw reply
* Re: Cherry picking instead of merges.
From: Avery Pennarun @ 2008-07-03 20:15 UTC (permalink / raw)
To: git
In-Reply-To: <20080703182650.GA11166@old.davidb.org>
On 7/3/08, David Brown <git@davidb.org> wrote:
> First we tried a git-merge and resolved the conflicts. The problem here is
> that the resultant code didn't work. git-bisect wasn't very useful because
> the intermediate versions don't have resolved conflicts.
>
> Yesterday, one developer cherry picked company B's changes into a branch.
> It appears he resolved the conflicts for each commit, which should make
> bisecting easier.
>
> The problem is that we now have very divergent history.
>
> Any advice on how to make use of how he resolved conflicts in order to
> merge company B's changes in using git-merge. [...]
Unfortunately, since your mismerged branches are already published,
rewriting history would cause a lot of pain for everyone. It would be
better to avoid doing that entirely. However, I can see why you'd
want to do that in order to make future git-bisect easier.
Basically, if you're going to try to fix the git-bisect intermediate
versions, you're going to have to rewrite history anyway; in which
case, why not just make your developer's cherry-picked branch the
official one? Then your problems are solved, other than getting all
your developers onto the new history.
Alternatively, if you just want to fix your main development tree so
that it's "correct", then you could do this:
- Go to a point in time where both branches (main and developer) have
exactly the same set of patches: that is, your company + company B.
The exact history (ie. commit ids) will look different, because the
two branches took different paths to get there, but the code *should*
have been identical at those two times, since you have the same set of
patches. Call the two points X and Y.
- X (on the main branch) is actually different from Y (on your
developer's branch) because someone mis-resolved the conflicts on X.
However, the only difference between X and Y should be the mis-merge.
Thus:
- git diff X..Y >fix-mis-merge.patch
- git checkout HEAD
- git apply fix-mis-merge.patch
This will apply the correct conflict resolution to the tip of your
newest branch. All the revisions between X and HEAD will still be
broken, but that's usually better than trying to rewrite history and
pretend the broken revisions never existed. You can always use "git
bisect skip" for cases like that.
Have fun,
Avery
^ permalink raw reply
* Re: Cherry picking instead of merges.
From: Alex Riesen @ 2008-07-03 20:13 UTC (permalink / raw)
To: git
In-Reply-To: <20080703182650.GA11166@old.davidb.org>
David Brown, Thu, Jul 03, 2008 20:26:50 +0200:
> Yesterday, one developer cherry picked company B's changes into a branch.
> It appears he resolved the conflicts for each commit, which should make
> bisecting easier.
>
> The problem is that we now have very divergent history.
...and have absolutely no idea on what were the changes of the company
B were made.
> Any advice on how to make use of how he resolved conflicts in order to
> merge company B's changes in using git-merge. ...
Let that developer do the merge next time? Or let the B do a merge
with a commonly accepted base?
^ permalink raw reply
* Re: about c8af1de9 (git status uses pager)
From: Wincent Colaiuta @ 2008-07-03 20:10 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, Tim Stoakes, Junio C Hamano, git
In-Reply-To: <20080703190829.GA18205@sigill.intra.peff.net>
El 3/7/2008, a las 21:08, Jeff King escribió:
> On Thu, Jul 03, 2008 at 03:37:30PM +0200, Wincent Colaiuta wrote:
>
>>>> This [changing git status to use the pager automatically] was
>>>> quite a
>>>> nasty change to sneak on people I think.
>>>
>>> Well, I think that a command producing pages and pages of output
>>> without stopping is useless. Therefore, _I_ maintain that it makes
>>> tons of sense.
>>
>> Ditto. For me, the change was very welcome. Sure, usually the status
>> output is short, but on those occasions where it isn't (big file
>> reorganizations etc) I was quite sick of doing the "git status, oops
>> that output was too long let's try again, git -p status" dance. It
>> was
>> also incongruous with my very first experiences with Git, where I had
>> been pleasantly surprised that "git log" automatically invoked the
>> pager whereas "svn log" just spewed output into my console until ^C.
>
> I think there is not much point in discussing whether "git status"
> should use a pager. It is obvious at this point that some people love
> it, and some people despise it, and it seems to be a matter of
> personal
> preference. The only sane way forward seems to be configurable paging.
I completely agree. I just don't like it when people use words like
"nasty" and "sneak" to describe a change that was discussed in one
short and one long thread prior to being incorporated. But you're
right; there's no sense in discussing the "whether" any more.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH/v2] git-basis, a script to manage bases for git-bundle
From: Jeff King @ 2008-07-03 19:59 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Adam Brewster, git
In-Reply-To: <200807021144.46423.jnareb@gmail.com>
On Wed, Jul 02, 2008 at 11:44:45AM +0200, Jakub Narebski wrote:
> Well, there is one situation where either separate git-bases program
> (which is a good start; it can be named git-bundle--bases; there are
> some precedents for that ;-)), or allowing to create 'bases' file
> without creating bundle would be good to have. Namely situation
> where two computers are _sometimes off-line (disconnected)_. If you
> want to transfer new commits from machine B to machine A, you would
> generate 'bases' file on machine A, then transfer this file using some
> off-line medium, then generate bundle on machine B using those bases,
> etc.
Yes, certainly it is more flexible to have them split. I find Adam's
argument the most compelling, though. Think about moving commits as a
multi-step protocol:
1. Local -> Remote: Here are some new commits, basis..current
2. Remote -> Local: OK, I am now at current.
3. Local: update basis to current
git-push has the luxury of asking for "basis" each time, so we know it
is correct. But with bundles, we can't do that. And failing to update
"basis" means we will send some extra commits next time. But updating
"basis" when we shouldn't means that the next bundle will be broken.
So I think even if people _do_ want to update "basis" when they create
the bundle (because it is more convenient, and they are willing to
accept the possibility of losing sync), it is trivial to create that
workflow on top of the separate components. But I can see why somebody
might prefer the separate components, and it is hard to create them if
the feature is lumped into "git-bundle" (meaning in such a way that you
cannot perform the steps separately; obviously git-bundle --basis would
be equivalent).
But I am not a bundle user, so that is just my outsider perspective.
-Peff
^ permalink raw reply
* Re: finding deleted file names
From: Jeff King @ 2008-07-03 19:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Geoff Russell, Shawn O. Pearce, git
In-Reply-To: <7vzloyzrew.fsf@gitster.siamese.dyndns.org>
On Thu, Jul 03, 2008 at 12:41:43PM -0700, Junio C Hamano wrote:
> Once reflog entries begin expiring, we won't have the null_sha1 on the
> previous side of them, and after that we will keep returning the
> oldest-known one, so nobody should be expecting to find out when a ref was
> first created in this repository by checking with what timestamp that
> syntax breaks in the existing interface.
Right, that was my thought: though perhaps not technically as precise,
it is much more DWIM.
My other concern, which I failed to voice in the original mail was:
should we do this magic for other reflog entries besides the first one?
It is possible to end up with a null sha1 in any other entry? Branch
creation shouldn't do it, because we will have deleted any previous
reflog when killing off the branch before, so we always end up putting
in the "oldest" reflog entry. Cloning shouldn't do it for obvious
reasons.
> Sorry for not being Shawn ;-)
Heh.
-Peff
^ permalink raw reply
* Re: finding deleted file names
From: Junio C Hamano @ 2008-07-03 19:41 UTC (permalink / raw)
To: Jeff King; +Cc: Geoff Russell, Shawn O. Pearce, git
In-Reply-To: <20080703103658.GA11571@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Jul 03, 2008 at 09:31:55AM +0930, Geoff Russell wrote:
>
>> git diff --diff-filter=D --name-only HEAD@{'7 days ago'}
>>
>> finds files deleted during the last 7 days, but if my repository is
>> only 6 days old I get a
>> fatal error.
>>
>> fatal: bad object HEAD@{7 days ago}
>
> This should issue a warning, but use the oldest reflog entry. However,
> there is a slight problem with that. See below for details.
I think your patch is a sane thing to do.
Once reflog entries begin expiring, we won't have the null_sha1 on the
previous side of them, and after that we will keep returning the
oldest-known one, so nobody should be expecting to find out when a ref was
first created in this repository by checking with what timestamp that
syntax breaks in the existing interface.
Sorry for not being Shawn ;-)
^ permalink raw reply
* Re: [BUG] Git looks for repository in wrong directory
From: Junio C Hamano @ 2008-07-03 19:28 UTC (permalink / raw)
To: David ‘Bombe’ Roden; +Cc: git
In-Reply-To: <200807030305.17767.bombe@pterodactylus.net>
David ‘Bombe’ Roden <bombe@pterodactylus.net> writes:
>> If you have both, you already have found the way to disambiguate ;-)
>
> Yes, _now_ I know. In my opinion it’s very unintuitive and should be changed.
I'm of mixed mind about this.
I am guessing that the original motivation was that people can have
unversioned "project/" that perhaps is an extract from the tarball and
"project.git/" that is version-controlled by git, and when they want to
talk about the repository they can say either "project.git" and "project"
to get to the git-managed one, even if "project/" directory exists.
It may be an improvement if we checked to see if the original name does
refer to a git repository and use that without falling back. Even though
I do not personally care so deeply about this, I won't be opposed to such
an improvement.
^ permalink raw reply
* Re: [PATCH 0/3] Making remote tracking statistics available to other tools
From: Olivier Marin @ 2008-07-03 19:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ingo Molnar, Bruce Stephens, git
In-Reply-To: <7v8wwi23dr.fsf@gitster.siamese.dyndns.org>
Junio C Hamano a écrit :
>
> remote.c | 4 +++
> revision.h | 1 +
> t/t6040-tracking-info.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 57 insertions(+), 0 deletions(-)
Tested: it works. Thanks.
Olivier.
^ permalink raw reply
* Re: [PATCH 2/3] git-add--interactive: remove hunk coalescing
From: Thomas Rast @ 2008-07-03 19:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vod5f7w3f.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 797 bytes --]
Junio C Hamano wrote:
>
> > Blindly concatenating the above two and feeding them to "git apply" *may*
> > happen to work by accident, not by design. This very much feels like a
> > hack of "This works most of the time for me, your mileage may vary" kind,
> > which we would want to avoid when we can.
>
> Well, I changed my mind. Let's run with this and see what happens.
In support of this being a feature of git-apply, notice that it even
handles the situation correctly where the context of a hunk has been
influenced by previous hunks, as in
@@ -1,2 +1,3 @@
foo
+quux
bar
@@ -1,3 +1,4 @@
foo
quux
+abc
bar
With Don Zickus' recent patch, it also handles patches that go over
the same file twice.
- Thomas
--
Thomas Rast
trast@student.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: about c8af1de9 (git status uses pager)
From: Jeff King @ 2008-07-03 19:08 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Johannes Schindelin, Tim Stoakes, Junio C Hamano, git
In-Reply-To: <08353871-5C94-4E97-9589-DC3980C47CD4@wincent.com>
On Thu, Jul 03, 2008 at 03:37:30PM +0200, Wincent Colaiuta wrote:
>>> This [changing git status to use the pager automatically] was quite a
>>> nasty change to sneak on people I think.
>>
>> Well, I think that a command producing pages and pages of output
>> without stopping is useless. Therefore, _I_ maintain that it makes
>> tons of sense.
>
> Ditto. For me, the change was very welcome. Sure, usually the status
> output is short, but on those occasions where it isn't (big file
> reorganizations etc) I was quite sick of doing the "git status, oops
> that output was too long let's try again, git -p status" dance. It was
> also incongruous with my very first experiences with Git, where I had
> been pleasantly surprised that "git log" automatically invoked the
> pager whereas "svn log" just spewed output into my console until ^C.
I think there is not much point in discussing whether "git status"
should use a pager. It is obvious at this point that some people love
it, and some people despise it, and it seems to be a matter of personal
preference. The only sane way forward seems to be configurable paging.
So the next steps are:
- getting a configurable paging patch that is good enough; I just
posted another trial, but there are still a few caveats. I would
love to hear commentary on whether people find it acceptable.
- once there is a patch, there is still the matter of "on by default,
make people configure off" versus "off by default, make people
configure on". And then you can make arguments about how the default
changes the new user experience, but hopefully there will be a
little less vehemence on both sides, since it is "here's what new
users should see" and not "here's what I am forced to see every
day".
-Peff
^ permalink raw reply
* Re: [PATCH 0/3] Making remote tracking statistics available to other tools
From: Junio C Hamano @ 2008-07-03 19:07 UTC (permalink / raw)
To: Olivier Marin; +Cc: Ingo Molnar, Bruce Stephens, git
In-Reply-To: <486CFDA3.9070704@free.fr>
Olivier Marin <dkr+ml.git@free.fr> writes:
> Junio C Hamano a écrit :
>> The one I sent out was a bit hacky as the existing implementation inside
>> git-checkout was not designed to be cleanly reusable.
>>
>> Here is a cleaned up series that could be applied.
>>
>> [PATCH 1/3] Refactor "tracking statistics" code used by "git checkout"
>> [PATCH 2/3] git-status: show the remote tracking statistics
>> [PATCH 3/3] git-branch -v: show the remote tracking statistics
>
> Sorry, but the third patch does not work for me. FWIU, the problem comes from
> the revision walking code that is not reentrant because of the object cache.
> IOW, calling stats_tracking_info() more than once does not work: objects
> states changed after first call.
>
> I do not understand the code enough yet and I failed to fix it. The only way
> I found is by clearing obj_hash (object.c) but this seems the wrong thing to
> do.
>
> Ideas?
Thanks.
remote.c | 4 +++
revision.h | 1 +
t/t6040-tracking-info.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/remote.c b/remote.c
index bd5c3be..df8bd72 100644
--- a/remote.c
+++ b/remote.c
@@ -1295,6 +1295,10 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
else
(*num_theirs)++;
}
+
+ /* clear object flags smudged by the above traversal */
+ clear_commit_marks(ours, ALL_REV_FLAGS);
+ clear_commit_marks(theirs, ALL_REV_FLAGS);
return 1;
}
diff --git a/revision.h b/revision.h
index abce500..e8bac6d 100644
--- a/revision.h
+++ b/revision.h
@@ -11,6 +11,7 @@
#define ADDED (1u<<7) /* Parents already parsed and added? */
#define SYMMETRIC_LEFT (1u<<8)
#define TOPOSORT (1u<<9) /* In the active toposort list.. */
+#define ALL_REV_FLAGS ((1u<<10)-1)
struct rev_info;
struct log_info;
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
new file mode 100755
index 0000000..960db2d
--- /dev/null
+++ b/t/t6040-tracking-info.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='remote tracking stats'
+
+. ./test-lib.sh
+
+advance () {
+ echo "$1" >"$1" &&
+ git add "$1" &&
+ test_tick &&
+ git commit -m "$1"
+}
+
+test_expect_success setup '
+ for i in a b c;
+ do
+ advance $i || break
+ done &&
+ git clone . test &&
+ (
+ cd test &&
+ git checkout -b b1 origin &&
+ git reset --hard HEAD^ &&
+ advance d &&
+ git checkout -b b2 origin &&
+ git reset --hard b1 &&
+ git checkout -b b3 origin &&
+ git reset --hard HEAD^ &&
+ git checkout -b b4 origin &&
+ advance e &&
+ advance f
+ )
+'
+
+script='s/^..\(b.\)[ 0-9a-f]*\[\([^]]*\)\].*/\1 \2/p'
+cat >expect <<\EOF
+b1 ahead 1, behind 1
+b2 ahead 1, behind 1
+b3 behind 1
+b4 ahead 2
+EOF
+
+test_expect_success 'branch -v' '
+ (
+ cd test &&
+ git branch -v
+ ) |
+ sed -n -e "$script" >actual &&
+ test_cmp expect actual
+'
+
+test_done
^ permalink raw reply related
* [BUG] git-svn failure with python repository
From: Gabriel @ 2008-07-03 19:00 UTC (permalink / raw)
To: git
I got an error importing the python repository with git-svn.
The error could be due to a funky branch name.
git --version
git version 1.5.6.1.89.gd5446
LANG= git svn clone -s http://svn.python.org/projects/python
…
r5002 = 8aa67da1bc987c02661b5331eae7c349be2da0ef (trunk)
Found possible branch point: http://svn.python.org/projects/python/trunk => http://svn.python.org/projects/python/tags/Beta_14-Mar-1995-#2, 5002
Found branch parent: (tags/Beta_14-Mar-1995-#2) 8aa67da1bc987c02661b5331eae7c349be2da0ef
Following parent with do_switch
Invalid filesystem path syntax: REPORT request failed on '/projects/!svn/vcc/default': Target path does not exist at /home/g2p/bin/git-svn line 3892
^ permalink raw reply
* Re: ':/<oneline prefix>' notation doesn't support full file syntax
From: Dana How @ 2008-07-03 18:27 UTC (permalink / raw)
To: Eric Raible; +Cc: Junio C Hamano, git, Johannes.Schindelin, danahow
In-Reply-To: <279b37b20807030150t2e9cbcc8wf099a5872568af8@mail.gmail.com>
On Thu, Jul 3, 2008 at 1:50 AM, Eric Raible <raible@gmail.com> wrote:
> On Thu, Jul 3, 2008 at 1:34 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> "Eric Raible" <raible@gmail.com> writes:
>>
>> Is there anything to fix? In that example, you are looking for a commit
>> that talks about "object name:sha1_name.c" in the comment.
>
> Yes. What if I'm looking for specific file (i.e. sha1_name.c) in the commit
> described by ":/object name:", just like I can do with 28a4d9404:sha1_name.c?
>
> This is not ambiguous if we first consider the entire string as the prefix.
> If that fails we look for a filename after the final ':'.
In part you are proposing this because it is a consistent extension.
But a problem with the current :/string is that it adds 2nd meanings
to both : and / ,
which is not all that consistent to start with.
Last year Junio proposed that :/ be changed to ?
to eliminate the overloading; thus your proposal becomes:
?string:filename
He chose ? because it results in a search backwards through commits.
(You could make that ?string?:filename if you prefer, where the 2nd ?
is only needed if you include a filename.)
I was surprised to see Dscho advocating removing this feature altogether.
Others proposed other command sequences which avoided :/ .
If :/ is now going to be extended and thus perhaps more likely to
appear in scripts,
is now the time to change it to ? which has no other special meaning to git?
Thanks,
--
Dana L. How danahow@gmail.com +1 650 804 5991 cell
^ permalink raw reply
* Cherry picking instead of merges.
From: David Brown @ 2008-07-03 18:26 UTC (permalink / raw)
To: git
We have a branch coming from "company B" who is also doing development on
the same tree as us. We would like to do regular merges with them, but
unfortunately, there are quite a few changes in between.
First we tried a git-merge and resolved the conflicts. The problem here is
that the resultant code didn't work. git-bisect wasn't very useful because
the intermediate versions don't have resolved conflicts.
Yesterday, one developer cherry picked company B's changes into a branch.
It appears he resolved the conflicts for each commit, which should make
bisecting easier.
The problem is that we now have very divergent history.
Any advice on how to make use of how he resolved conflicts in order to
merge company B's changes in using git-merge. I could always use the
"ours" strategy to just kind of force the merge, so at least future merges
would work, but it'd kind of be nice to have the proper history.
There are about 110 commits in question.
Thanks,
David Brown
^ permalink raw reply
* Re: [PATCH 3/7] Documentation: complicate example of "man git-command"
From: J. Bruce Fields @ 2008-07-03 18:18 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, git, Christian Couder, Nguyen Thai Ngoc Duy,
Jon Loeliger
In-Reply-To: <Pine.GSO.4.62.0807022010280.10323@harper.uchicago.edu>
On Wed, Jul 02, 2008 at 08:45:59PM -0500, Jonathan Nieder wrote:
> J. Bruce Fields wrote:
>
> > On Tue, Jul 01, 2008 at 04:54:53PM -0700, Junio C Hamano wrote:
> >
> >> We would want to mention the typesetting convention early in the manuals
> >> (git(7), gittutorial(7) and user-manual.html) as well, so how about...
> >>
> >> Conventions used in this document
> >> ---------------------------------
> >>
> >> When talking about a git subcommand 'cmd', this documentation
> >> typesets the name of it like 'git-cmd', and that is the name you
> >> ask for its manual page.
> >>
> >> Examples are typeset like this: `$ git cmd` (`$` is your command
> >> prompt, do not actually type it to your shell). Note that a
> >> subcommand is specified as the first parameter to the 'git'
> >> program when you actually run it from the command line.
> >
> > I'm not convinced this last sentence is necessary.
>
> I agree, but I think it doesn't hurt. I think the point was to
> establish the word and concept "subcommand".
We don't need to define it. (The word "subcommand" is pretty intuitive,
especially for anyone with some commandline experience, which we do
assume throughout.)
> > > [example showing typographical conventions]
> >
> > Typographical conventions shouldn't need so much explanation.
>
> Yes, I suppose. I'm used to printed manuals having a page on
> the meaning of different typefaces inside, but that's a bit
> of a different situation.
Yes.
> > I'm curious: Jonathan, was this the original patch the result of a
> > real-life instance of confusion? What happened?
>
> No, I'm actually a bit ashamed to have sent the patch... I was just
> changing `git subcommand` to `git-subcommand` wherever it was the name
> of a command, rather than the command line to run it, that was in
> question. Consistency would have made the old example awkward, so I
> looked around for alternatives.
That being the case, I'd rather leave the text as is; I'm uncomfortable
adding new text to address something that isn't in practice a problem.
--b.
>
> Why worry about whether the man pages have no consistent rule about
> dashes? Since it is not obvious why the man pages use the dashed form
> when they do, I think a fraction of people will naturally use the
> dashed form by default. That means trouble once Git 1.6.0 comes out
> (e.g. see Ingo's recent post
> <http://thread.gmane.org/gmane.comp.version-control.git/87012/focus=87020>).
>
> Here's a patch implementing Junio's suggestion, because I do like it.
> Please let me know what you think (especially ideas for making it
> shorter).
>
> Thanks for all your thoughts so far. Sorry I took so long to get back.
>
> --- %< --- %< --- %< ----
> Subject: gittutorial(7): add "Conventions used in this document" section
>
> The manual page for the git subcommand invoked as "git clone" is
> named git-clone(1), and similarly for the rest of the git
> subcommands. This patch should make the convention a little
> clearer when it is introduced at the beginning of gittutorial(7).
>
> Thanks to Junio C Hamano for the idea and wording.
>
> It remains to make an analogous change for user-manual.html
> and maybe git(1).
>
> Signed-off-by: Jonathan Nieder <jrnieder@uchicago.edu>
> ---
> Documentation/gittutorial.txt | 35 ++++++++++++++++++++++++++++++-----
> 1 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/gittutorial.txt
> b/Documentation/gittutorial.txt
> index 036a27c..51ad814 100644
> --- a/Documentation/gittutorial.txt
> +++ b/Documentation/gittutorial.txt
> @@ -19,12 +19,37 @@ If you are instead primarily interested in using
> git to fetch a project,
> for example, to test the latest version, you may prefer to start with
> the first two chapters of link:user-manual.html[The Git User's Manual].
>
> -First, note that you can get documentation for a command such as
> -`git log --graph` with:
> +Conventions used in this document
> +---------------------------------
>
> -------------------------------------------------
> -$ man git-log
> -------------------------------------------------
> +When discussing a git subcommand 'cmd', this documentation
> +typesets the name of it like 'git-cmd', and that is the name you
> +ask for its manual page by.
> +
> +Examples are typeset like this: `$ git cmd`. (`$` is your command
> +prompt; do not actually type it to your shell.) A subcommand
> +is specified as the first parameter to the 'git' program
> +when you actually run it from the command line.
> +
> +So a typical command description may go like this:
> +
> +To propagate the changes you made back to the original subversion
> +repository, you would use the 'git-svn dcommit' command. It does
> +these things (long description here). Some examples:
> +
> +------------
> +$ ... some example command sequence ...
> +$ git svn dcommit
> +------------
> +
> +For full details, type:
> +
> +------------
> +$ man git-svn
> +------------
> +
> +Introducing yourself to git
> +---------------------------
>
> It is a good idea to introduce yourself to git with your name and
> public email address before doing any operation. The easiest
> --
> 1.5.5.GIT
>
^ permalink raw reply
* "make test" works again (sort-of) on cygwin.
From: Ramsay Jones @ 2008-07-03 17:44 UTC (permalink / raw)
To: GIT Mailing-list; +Cc: Junio C Hamano
Hi *,
Having upgraded to version 1.5.6 (via tarball), I can "make test" once
again, without crashing my machine. (well, it grinds to a halt, so that
I have to pull the power cord, followed by the battery!) I have been
unable to do that since v1.5.3!
I spent many hours (not recently) trying to determine the reason for
the crash, but it seems to be essentially "random gremlins" :-)
However, since I mostly use Linux, I've not been very motivated to
find a solution, particularly when git works fine (touch wood) when
used for real work. (i.e. only "make test" crashes my machine).
Also, as Alex and Shawn have not reported problems, I have assumed
it is something specific to my environment. Dunno.
Anyhow, the "sort-of" in the subject line, relates to the fact that
I am seeing some test failures. In particular, all tests in
t0004-unwritable.sh and tests 21->24 in t3700-add.sh. All of these
tests involve chmod/permissions ...
I haven't spent too long trying to debug this, but looking at the
first (actually second) test in t0004, I did an strace on git-write-tree.
After wading through the output, it seems that cygwin allows mkdir()
to succeed in the (un-writable) .git/object directory; eg. when
writing the tree object 47d832... it succeeds in making the 47 directory
(which does have write permission) followed by the d832... file.
So I tried this by hand:
$ mkdir obj
$ chmod a-w obj
$ echo file >obj/file
bash: obj/file: Permission denied
$ mkdir obj/dir
$
OK.
[Junio, in the tarball, the test files t5304-prune.sh and
t7610-mergetool.sh do not have the executable bit set]
ATB,
Ramsay Jones
^ permalink raw reply
* [PATCH/RFC] Fix some warnings (on cygwin) to allow -Werror
From: Ramsay Jones @ 2008-07-03 15:52 UTC (permalink / raw)
To: GIT Mailing-list; +Cc: Junio C Hamano
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Despite the subject line, this should be useful on any system for
which uint32_t is defined to be unsigned long rather than
unsigned int. (and where the return type of htonl() is similarly
defined).
Note that this was built on the v1.5.6 tarball, so will probably
not apply cleanly to the git repo! (hey, it's been more than a
few days). Hence, the RFC in the subject line. (is there a better
TLA for "so old it won't apply" ;-)
Also, note that git-compat-compat-util.h contains an #ifdef'ed
definition of PRIuMAX despite an unconditional #include of
inttypes.h; this seems a bit odd. This seems to imply a system
with a pre-standard header file, missing the PRIuMAX #define.
If so, then similar definitions for PRIu32 and PRIx32 may need
to be added.
ATB,
Ramsay
builtin-fast-export.c | 4 ++--
builtin-fetch-pack.c | 3 ++-
builtin-pack-objects.c | 15 ++++++++++-----
builtin-unpack-objects.c | 3 ++-
index-pack.c | 6 ++++--
pack-check.c | 4 ++--
receive-pack.c | 3 ++-
sha1_file.c | 12 ++++++------
show-index.c | 3 ++-
9 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index d0a462f..3500ff0 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -102,7 +102,7 @@ static void handle_object(const unsigned char *sha1)
mark_object(object);
- printf("blob\nmark :%d\ndata %lu\n", last_idnum, size);
+ printf("blob\nmark :%"PRIu32"\ndata %lu\n", last_idnum, size);
if (size && fwrite(buf, size, 1, stdout) != 1)
die ("Could not write blob %s", sha1_to_hex(sha1));
printf("\n");
@@ -190,7 +190,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
reencoded = reencode_string(message, "UTF-8", encoding);
if (!commit->parents)
printf("reset %s\n", (const char*)commit->util);
- printf("commit %s\nmark :%d\n%.*s\n%.*s\ndata %u\n%s",
+ printf("commit %s\nmark :%"PRIu32"\n%.*s\n%.*s\ndata %u\n%s",
(const char *)commit->util, last_idnum,
(int)(author_end - author), author,
(int)(committer_end - committer), committer,
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index de1e8d1..e094e2d 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -519,7 +519,8 @@ static int get_pack(int xd[2], char **pack_lockfile)
if (read_pack_header(demux.out, &header))
die("protocol error: bad pack header");
- snprintf(hdr_arg, sizeof(hdr_arg), "--pack_header=%u,%u",
+ snprintf(hdr_arg, sizeof(hdr_arg),
+ "--pack_header=%"PRIu32",%"PRIu32,
ntohl(header.hdr_version), ntohl(header.hdr_entries));
if (ntohl(header.hdr_entries) < unpack_limit)
do_keep = 0;
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 447d492..5e97355 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -590,7 +590,8 @@ static void write_pack_file(void)
free(written_list);
stop_progress(&progress_state);
if (written != nr_result)
- die("wrote %u objects while expecting %u", written, nr_result);
+ die("wrote %"PRIu32" objects while expecting %"PRIu32,
+ written, nr_result);
/*
* We have scanned through [0 ... i). Since we have written
* the correct number of objects, the remaining [i ... nr_objects)
@@ -602,7 +603,8 @@ static void write_pack_file(void)
j += !e->idx.offset && !e->preferred_base;
}
if (j)
- die("wrote %u objects as expected but %u unwritten", written, j);
+ die("wrote %"PRIu32" objects as expected but %"PRIu32
+ " unwritten", written, j);
}
static int locate_object_entry_hash(const unsigned char *sha1)
@@ -1718,7 +1720,8 @@ static int add_ref_tag(const char *path, const unsigned char *sha1, int flag, vo
static void prepare_pack(int window, int depth)
{
struct object_entry **delta_list;
- uint32_t i, n, nr_deltas;
+ uint32_t i, nr_deltas;
+ unsigned n;
get_object_details();
@@ -1809,7 +1812,8 @@ static int git_pack_config(const char *k, const char *v, void *cb)
if (!strcmp(k, "pack.indexversion")) {
pack_idx_default_version = git_config_int(k, v);
if (pack_idx_default_version > 2)
- die("bad pack.indexversion=%d", pack_idx_default_version);
+ die("bad pack.indexversion=%"PRIu32,
+ pack_idx_default_version);
return 0;
}
if (!strcmp(k, "pack.packsizelimit")) {
@@ -2243,7 +2247,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
prepare_pack(window, depth);
write_pack_file();
if (progress)
- fprintf(stderr, "Total %u (delta %u), reused %u (delta %u)\n",
+ fprintf(stderr, "Total %"PRIu32" (delta %"PRIu32"),"
+ " reused %"PRIu32" (delta %"PRIu32")\n",
written, written_delta, reused, reused_delta);
return 0;
}
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 85043d1..a891866 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -471,7 +471,8 @@ static void unpack_all(void)
if (ntohl(hdr->hdr_signature) != PACK_SIGNATURE)
die("bad pack file");
if (!pack_version_ok(hdr->hdr_version))
- die("unknown pack file version %d", ntohl(hdr->hdr_version));
+ die("unknown pack file version %"PRIu32,
+ ntohl(hdr->hdr_version));
use(sizeof(struct pack_header));
if (!quiet)
diff --git a/index-pack.c b/index-pack.c
index 5ac91ba..25db5db 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -190,7 +190,8 @@ static void parse_pack_header(void)
if (hdr->hdr_signature != htonl(PACK_SIGNATURE))
die("pack signature mismatch");
if (!pack_version_ok(hdr->hdr_version))
- die("pack version %d unsupported", ntohl(hdr->hdr_version));
+ die("pack version %"PRIu32" unsupported",
+ ntohl(hdr->hdr_version));
nr_objects = ntohl(hdr->hdr_entries);
use(sizeof(struct pack_header));
@@ -771,7 +772,8 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)
if (!strcmp(k, "pack.indexversion")) {
pack_idx_default_version = git_config_int(k, v);
if (pack_idx_default_version > 2)
- die("bad pack.indexversion=%d", pack_idx_default_version);
+ die("bad pack.indexversion=%"PRIu32,
+ pack_idx_default_version);
return 0;
}
return git_default_config(k, v, cb);
diff --git a/pack-check.c b/pack-check.c
index f489873..98a6d56 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -146,11 +146,11 @@ static void show_pack_info(struct packed_git *p)
for (i = 0; i <= MAX_CHAIN; i++) {
if (!chain_histogram[i])
continue;
- printf("chain length = %d: %d object%s\n", i,
+ printf("chain length = %"PRIu32": %"PRIu32" object%s\n", i,
chain_histogram[i], chain_histogram[i] > 1 ? "s" : "");
}
if (chain_histogram[0])
- printf("chain length > %d: %d object%s\n", MAX_CHAIN,
+ printf("chain length > %d: %"PRIu32" object%s\n", MAX_CHAIN,
chain_histogram[0], chain_histogram[0] > 1 ? "s" : "");
}
diff --git a/receive-pack.c b/receive-pack.c
index b26f2e3..fa653b4 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -370,7 +370,8 @@ static const char *unpack(void)
hdr_err = parse_pack_header(&hdr);
if (hdr_err)
return hdr_err;
- snprintf(hdr_arg, sizeof(hdr_arg), "--pack_header=%u,%u",
+ snprintf(hdr_arg, sizeof(hdr_arg),
+ "--pack_header=%"PRIu32",%"PRIu32,
ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries));
if (ntohl(hdr.hdr_entries) < unpack_limit) {
diff --git a/sha1_file.c b/sha1_file.c
index 191f814..7610c43 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -471,7 +471,7 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
version = ntohl(hdr->idx_version);
if (version < 2 || version > 2) {
munmap(idx_map, idx_size);
- return error("index file %s is version %d"
+ return error("index file %s is version %"PRIu32
" and is not supported by this binary"
" (try upgrading GIT to a newer version)",
path, version);
@@ -682,14 +682,14 @@ static int open_packed_git_1(struct packed_git *p)
if (hdr.hdr_signature != htonl(PACK_SIGNATURE))
return error("file %s is not a GIT packfile", p->pack_name);
if (!pack_version_ok(hdr.hdr_version))
- return error("packfile %s is version %u and not supported"
- " (try upgrading GIT to a newer version)",
+ return error("packfile %s is version %"PRIu32" and not"
+ " supported (try upgrading GIT to a newer version)",
p->pack_name, ntohl(hdr.hdr_version));
/* Verify the pack matches its index. */
if (p->num_objects != ntohl(hdr.hdr_entries))
- return error("packfile %s claims to have %u objects"
- " while index indicates %u objects",
+ return error("packfile %s claims to have %"PRIu32" objects"
+ " while index indicates %"PRIu32" objects",
p->pack_name, ntohl(hdr.hdr_entries),
p->num_objects);
if (lseek(p->pack_fd, p->pack_size - sizeof(sha1), SEEK_SET) == -1)
@@ -1719,7 +1719,7 @@ off_t find_pack_entry_one(const unsigned char *sha1,
}
if (debug_lookup)
- printf("%02x%02x%02x... lo %u hi %u nr %u\n",
+ printf("%02x%02x%02x... lo %u hi %u nr %"PRIu32"\n",
sha1[0], sha1[1], sha1[2], lo, hi, p->num_objects);
if (use_lookup < 0)
diff --git a/show-index.c b/show-index.c
index 7253991..45bb535 100644
--- a/show-index.c
+++ b/show-index.c
@@ -68,7 +68,8 @@ int main(int argc, char **argv)
ntohl(off64[1]);
off64_nr++;
}
- printf("%" PRIuMAX " %s (%08x)\n", (uintmax_t) offset,
+ printf("%" PRIuMAX " %s (%08"PRIx32")\n",
+ (uintmax_t) offset,
sha1_to_hex(entries[i].sha1),
ntohl(entries[i].crc));
}
--
1.5.6
^ permalink raw reply related
* Re: Question about git-merge-stupid
From: Linus Torvalds @ 2008-07-03 17:08 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <20080703123314.GS4729@genesis.frugalware.org>
On Thu, 3 Jul 2008, Miklos Vajna wrote:
>
> I'm trying to understand what is the difference between different merge
> strategies. git-merge-stupid is something I failed to understand from
> the git history/code/documentation, so I'm asking here.
>
> 1) From git history:
>
> It seems git-merge-stupid was created by 2276aa6 when Junio renamed
> -resolve to -stupid and let -resolve use read-tree. Actually
>
> git show 2276aa6:git-merge-stupid.sh
>
> says -stupid uses read-tree as well.
I think -stupid should probably be removed.
The history of -stupid is from doing the simple single-tree resolve that
git-read-tree can do, but then doing the obvious hack of just trying to
pick the base that gives the least number of conflicts.
HOWEVER.
- in practice, there's seldom any actual point to it. In 99% of all
cases, you only have a single merge base anyway.
- if you have a workflow that encourages criss-cross merges (which makes
the above "in practice" not be true), the common case will be that the
merge base doesn't much matter.
- Counting conflicts by looking at the numbe of files that conflict is a
pretty stupid metric anyway. Yes, it's obvious, and yes, I bet there
are cases where it does the right thing, but I also bet there are cases
where it does the _wrong_ thing - it might pick a merge base with fewer
files conflicting, but with harder conflicts.
- the "recursive" merge strategy simply handles things better. There's
not really any reason to use a "pick random merge base that happens to
give least conflicts", when the recursive strategy does something much
more natural.
So you shouldn't really compare -stupid to -resolve. You should compare
-stupid to -recursive, and the latter is simply much better.
> 2) From code:
>
> It seems -stupid is better than -resolve when there are multiple bases.
Maybe. And maybe not.
> $ git merge -s resolve c
> Trying simple merge.
> Merge made by resolve.
>
> So it seems resolve does not completely fail if there are multiple
> bases, either.
I think -resolve can handle up to 6 bases, or something like that. After
that it should fail with a "I cannot read more than 8 trees" or something
(eight being the two trees to be merged, plus the six bases).
And with multiple bases, it will already pick the best one on a per-file
basis (I think - I should know the threeway merge, but it is pretty
confusing code) rather than trying to pick one globally. Not pretty, but
it's yet another reason why -stupid is actually stupid, and not worth it.
So -stupid in _theory_ can handle cases that -resolve cannot (more than
six bases), but (a) that doesn't happen and (b) you'd be better off with
-recursive anyway.
> 3) From documentation:
>
> Actually -stupid is missing from Documentation/merge-strategies.txt. I
> plan to send a patch to add it, once I understnad what it does. :-)
Well, see above. I think there's a reason why -stupid isn't even worth
documenting. It might be better off just removed.
Linus
^ permalink raw reply
* [RFC/PATCH (WIP)] Git.pm: Add get_config() method and related subroutines
From: Jakub Narebski @ 2008-07-03 16:24 UTC (permalink / raw)
To: git; +Cc: Petr Baudis, Lea Wiemann
Add get_config([PREFIX]) method, taken from current gitweb, which
parses whole (or selected part) config file into hash (reading
"git config -z -l" output). This means that we do not have to call
one git command per config variable... but it also means that
conversion to boolean, to integer, or to color must be done from
within Perl; you can use config_val_to_* functions for that.
NOTE: Currently config_val_to_color and config_val_to_colorbool
are lacking; error checking is more relaxed in config_val_to_bool().
One advantage of ->get_config() over ->config(VARIABLE) is that it can
deal correctly with "no value" variables: they are !defined(), but
they do exists().
Tests are included; while at it add some more tests for generic
->config*() methods.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Cc-ed Petr "Pasky" Baudis, who is author of Git.pm, and Lea Wiemann,
who is author of Git.pm test t/t9700-perl-git.sh. Lea is also
working on object interface to git in Perl (Git::Repo etc.); I hope
I am not repeating her work.
This is WIP (Work In Progress) as much as an RFC (Request For Comments)
patch, as there are a few things which are not finished or not cleaned
up:
* there is no config_val_to_*() equivalent of ->get_colorbool() and
->get_color() methods to convert config values to ANSI color escape
sequences.
* config_val_to_bool() and config_val_to_int() does not error out
on values which are not boolean or not integer, contrary to what
usage of "git config --bool" and "git config --int" does in
->config_bool() and ->config_int() methods, respectively.
This should be fairly easy to add by manually throwing Error...
the minor trouble would be to follow what ->config_bool etc. does.
* neither config_val_to_bool nor config_val_to_int are exported.
* tests contain some cruft in 'set up test repository' stage, which
was inspected manually that is correct (by examining Data::Dumper
output of new ->get_config() method against tested config file),
but for which actual tests were written.
There are also a few things which I'd like some comments about:
* Do config_val_to_bool and config_val_to_int should be exported
by default?
* Should config_val_to_bool and config_val_to_int throw error or
just return 'undef' on invalid values? One can check if variable
is defined using "exists($config_hash{'varname'})".
* How config_val_to_bool etc. should be named? Perhaps just
config_to_bool, like in gitweb?
* Is "return wantarray ? %config : \%config;" DWIM-mery good style?
I am _not_ a Perl hacker...
* Should ->get_config() use ->command_output_pipe, or simpler
->command() method, reading whole config into array?
* What should ->get_config() method be named? ->get_config()
or perhaps ->config_hash(), or ->config_hashref()?
* What should ->get_config() have as an optional parameter:
PREFIX (/^$prefix/o), or simply SECTION (/^(?:$section)\./o)?
* Should config_val_to_* be tested against ->config_* output?
* Should we perltie hash?
As this is an RFC I have not checked if manpage (generated from
embedded POD documentation) renders correctly.
perl/Git.pm | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++
t/t9700-perl-git.sh | 10 ++++-
t/t9700/test.pl | 32 +++++++++++++++
3 files changed, 148 insertions(+), 1 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 97e61ef..2f4a306 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -668,6 +668,113 @@ sub get_color {
return $color;
}
+=item get_config ( [ PREFIX ] )
+
+Return hash (in list context) or hashref of the whole configuration,
+in the similar manner that C<config --list>, optionally limited
+to config entries which fully qualified key (variable) name begins
+with C<PREFIX> (usually name of section).
+
+The keys of returned hash are fully qualified value names (section,
+optional subsection, and variable name joined using '.'). If variable
+is set only once its value is used as hash value, if variable is set
+multiple times array reference of all values is used as hash value
+for given key.
+
+Please remember that section names and key names in config hash keys
+(in fully qualified config variable name) are normalized, which means
+that they are in lowercase.
+
+=cut
+
+sub get_config {
+ my ($self, $prefix) = _maybe_self(@_);
+
+ my @cmd = ('config');
+ unshift @cmd, $self if $self;
+ my ($fh, $ctx) = command_output_pipe(@cmd, '-z', '--list');
+
+ my %config;
+ local $/ = "\0";
+ $prefix = quotemeta($prefix) if defined($prefix);
+ while (my $keyval = <$fh>) {
+ chomp $keyval;
+ my ($key, $value) = split(/\n/, $keyval, 2);
+
+ if (!defined $prefix || $key =~ /^$prefix/o) {
+ # store multiple values for single key as anonymous array reference
+ # single values stored directly in the hash, not as [ <value> ]
+ if (!exists $config{$key}) {
+ $config{$key} = $value;
+ } elsif (!ref $config{$key}) {
+ $config{$key} = [ $config{$key}, $value ];
+ } else {
+ push @{$config{$key}}, $value;
+ }
+ }
+ }
+ my @ctx = ($fh, $ctx);
+ unshift @ctx, $self if $self;
+ command_close_pipe(@ctx);
+
+ return wantarray ? %config : \%config;
+}
+
+=item config_val_to_bool ( VALUE )
+
+Convert config value C<VALUE> to boolean; no value, number > 0, 'true'
+and 'yes' values are true, rest of values are treated as false (never
+as error, at least for now).
+
+This function is meant to be used on values in hash returned by
+C<get_config>.
+
+=cut
+
+sub config_val_to_bool {
+ my $val = shift;
+
+ # strip leading and trailing whitespace
+ $val =~ s/^\s+//;
+ $val =~ s/\s+$//;
+
+ return (!defined $val || # section.key
+ ($val =~ /^\d+$/ && $val) || # section.key = 1
+ ($val =~ /^(?:true|yes)$/i)); # section.key = true
+}
+
+=item config_val_to_int ( VALUE )
+
+Convert config value C<VALUE> to simple decimal number; an optional
+value suffix of 'k', 'm', or 'g' will cause the value to be multiplied
+by 1024, 1048576 (1024 x 1024), or 1073741824 (1024 x 1024 x 1024),
+respectively (unknown unit is treated as 1, at least for now).
+
+It does not throw error on argument which is not integer.
+
+This function is meant to be used on values in hash returned by
+C<get_config>.
+
+=cut
+
+sub config_val_to_int {
+ my $val = shift;
+
+ # strip leading and trailing whitespace
+ $val =~ s/^\s+//;
+ $val =~ s/\s+$//;
+
+ if (my ($num, $unit) = ($val =~ /^([0-9]*)([kmg])$/i)) {
+ $unit = lc($unit);
+ # unknown unit is treated as 1
+ return $num * ($unit eq 'g' ? 1073741824 :
+ $unit eq 'm' ? 1048576 :
+ $unit eq 'k' ? 1024 : 1);
+ }
+ return $val;
+}
+
+
=item ident ( TYPE | IDENTSTR )
=item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 9706ee5..af6ac58 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -34,9 +34,17 @@ test_expect_success \
git-config --add test.booltrue true &&
git-config --add test.boolfalse no &&
git-config --add test.boolother other &&
- git-config --add test.int 2k
+ git-config --add test.int 2k &&
+ git-config --add teSt.duP val1 &&
+ git-config --add tesT.Dup val2 &&
+ git-config --add test.subsection.noDup val &&
+ git-config --add test.subSection.nodup val &&
+ git-config --add "test.sub # \\ \" '\'' section.key" val &&
+ echo "[test] noval" >> .git/config
'
+test_debug 'cat .git/config'
+
test_external_without_stderr \
'Perl API' \
perl ../t9700/test.pl
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 4d23125..4dd8bbf 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -11,6 +11,8 @@ use Cwd;
use File::Basename;
use File::Temp;
+use Data::Dumper;
+
BEGIN { use_ok('Git') }
# set up
@@ -30,11 +32,36 @@ is($r->config_int("test.int"), 2048, "config_int: integer");
is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent");
ok($r->config_bool("test.booltrue"), "config_bool: true");
ok(!$r->config_bool("test.boolfalse"), "config_bool: false");
+ok($r->config_bool("test.noval"), "config_bool: true (noval)");
our $ansi_green = "\x1b[32m";
is($r->get_color("color.test.slot1", "red"), $ansi_green, "get_color");
# Cannot test $r->get_colorbool("color.foo")) because we do not
# control whether our STDOUT is a terminal.
+# testing get_config() and related functions/subroutines/methods
+is_deeply(scalar($r->get_config('color.')), {'color.test.slot1' => 'green'},
+ "get_config('color.')");
+my %config;
+ok(%config = $r->get_config(), "get_config(): list context");
+is($config{"test.string"}, "value",
+ "\%config scalar: string");
+is_deeply($config{"test.dupstring"}, ["value1", "value2"],
+ "\%config array: string");
+is($config{"test.nonexistent"}, undef,
+ "\%config scalar: nonexistent (undef)");
+ok(!exists($config{"test.nonexistent"}),
+ "\%config scalar: nonexistent (!exists)");
+is(Git::config_val_to_int($config{"test.int"}), 2048,
+ "config_val_to_int: integer");
+is(Git::config_val_to_int($config{"test.nonexistent"}), undef,
+ "config_val_to_int: nonexistent");
+ok( Git::config_val_to_bool($config{"test.booltrue"}),
+ "config_val_to_bool: true");
+ok(!Git::config_val_to_bool($config{"test.boolfalse"}),
+ "config_val_to_bool: false");
+ok( Git::config_val_to_bool($config{"test.noval"}),
+ "config_val_to_bool: true (noval)");
+
# Failure cases for config:
# Save and restore STDERR; we will probably extract this into a
# "dies_ok" method and possibly move the STDERR handling to Git.pm.
@@ -43,6 +70,11 @@ eval { $r->config("test.dupstring") };
ok($@, "config: duplicate entry in scalar context fails");
eval { $r->config_bool("test.boolother") };
ok($@, "config_bool: non-boolean values fail");
+TODO: {
+ $TODO = "config_val_to_bool returns false on non-bool values";
+ eval { Git::config_val_to_bool($config{"test.boolother"}) };
+ ok($@, "config_val_to_bool: non-boolean values fail");
+}
open STDERR, ">&", $tmpstderr or die "cannot restore STDERR";
# ident
--
1.5.6.1
^ permalink raw reply related
* Re: [PATCH 0/3] Making remote tracking statistics available to other tools
From: Olivier Marin @ 2008-07-03 16:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ingo Molnar, Bruce Stephens, git
In-Reply-To: <7v3amsg1wx.fsf@gitster.siamese.dyndns.org>
Junio C Hamano a écrit :
> The one I sent out was a bit hacky as the existing implementation inside
> git-checkout was not designed to be cleanly reusable.
>
> Here is a cleaned up series that could be applied.
>
> [PATCH 1/3] Refactor "tracking statistics" code used by "git checkout"
> [PATCH 2/3] git-status: show the remote tracking statistics
> [PATCH 3/3] git-branch -v: show the remote tracking statistics
Sorry, but the third patch does not work for me. FWIU, the problem comes from
the revision walking code that is not reentrant because of the object cache.
IOW, calling stats_tracking_info() more than once does not work: objects
states changed after first call.
I do not understand the code enough yet and I failed to fix it. The only way
I found is by clearing obj_hash (object.c) but this seems the wrong thing to
do.
Ideas?
This test case should trigger the bug:
$ for i in a b c; do echo $i>$i && git add $i && git commit -m $i; done
$ git clone . clone1
$ cd clone1
$ git checkout -b b1 origin/master
$ git reset --hard HEAD~
$ echo d>d && git add d && git commit -m d
$ git checkout -b b2 origin/master
$ git reset --hard HEAD~
$ git merge b1
$ git branch -v
b1 c07015a [ahead 1, behind 1] d
* b2 c07015a d
master 08ca18b c
b1 and b2 are both 1 commit ahead and 1 commit behind.
$ git branch -D b1
$ git branch -v
* b2 c07015a [ahead 1, behind 1] d
master 08ca18b c
deleting b1 show us the side effect.
Olivier.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox