* Re: git status in clean working dir
From: Jeff King @ 2008-07-22 7:10 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722064603.GA25221@sigill.intra.peff.net>
On Tue, Jul 22, 2008 at 02:46:04AM -0400, Jeff King wrote:
> I am tempted by the "order switching" I mentioned, but that would entail
> the git process waiting to clean the pager, during which time it may be
> consuming memory. But maybe that isn't worth worrying about.
It feels very wrong proposing this during release freeze, but here is
the "pager is child of git" implementation.
Patch 1/1 adds a bit of necessary infrastructure to run-command, and
patch 2/2 does the deed. The nice thing is that it unifies the Windows
and Unix implementations of setup_pager, so we get a nice line
reduction.
pager.c | 49 ++++++++-----------------------------------------
run-command.c | 2 ++
run-command.h | 1 +
3 files changed, 11 insertions(+), 41 deletions(-)
^ permalink raw reply
* Re: [StGit PATCH] Test that we can add a new file to a non-topmost patch with refresh -p
From: Karl Hasselström @ 2008-07-22 7:14 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git, Jon Smirl
In-Reply-To: <b0943d9e0807211501g68d1cffeh12bcb34df0f28dae@mail.gmail.com>
On 2008-07-21 23:01:17 +0100, Catalin Marinas wrote:
> I don't think we should spend time on fixing the current code as you
> already have a new implementation. Maybe we could add a simple test
> and refuse refreshing other than the topmost patch in case of files
> added to the index.
Yes, I guess that's OK. Hmm, how do we check that cheaply?
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH] Enable threaded delta search on *BSD and Linux.
From: Pierre Habouzit @ 2008-07-22 6:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsjeplhl.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 840 bytes --]
On Tue, Jul 22, 2008 at 04:54:14AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> >
> > Following the discussion we had 10 days ago, here is a proposal to enable
> > threaded delta search on systems that are likely to behave properly wrt
> > memory and CPU usage.
>
> Hmmm.
>
> I do not want to do this kind of thing very close to the release (like
> now), but rather immediately after 1.6.0. Will queue for 'next'.
>
> Distro people can decide for themselves and their users, but I am of
> conservative kind.
makes sense to me.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: git status in clean working dir
From: Jeff King @ 2008-07-22 6:46 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722061807.GA6714@glandium.org>
On Tue, Jul 22, 2008 at 08:18:07AM +0200, Mike Hommey wrote:
> > We already do that (see pager.c:53). The original poster still had a
> > problem, but I don't know if it was for actual usage or simply a toy
> >
> > $ git status
> > $ echo $?
> > $ echo "why don't exit codes work in status?" | mail git@vger
> >
> > question.
>
> As you said in another branch of the thread, this part would be solved by
> having parent/child being reverted.
>
> Now, for the case where diff-files can have a pager if the user shoots
> himself in the foot, if the output is not a terminal and pager.c already
> does the right thing, I don't see where diff-files having a pager will
> be a problem.
Ah, OK. I misunderstood your original post. Yes, there are two ways
paging can screw you: munging the data in a pipeline and munging the
exit code. We already deal with former, so it is really just the latter
that is posing a problem in this thread.
I am tempted by the "order switching" I mentioned, but that would entail
the git process waiting to clean the pager, during which time it may be
consuming memory. But maybe that isn't worth worrying about.
-Peff
^ permalink raw reply
* Re: [PATCH] bring description of git diff --cc up to date
From: Junio C Hamano @ 2008-07-22 6:41 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, David Greaves
In-Reply-To: <Pine.GSO.4.62.0807211316530.2841@harper.uchicago.edu>
Jonathan Nieder <jrnieder@uchicago.edu> writes:
> + This flag implies the '-c' option and makes the patch output
> + even more compact by omitting uninteresting hunks. A hunk is
> + considered interesting only if either (a) it shows changes from
> + all parents or (b) in an Octopus merge, it shows different changes
> + from at least three different parents.
I am not sure where that "at lesta three different parents" comes from.
It might be that what the logic does can be expressed that way, but that
was not the guiding principle why the code does what it currently does.
These two might make it clearer what hunks are considered interesting:
http://thread.gmane.org/gmane.comp.version-control.git/15098/focus=15109
http://thread.gmane.org/gmane.comp.version-control.git/15486/focus=15491
Especially the latter thread, not just the focused one but the whole
thing, is quite amusing.
This one gives an insight into the logical consequence of the hunk
selection criteria:
http://thread.gmane.org/gmane.comp.version-control.git/15486/focus=15600
it is worth a read after understanding why some hunks are omitted and some
hunks are included by reading other articles in the thread.
^ permalink raw reply
* Re: git status in clean working dir
From: Mike Hommey @ 2008-07-22 6:18 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722060643.GA25023@sigill.intra.peff.net>
On Tue, Jul 22, 2008 at 02:06:43AM -0400, Jeff King wrote:
> On Tue, Jul 22, 2008 at 07:39:21AM +0200, Mike Hommey wrote:
>
> > > This marks diff-files as FORBID_PAGER; I will leave it to others to
> > > fight about which commands should have it. But it doesn't make sense to
> > > mark "status" since some people obviously _want_ the paging there.
> >
> > Why not "simply" forbid the pager when output is not a terminal ?
>
> We already do that (see pager.c:53). The original poster still had a
> problem, but I don't know if it was for actual usage or simply a toy
>
> $ git status
> $ echo $?
> $ echo "why don't exit codes work in status?" | mail git@vger
>
> question.
As you said in another branch of the thread, this part would be solved by
having parent/child being reverted.
Now, for the case where diff-files can have a pager if the user shoots
himself in the foot, if the output is not a terminal and pager.c already
does the right thing, I don't see where diff-files having a pager will
be a problem.
If diff-files' output is a terminal, it's obviously intended to be
displayed, be it in a script or not. But most of the time, its output
will be piped, thus not triggering the pager anyways.
And for diff-files' exit code, well, see the first paragraph.
Mike
^ permalink raw reply
* Re: extracting the history of a single file as a new project [Was: Re: making a branch with just one file and keeping its whole]
From: Sverre Rabbelier @ 2008-07-22 6:15 UTC (permalink / raw)
To: ncrfgs, Junio C Hamano, Johannes Schindelin; +Cc: git, madewokherd
In-Reply-To: <20080722041457.52120c2f@mail.tin.it>
On Tue, Jul 22, 2008 at 4:14 AM, <ncrfgs@tin.it> wrote:
> Let's say we are at commit #3000, I have this content I want to start a
> new project from, which has been put in three different files:
>
> path1/filename1 from commit 1 to commit 1000
> path2/filename1 from commit 1001 to commit 2000
> path2/filename2 from commit 2001 to commit 3000
>
> In the meanwhile path1/filename1 has been created on commit 2500 with
> path1/filename1 having nothing to do with the new project I'd like to
> start.
<snip>
> My first idea to accomplish what I'd like to do, was to use the ouput of
> `git-log -p --follow path2/filename2` with another git command; on
> #git@freenode I've been suggested to use git-clone and
> git-filter-branch.
>
It would seem that this is a valid use-case for a properly working
"git log --follow", yes?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [PATCH 2/2] bisect: only check merge bases when needed
From: Christian Couder @ 2008-07-22 6:16 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: Michael Haggerty, Jeff King, git
When one good revision is not an ancestor of the bad revision, the
merge bases between the good and the bad revision should be checked
to make sure that they are also good revisions.
Some previous patches take care of that, but they may check the
merge bases more often than really needed. In fact the previous
patches did not try to optimize this as much as possible because
it is not so simple. So this is the purpose of this patch.
One may think that when all the merge bases have been checked then
we can save a flag, so that we don't need to check the merge bases
again during the bisect process.
The problem is that the user may choose to checkout and test
something completely different from what the bisect process
suggested. In this case we have to check the merge bases again,
because there may be new merge bases relevant to the bisect
process.
That's why, in this patch, when we detect that the user tested
something else than what the bisect process suggested, we remove
the flag that says that we don't need to check the merge bases
again.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
git-bisect.sh | 47 +++++++++++++++++++++++++++++++-----------
t/t6030-bisect-porcelain.sh | 23 +++++++++++++++++++++
2 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index f6cb110..a7f5347 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -172,6 +172,25 @@ bisect_write() {
test -n "$nolog" || echo "git bisect $state $rev" >>"$GIT_DIR/BISECT_LOG"
}
+is_expected_rev() {
+ test -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
+ test "$1" = $(cat "$GIT_DIR/BISECT_EXPECTED_REV")
+}
+
+mark_expected_rev() {
+ echo "$1" > "$GIT_DIR/BISECT_EXPECTED_REV"
+}
+
+check_expected_revs() {
+ for _rev in "$@"; do
+ if ! is_expected_rev "$_rev"; then
+ rm -f "$GIT_DIR/BISECT_ANCESTORS_OK"
+ rm -f "$GIT_DIR/BISECT_EXPECTED_REV"
+ return
+ fi
+ done
+}
+
bisect_state() {
bisect_autostart
state=$1
@@ -181,7 +200,8 @@ bisect_state() {
1,bad|1,good|1,skip)
rev=$(git rev-parse --verify HEAD) ||
die "Bad rev input: HEAD"
- bisect_write "$state" "$rev" ;;
+ bisect_write "$state" "$rev"
+ check_expected_revs "$rev" ;;
2,bad|*,good|*,skip)
shift
eval=''
@@ -191,7 +211,8 @@ bisect_state() {
die "Bad rev input: $rev"
eval="$eval bisect_write '$state' '$sha'; "
done
- eval "$eval" ;;
+ eval "$eval"
+ check_expected_revs "$@" ;;
*,bad)
die "'git bisect bad' can take only one argument." ;;
*)
@@ -320,6 +341,7 @@ checkout() {
_rev="$1"
_msg="$2"
echo "Bisecting: $_msg"
+ mark_expected_rev "$_rev"
git checkout -q "$_rev" || exit
git show-branch "$_rev"
}
@@ -339,18 +361,10 @@ mark_merge_base_ok() {
echo "$1 $2 ok" >> "$GIT_DIR/BISECT_MERGE_BASES"
}
-is_testing_merge_base() {
- grep "^testing $1$" "$GIT_DIR/BISECT_MERGE_BASES" >/dev/null 2>&1
-}
-
-mark_testing_merge_base() {
- echo "testing $1" >> "$GIT_DIR/BISECT_MERGE_BASES"
-}
-
handle_bad_merge_base() {
_badmb="$1"
_g="$2"
- if is_testing_merge_base "$_badmb"; then
+ if is_expected_rev "$_badmb"; then
cat >&2 <<EOF
The merge base $_badmb is bad.
This means the bug has been fixed between $_badmb and $_g.
@@ -391,7 +405,6 @@ check_merge_bases() {
elif is_among "$_mb" "$_skip"; then
handle_skipped_merge_base "$_bad" "$_g" "_mb"
else
- mark_testing_merge_base "$_mb"
checkout "$_mb" "a merge base must be tested"
checkout_done=1
return
@@ -402,11 +415,17 @@ check_merge_bases() {
}
check_good_are_ancestors_of_bad() {
+ test -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
+ return
_bad="$1"
_good=$(echo $2 | sed -e 's/\^//g')
_skip="$3"
_side=$(git rev-list $_good ^$_bad)
- test -n "$_side" && check_merge_bases "$_bad" "$_good" "$_skip"
+ if test -n "$_side"; then
+ check_merge_bases "$_bad" "$_good" "$_skip" || return
+ test "$checkout_done" -eq "1" && return
+ fi
+ : > "$GIT_DIR/BISECT_ANCESTORS_OK"
}
bisect_next() {
@@ -496,6 +515,8 @@ bisect_clean_state() {
git update-ref -d $ref $hash || exit
done
rm -f "$GIT_DIR/BISECT_MERGE_BASES" &&
+ rm -f "$GIT_DIR/BISECT_EXPECTED_REV" &&
+ rm -f "$GIT_DIR/BISECT_ANCESTORS_OK" &&
rm -f "$GIT_DIR/BISECT_LOG" &&
rm -f "$GIT_DIR/BISECT_NAMES" &&
rm -f "$GIT_DIR/BISECT_RUN" &&
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 503229b..17f264b 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -440,6 +440,29 @@ test_expect_success 'good merge bases when good and bad are siblings' '
git bisect reset
'
+check_trace() {
+ grep "$1" "$GIT_TRACE" | grep "\^$2" | grep "$3" >/dev/null
+}
+
+test_expect_success 'optimized merge base checks' '
+ GIT_TRACE="$(pwd)/trace.log" &&
+ export GIT_TRACE &&
+ git bisect start "$HASH7" "$SIDE_HASH7" > my_bisect_log.txt &&
+ grep "merge base must be tested" my_bisect_log.txt &&
+ grep "$HASH4" my_bisect_log.txt &&
+ check_trace "rev-list" "$HASH7" "$SIDE_HASH7" &&
+ git bisect good > my_bisect_log2.txt &&
+ test -f ".git/BISECT_ANCESTORS_OK" &&
+ test "$HASH6" = $(git rev-parse --verify HEAD) &&
+ : > "$GIT_TRACE" &&
+ git bisect bad > my_bisect_log3.txt &&
+ test_must_fail check_trace "rev-list" "$HASH6" "$SIDE_HASH7" &&
+ git bisect good "$A_HASH" > my_bisect_log4.txt &&
+ grep "merge base must be tested" my_bisect_log4.txt &&
+ test_must_fail test -f ".git/BISECT_ANCESTORS_OK" &&
+ check_trace "rev-list" "$HASH6" "$A_HASH"
+'
+
#
#
test_done
--
1.5.6.2.300.gbed4
^ permalink raw reply related
* [PATCH 1/2] bisect: test merge base if good rev is not an ancestor of bad rev
From: Christian Couder @ 2008-07-22 6:15 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: Michael Haggerty, Jeff King, git
Before this patch, "git bisect", when it was given some good revs that
are not ancestor of the bad rev, didn't check if the merge bases were
good. "git bisect" just supposed that the user knew what he was doing,
and that, when he said the revs were good, he knew that it meant that
all the revs in the history leading to the good revs were also
considered good.
But in pratice, the user may not know that a good rev is not an
ancestor of the bad rev, or he may not know/remember that all revs
leading to the good rev will be considered good. So he may give a good
rev that is a sibling, instead of an ancestor, of the bad rev, when in
fact there can be one rev becoming good in the branch of the good rev
(because the bug was already fixed there for example) instead of one
rev becoming bad in the branch of the bad rev.
For example, if there is the following history:
A-B-C-D
\E-F
and we launch "git bisect start D F" then only C and D would have been
considered as possible first bad commit before this patch. This may be
wrong because A, B and E may be bad too if the bug exists everywhere
except in F that fixes it.
The purpose of this patch is to detect when "git bisect" is passed
some good revs that are not ancestor of the bad rev, and then to first
ask the user to test the merge bases between the good and bad revs.
If the merge bases are good then all is fine, we can continue
bisecting. Otherwise, if one merge base is bad, it means that the
assumption that all revs leading to the good one are good too is
wrong and we error out. In the case where one merge base is skipped we
issue a warning and then continue bisecting anyway.
These checks will also catch the case where good and bad have been
mistaken. This means that we can remove the check that was done latter
on the output of "git rev-list --bisect-vars".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
git-bisect.sh | 132 ++++++++++++++++++++++++++++++++++--------
t/t6030-bisect-porcelain.sh | 90 +++++++++++++++++++++++++++++
2 files changed, 197 insertions(+), 25 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 3cac20d..f6cb110 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -242,33 +242,18 @@ bisect_auto_next() {
bisect_next_check && bisect_next || :
}
-eval_rev_list() {
- _eval="$1"
-
- eval $_eval
- res=$?
-
- if [ $res -ne 0 ]; then
- echo >&2 "'git rev-list --bisect-vars' failed:"
- echo >&2 "maybe you mistake good and bad revs?"
- exit $res
- fi
-
- return $res
-}
-
filter_skipped() {
_eval="$1"
_skip="$2"
if [ -z "$_skip" ]; then
- eval_rev_list "$_eval"
+ eval "$_eval"
return
fi
# Let's parse the output of:
# "git rev-list --bisect-vars --bisect-all ..."
- eval_rev_list "$_eval" | while read hash line
+ eval "$_eval" | while read hash line
do
case "$VARS,$FOUND,$TRIED,$hash" in
# We display some vars.
@@ -331,20 +316,118 @@ exit_if_skipped_commits () {
fi
}
+checkout() {
+ _rev="$1"
+ _msg="$2"
+ echo "Bisecting: $_msg"
+ git checkout -q "$_rev" || exit
+ git show-branch "$_rev"
+}
+
+is_among() {
+ _rev="$1"
+ _list="$2"
+ case "$_list" in *$_rev*) return 0 ;; esac
+ return 1
+}
+
+is_merge_base_ok() {
+ grep "^$1 $2 ok$" "$GIT_DIR/BISECT_MERGE_BASES" >/dev/null 2>&1
+}
+
+mark_merge_base_ok() {
+ echo "$1 $2 ok" >> "$GIT_DIR/BISECT_MERGE_BASES"
+}
+
+is_testing_merge_base() {
+ grep "^testing $1$" "$GIT_DIR/BISECT_MERGE_BASES" >/dev/null 2>&1
+}
+
+mark_testing_merge_base() {
+ echo "testing $1" >> "$GIT_DIR/BISECT_MERGE_BASES"
+}
+
+handle_bad_merge_base() {
+ _badmb="$1"
+ _g="$2"
+ if is_testing_merge_base "$_badmb"; then
+ cat >&2 <<EOF
+The merge base $_badmb is bad.
+This means the bug has been fixed between $_badmb and $_g.
+EOF
+ exit 3
+ else
+ cat >&2 <<EOF
+Some good revs are not ancestor of the bad rev.
+git bisect cannot work properly in this case.
+Maybe you mistake good and bad revs?
+EOF
+ exit 1
+ fi
+}
+
+handle_skipped_merge_base() {
+ _bad="$1"
+ _g="$2"
+ _mb="$3"
+ cat >&2 <<EOF
+Warning: the merge base between $_bad and $_g must be skipped.
+So we cannot be sure the first bad commit is between $_mb and $_bad.
+We continue anyway.
+EOF
+}
+
+check_merge_bases() {
+ _bad="$1"
+ _good="$2"
+ _skip="$3"
+ for _g in $_good; do
+ is_merge_base_ok "$_bad" "$_g" && continue
+ for _mb in $(git merge-base --all $_g $_bad); do
+ if test "$_mb" = "$_g" || is_among "$_mb" "$_good"; then
+ continue
+ elif test "$_mb" = "$_bad"; then
+ handle_bad_merge_base "$_bad" "$_g"
+ elif is_among "$_mb" "$_skip"; then
+ handle_skipped_merge_base "$_bad" "$_g" "_mb"
+ else
+ mark_testing_merge_base "$_mb"
+ checkout "$_mb" "a merge base must be tested"
+ checkout_done=1
+ return
+ fi
+ done
+ mark_merge_base_ok "$_bad" "$_g"
+ done
+}
+
+check_good_are_ancestors_of_bad() {
+ _bad="$1"
+ _good=$(echo $2 | sed -e 's/\^//g')
+ _skip="$3"
+ _side=$(git rev-list $_good ^$_bad)
+ test -n "$_side" && check_merge_bases "$_bad" "$_good" "$_skip"
+}
+
bisect_next() {
case "$#" in 0) ;; *) usage ;; esac
bisect_autostart
bisect_next_check good
+ # Get bad, good and skipped revs
+ bad=$(git rev-parse --verify refs/bisect/bad) &&
+ good=$(git for-each-ref --format='^%(objectname)' \
+ "refs/bisect/good-*" | tr '\012' ' ') &&
skip=$(git for-each-ref --format='%(objectname)' \
- "refs/bisect/skip-*" | tr '\012' ' ') || exit
+ "refs/bisect/skip-*" | tr '\012' ' ') &&
+ # Maybe some merge bases must be tested first
+ check_good_are_ancestors_of_bad "$bad" "$good" "$skip" || exit
+ test "$checkout_done" -eq "1" && checkout_done='' && return
+
+ # Get bisection information
BISECT_OPT=''
test -n "$skip" && BISECT_OPT='--bisect-all'
-
- bad=$(git rev-parse --verify refs/bisect/bad) &&
- good=$(git for-each-ref --format='^%(objectname)' \
- "refs/bisect/good-*" | tr '\012' ' ') &&
eval="git rev-list --bisect-vars $BISECT_OPT $good $bad --" &&
eval="$eval $(cat "$GIT_DIR/BISECT_NAMES")" &&
eval=$(filter_skipped "$eval" "$skip") &&
@@ -365,9 +448,7 @@ bisect_next() {
# commit is also a "skip" commit (see above).
exit_if_skipped_commits "$bisect_rev"
- echo "Bisecting: $bisect_nr revisions left to test after this"
- git checkout -q "$bisect_rev" || exit
- git show-branch "$bisect_rev"
+ checkout "$bisect_rev" "$bisect_nr revisions left to test after this"
}
bisect_visualize() {
@@ -414,6 +495,7 @@ bisect_clean_state() {
do
git update-ref -d $ref $hash || exit
done
+ rm -f "$GIT_DIR/BISECT_MERGE_BASES" &&
rm -f "$GIT_DIR/BISECT_LOG" &&
rm -f "$GIT_DIR/BISECT_NAMES" &&
rm -f "$GIT_DIR/BISECT_RUN" &&
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 0626544..503229b 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -350,6 +350,96 @@ test_expect_success 'bisect does not create a "bisect" branch' '
git branch -D bisect
'
+# This creates a "side" branch to test "siblings" cases.
+#
+# H1-H2-H3-H4-H5-H6-H7 <--other
+# \
+# S5-S6-S7 <--side
+#
+test_expect_success 'side branch creation' '
+ git bisect reset &&
+ git checkout -b side $HASH4 &&
+ add_line_into_file "5(side): first line on a side branch" hello2 &&
+ SIDE_HASH5=$(git rev-parse --verify HEAD) &&
+ add_line_into_file "6(side): second line on a side branch" hello2 &&
+ SIDE_HASH6=$(git rev-parse --verify HEAD) &&
+ add_line_into_file "7(side): third line on a side branch" hello2 &&
+ SIDE_HASH7=$(git rev-parse --verify HEAD)
+'
+
+test_expect_success 'good merge base when good and bad are siblings' '
+ git bisect start "$HASH7" "$SIDE_HASH7" > my_bisect_log.txt &&
+ grep "merge base must be tested" my_bisect_log.txt &&
+ grep $HASH4 my_bisect_log.txt &&
+ git bisect good > my_bisect_log.txt &&
+ test_must_fail grep "merge base must be tested" my_bisect_log.txt &&
+ grep $HASH6 my_bisect_log.txt &&
+ git bisect reset
+'
+test_expect_success 'skipped merge base when good and bad are siblings' '
+ git bisect start "$SIDE_HASH7" "$HASH7" > my_bisect_log.txt &&
+ grep "merge base must be tested" my_bisect_log.txt &&
+ grep $HASH4 my_bisect_log.txt &&
+ git bisect skip > my_bisect_log.txt 2>&1 &&
+ grep "Warning" my_bisect_log.txt &&
+ grep $SIDE_HASH6 my_bisect_log.txt &&
+ git bisect reset
+'
+
+test_expect_success 'bad merge base when good and bad are siblings' '
+ git bisect start "$HASH7" HEAD > my_bisect_log.txt &&
+ grep "merge base must be tested" my_bisect_log.txt &&
+ grep $HASH4 my_bisect_log.txt &&
+ test_must_fail git bisect bad > my_bisect_log.txt 2>&1 &&
+ grep "merge base $HASH4 is bad" my_bisect_log.txt &&
+ grep "fixed between $HASH4 and $SIDE_HASH7" my_bisect_log.txt &&
+ git bisect reset
+'
+
+# This creates a few more commits (A and B) to test "siblings" cases
+# when a good and a bad rev have many merge bases.
+#
+# We should have the following:
+#
+# H1-H2-H3-H4-H5-H6-H7
+# \ \ \
+# S5-A \
+# \ \
+# S6-S7----B
+#
+# And there A and B have 2 merge bases (S5 and H5) that should be
+# reported by "git merge-base --all A B".
+#
+test_expect_success 'many merge bases creation' '
+ git checkout "$SIDE_HASH5" &&
+ git merge -m "merge HASH5 and SIDE_HASH5" "$HASH5" &&
+ A_HASH=$(git rev-parse --verify HEAD) &&
+ git checkout side &&
+ git merge -m "merge HASH7 and SIDE_HASH7" "$HASH7" &&
+ B_HASH=$(git rev-parse --verify HEAD) &&
+ git merge-base --all "$A_HASH" "$B_HASH" > merge_bases.txt &&
+ test $(wc -l < merge_bases.txt) = "2" &&
+ grep "$HASH5" merge_bases.txt &&
+ grep "$SIDE_HASH5" merge_bases.txt
+'
+
+test_expect_success 'good merge bases when good and bad are siblings' '
+ git bisect start "$B_HASH" "$A_HASH" > my_bisect_log.txt &&
+ grep "merge base must be tested" my_bisect_log.txt &&
+ git bisect good > my_bisect_log2.txt &&
+ grep "merge base must be tested" my_bisect_log2.txt &&
+ {
+ {
+ grep "$SIDE_HASH5" my_bisect_log.txt &&
+ grep "$HASH5" my_bisect_log2.txt
+ } || {
+ grep "$SIDE_HASH5" my_bisect_log2.txt &&
+ grep "$HASH5" my_bisect_log.txt
+ }
+ } &&
+ git bisect reset
+'
+
#
#
test_done
--
1.5.6.2.300.gbed4
^ permalink raw reply related
* Re: [PATCH] bisect: test merge base if good rev is not an ancestor of bad rev
From: Christian Couder @ 2008-07-22 6:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Michael Haggerty, Jeff King, git
In-Reply-To: <alpine.DEB.1.00.0807131504130.4816@eeepc-johanness>
Hi,
Le dimanche 13 juillet 2008, Johannes Schindelin a écrit :
> Hi,
>
> On Sun, 13 Jul 2008, Christian Couder wrote:
> > [PATCH] bisect: check all merge bases instead of only one
>
> It would have been so much nicer to squash the two patches into one, as
> we generally frown upon "let's submit one patch that we know is faulty,
> and then another that fixes it". That's so CVS/SVN.
I didn't think the first one was "faulty". It just didn't fix everything.
> > @@ -384,19 +383,21 @@ check_merge_bases() {
> > _skip="$3"
> > for _g in $_good; do
>
> I really wonder if we can be blessed with less ugly variable names.
> Maybe some that do not start with an underscore for no apparent reason,
There is a reason though. It's because in "bisect_next", variables with
those names ("good", "bad" and "skip") are already used, so reusing the
same name is not easily possible.
> and maybe some that are longer than one letter, so that you have a chance
> to understand later what it is supposed to contain. I.e. something like:
>
> for good in $good_revisions
> do
>
> (You see that I also broke up the "for" and "do" into two lines, as is
> common practice in the rest of Git's shell code.)
There are other places in git-bisect.sh where "for" and "do" are in the same
line. Perhaps one day I will submit a patch to fix these.
> > is_merge_base_ok "$_bad" "$_g" && continue
> > - _mb=$(git merge-base $_g $_bad)
> > - if test "$_mb" = "$_g" || is_among "$_mb" "$_good"; then
> > - mark_merge_base_ok "$_bad" "$_g"
> > - elif test "$_mb" = "$_bad"; then
> > - handle_bad_merge_base "$_bad" "$_g"
> > - elif is_among "$_mb" "$_skip"; then
> > - handle_skipped_merge_base "$_bad" "$_g" "_mb"
> > - else
> > - mark_testing_merge_base "$_mb"
> > - checkout "$_mb" "a merge base must be tested"
> > - checkout_done=1
> > - break
> > - fi
> > + for _mb in $(git merge-base --all $_g $_bad); do
> > + if test "$_mb" = "$_g" || is_among "$_mb" "$_good"; then
> > + continue
> > + elif test "$_mb" = "$_bad"; then
> > + handle_bad_merge_base "$_bad" "$_g"
> > + elif is_among "$_mb" "$_skip"; then
> > + handle_skipped_merge_base "$_bad" "$_g" "_mb"
> > + else
> > + mark_testing_merge_base "$_mb"
> > + checkout "$_mb" "a merge base must be tested"
> > + checkout_done=1
> > + return
> > + fi
> > + done
> > + mark_merge_base_ok "$_bad" "$_g"
> > done
> > }
>
> I really wonder if we cannot do better than that, in terms of code
> complexity.
>
> For example, I wonder if we should special-case the start, and not just
> check everytime if there are unchecked merge bases instead. If there
> are, check the first.
In fact, there was such a thing in my patch, search
for "$GIT_DIR/BISECT_ANCESTORS_OK". But it was a little bit broken if
people didn't test the commit that "git bisect" suggested.
In the next version I will post just after this mail, this is in the 2/2
patch (and hopefully fixed).
> But that can wait until you come back from your vacation...
>
> Have fun,
> Dscho
Thanks,
Christian.
^ permalink raw reply
* Re: git status in clean working dir
From: Jeff King @ 2008-07-22 6:06 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722053921.GA4983@glandium.org>
On Tue, Jul 22, 2008 at 07:39:21AM +0200, Mike Hommey wrote:
> > This marks diff-files as FORBID_PAGER; I will leave it to others to
> > fight about which commands should have it. But it doesn't make sense to
> > mark "status" since some people obviously _want_ the paging there.
>
> Why not "simply" forbid the pager when output is not a terminal ?
We already do that (see pager.c:53). The original poster still had a
problem, but I don't know if it was for actual usage or simply a toy
$ git status
$ echo $?
$ echo "why don't exit codes work in status?" | mail git@vger
question.
-Peff
^ permalink raw reply
* Re: git status in clean working dir
From: Mike Hommey @ 2008-07-22 5:39 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722044157.GA20787@sigill.intra.peff.net>
On Tue, Jul 22, 2008 at 12:41:57AM -0400, Jeff King wrote:
> On Mon, Jul 21, 2008 at 07:40:28PM -0700, Junio C Hamano wrote:
>
> > Actually, the situation is now even worse than I originally thought
> > especially with Jeff's pager.<cmd> patch on 'master' recently. For
> > example, you can screw yourself quite badly by forcing diff-files used in
> > the scripts you run to page, defeating --exit-code option. Which means
>
> Actually, you could _always_ do that with "git -p diff-files". Which is
> obviously stupid, just as setting pager.diff-files is. In the reported
> case, though, "status" is broken, which we now do by default. So no
> stupidity required.
>
> > (2) Then why are we even allowing to configure the plumbing to page?
>
> 1. Laziness. We just never marked which shouldn't be allowed to page.
> But again, in this case, we have explicitly marked status as "this
> should page" so I don't think this is a plumbing / porcelain thing.
> Status fulfills both roles here (some people want it paged, because
> they use it as porcelain, and some people want the exit code).
>
> 2. We don't always know all git commands. We execute user scripts as
> "git foo", but we don't know what they do. Worse than that, we have
> to commit our pager choice early because we might be exec'ing (but
> this is somewhat of an artifact of the way the code is structured,
> and not necessarily an impossible obstacle).
>
> > Should we maintain a table of commands that we allow paging to be
> > customized, and ignore pager.<cmd> for commands that are not in the list?
>
> The patch below sets up the infrastructure, which is trivial. Note that
> this _doesn't_ handle the case of "git -p status", because we have to
> commit that choice at a different time (again, we might be able to
> overcome that with a little code restructuring).
>
> This marks diff-files as FORBID_PAGER; I will leave it to others to
> fight about which commands should have it. But it doesn't make sense to
> mark "status" since some people obviously _want_ the paging there.
Why not "simply" forbid the pager when output is not a terminal ?
Mike
^ permalink raw reply
* Re: [PATCH] builtin-merge: give a proper error message for invalid strategies in config
From: Junio C Hamano @ 2008-07-22 5:22 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <7vr69mpl5v.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
>> Till now 'git merge -s foobar' bailed out with an error message, but
>> foobar in pull.twohead or pull.octopus was just silently ignored. It's
>> better to inform the user then just silently doing nothing.
>>
>> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
>
> Doesn't this make "git merge -s 'recursive resolve'" to misbehave?
Perhaps this would be a better replacement. It makes get_strategy() to
validate and barf on unknown one. It is slightly smaller and more
contained. If/when we add user-defined ones, that logic will also be
contained in the function.
builtin-merge.c | 37 ++++++++++++-------------------------
t/t7601-merge-pull-config.sh | 12 ++++++++++++
2 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index e97c79e..0fd7985 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -77,6 +77,7 @@ static int option_parse_message(const struct option *opt,
static struct strategy *get_strategy(const char *name)
{
int i;
+ struct strbuf err;
if (!name)
return NULL;
@@ -84,7 +85,13 @@ static struct strategy *get_strategy(const char *name)
for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
if (!strcmp(name, all_strategy[i].name))
return &all_strategy[i];
- return NULL;
+
+ strbuf_init(&err, 0);
+ for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+ strbuf_addf(&err, " %s", all_strategy[i].name);
+ fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
+ fprintf(stderr, "Available strategies are:%s.\n", err.buf);
+ exit(1);
}
static void append_strategy(struct strategy *s)
@@ -96,25 +103,10 @@ static void append_strategy(struct strategy *s)
static int option_parse_strategy(const struct option *opt,
const char *name, int unset)
{
- int i;
- struct strategy *s;
-
if (unset)
return 0;
- s = get_strategy(name);
-
- if (s)
- append_strategy(s);
- else {
- struct strbuf err;
- strbuf_init(&err, 0);
- for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
- strbuf_addf(&err, " %s", all_strategy[i].name);
- fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
- fprintf(stderr, "Available strategies are:%s.\n", err.buf);
- exit(1);
- }
+ append_strategy(get_strategy(name));
return 0;
}
@@ -643,14 +635,9 @@ static void add_strategies(const char *string, unsigned attr)
memset(&list, 0, sizeof(list));
split_merge_strategies(string, &list, &list_nr, &list_alloc);
- if (list != NULL) {
- for (i = 0; i < list_nr; i++) {
- struct strategy *s;
-
- s = get_strategy(list[i].name);
- if (s)
- append_strategy(s);
- }
+ if (list) {
+ for (i = 0; i < list_nr; i++)
+ append_strategy(get_strategy(list[i].name));
return;
}
for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 95b4d71..6b9f638 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -126,4 +126,16 @@ test_expect_success 'merge picks up the best result' '
test $auto_count != $resolve_count
'
+test_expect_success 'merge errors out on invalid strategy' '
+ git config pull.twohead "foobar" &&
+ git reset --hard c5 &&
+ test_must_fail git merge c6
+'
+
+test_expect_success 'merge errors out on invalid strategy' '
+ git config --unset-all pull.twohead &&
+ git reset --hard c5 &&
+ test_must_fail git merge -s "resolve recursive" c6
+'
+
test_done
^ permalink raw reply related
* Re: [PATCH] builtin-merge: give a proper error message for invalid strategies in config
From: Junio C Hamano @ 2008-07-22 5:01 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <1216656647-16897-1-git-send-email-vmiklos@frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> Till now 'git merge -s foobar' bailed out with an error message, but
> foobar in pull.twohead or pull.octopus was just silently ignored. It's
> better to inform the user then just silently doing nothing.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
Doesn't this make "git merge -s 'recursive resolve'" to misbehave?
^ permalink raw reply
* Re: [PATCH] Allow pager of diff command be enabled/disabled
From: Jeff King @ 2008-07-22 4:58 UTC (permalink / raw)
To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20080721212849.GB4748@blimp.local>
On Mon, Jul 21, 2008 at 11:28:49PM +0200, Alex Riesen wrote:
> See for example, status and show commands. Besides,
> Documentation/RelNotes-1.6.0.txt mentions that pager.<cmd>
> can be used to enable/disable paging behavior per command.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
>
> Also, really export check_pager_config, which just got another call
> site.
This patch looks correct to me. There are also a bunch of other commands
for which pager.* doesn't work. Basically, anything that doesn't ask for
RUN_SETUP is on its own to set up paging if it wants (because Bad Things
happen when we try to look in the config before setup has been run).
Most of them people probably don't care about (really, who wants paging
by default on git-apply?). But it is yet another hidden inconsistency
from the paging patch.
-Peff
^ permalink raw reply
* Re: [PATCH] Enable threaded delta search on *BSD and Linux.
From: Junio C Hamano @ 2008-07-22 4:54 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <1216632223-14655-1-git-send-email-madcoder@debian.org>
Pierre Habouzit <madcoder@debian.org> writes:
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>
> Following the discussion we had 10 days ago, here is a proposal to enable
> threaded delta search on systems that are likely to behave properly wrt
> memory and CPU usage.
Hmmm.
I do not want to do this kind of thing very close to the release (like
now), but rather immediately after 1.6.0. Will queue for 'next'.
Distro people can decide for themselves and their users, but I am of
conservative kind.
^ permalink raw reply
* Re: git status in clean working dir
From: Jeff King @ 2008-07-22 4:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Bremner
In-Reply-To: <20080722044359.GB20787@sigill.intra.peff.net>
On Tue, Jul 22, 2008 at 12:44:00AM -0400, Jeff King wrote:
> We could also swap the parent/child relationship, and have the pager as
> child. But I assume that it is done the way we have it because otherwise
> the shell gets confused about when the command ends (i.e., we want it to
> run until pager completion). I didn't test, though.
Hmm, it looks like the MINGW32 codepath already _does_ spawn in that
order, but has a "wait_for_child" atexit handler. I wonder if there is a
reason all platforms can't use that trick (though the mingw approach
uses run_command, which makes it harder to do the "wait for input before
starting less" trick).
-Peff
^ permalink raw reply
* Re: git status in clean working dir
From: Jeff King @ 2008-07-22 4:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Bremner
In-Reply-To: <7vljzur5wd.fsf@gitster.siamese.dyndns.org>
On Mon, Jul 21, 2008 at 07:48:02PM -0700, Junio C Hamano wrote:
> Another possibility is to set up an extra process whose sole purpose is to
> wait for the main process that feeds the pager pipe and relay its exit
> status to the outside world. But I do not think we would want to go
> there...
We could also swap the parent/child relationship, and have the pager as
child. But I assume that it is done the way we have it because otherwise
the shell gets confused about when the command ends (i.e., we want it to
run until pager completion). I didn't test, though.
-Peff
^ permalink raw reply
* Re: git status in clean working dir
From: Jeff King @ 2008-07-22 4:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Bremner
In-Reply-To: <7vtzeir68z.fsf@gitster.siamese.dyndns.org>
On Mon, Jul 21, 2008 at 07:40:28PM -0700, Junio C Hamano wrote:
> Actually, the situation is now even worse than I originally thought
> especially with Jeff's pager.<cmd> patch on 'master' recently. For
> example, you can screw yourself quite badly by forcing diff-files used in
> the scripts you run to page, defeating --exit-code option. Which means
Actually, you could _always_ do that with "git -p diff-files". Which is
obviously stupid, just as setting pager.diff-files is. In the reported
case, though, "status" is broken, which we now do by default. So no
stupidity required.
> (2) Then why are we even allowing to configure the plumbing to page?
1. Laziness. We just never marked which shouldn't be allowed to page.
But again, in this case, we have explicitly marked status as "this
should page" so I don't think this is a plumbing / porcelain thing.
Status fulfills both roles here (some people want it paged, because
they use it as porcelain, and some people want the exit code).
2. We don't always know all git commands. We execute user scripts as
"git foo", but we don't know what they do. Worse than that, we have
to commit our pager choice early because we might be exec'ing (but
this is somewhat of an artifact of the way the code is structured,
and not necessarily an impossible obstacle).
> Should we maintain a table of commands that we allow paging to be
> customized, and ignore pager.<cmd> for commands that are not in the list?
The patch below sets up the infrastructure, which is trivial. Note that
this _doesn't_ handle the case of "git -p status", because we have to
commit that choice at a different time (again, we might be able to
overcome that with a little code restructuring).
This marks diff-files as FORBID_PAGER; I will leave it to others to
fight about which commands should have it. But it doesn't make sense to
mark "status" since some people obviously _want_ the paging there.
> Which codepath should issue error messages when the user tries to break
> the system by saying "pager.diff-files = true"?
No error, but it is silently ignored. :)
---
diff --git a/git.c b/git.c
index 74ea0e6..72cadb5 100644
--- a/git.c
+++ b/git.c
@@ -210,6 +210,7 @@ const char git_version_string[] = GIT_VERSION;
* RUN_SETUP for reading from the configuration file.
*/
#define NEED_WORK_TREE (1<<2)
+#define FORBID_PAGER (1<<3)
struct cmd_struct {
const char *cmd;
@@ -231,6 +232,8 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
use_pager = check_pager_config(p->cmd);
if (use_pager == -1 && p->option & USE_PAGER)
use_pager = 1;
+ if (use_pager == 1 && p->option & FORBID_PAGER)
+ use_pager = 0;
commit_pager_choice();
if (p->option & NEED_WORK_TREE)
@@ -286,7 +289,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "count-objects", cmd_count_objects, RUN_SETUP },
{ "describe", cmd_describe, RUN_SETUP },
{ "diff", cmd_diff },
- { "diff-files", cmd_diff_files, RUN_SETUP },
+ { "diff-files", cmd_diff_files, RUN_SETUP | FORBID_PAGER },
{ "diff-index", cmd_diff_index, RUN_SETUP },
{ "diff-tree", cmd_diff_tree, RUN_SETUP },
{ "fast-export", cmd_fast_export, RUN_SETUP },
^ permalink raw reply related
* Git Documentation
From: Scott Chacon @ 2008-07-22 3:35 UTC (permalink / raw)
To: git
Hey all,
I'm starting a project to host a really nice, user-friendly, easy to
use Git learning materials website for Git newbies to get new users
started and make it as easy to learn as possible. I'll be redoing or
editing some of my screencasts from gitcasts.com and starting an open
book at github and putting it all in one place for new users to get
started easily. Anyone will be free to submit changes, additions,
etc.
If anyone has any tips on how they think git should be taught, issues
they are asked a lot, problems newbies tend to have, something they
wish there were a screencast for or was better documented, etc -
please do contact me so I can incorporate it. I would contribute to
git itself, but my C-foo is seriously wanting, so if by teaching
people properly I can free up some time for you guys, I would love to
do so.
Please let me know if you have any pointers or think anything should
really be better documented for end-users. I plan to do a lot of
graphics, screencasts and whatever else makes it as simple as
possible.
Thanks, and thanks again for git.
Scott Chacon
^ permalink raw reply
* Re: [PATCH] mailinfo: better parse email adresses containg parentheses
From: Junio C Hamano @ 2008-07-22 3:16 UTC (permalink / raw)
To: Philippe Bruhat (BooK); +Cc: git
In-Reply-To: <1216647269-12287-1-git-send-email-book@cpan.org>
"Philippe Bruhat (BooK)" <book@cpan.org> writes:
> When using git-rebase, author fields containing a ')' at the last
> position had the close-parens character incorrectly removed
> because the From: parser incorrectly matched it as
>
> user@host (User Name)
>
> (removing parentheses), instead of
>
> User Name (me) <user@host>
>
> Signed-off-by: Philippe Bruhat (BooK) <book@cpan.org>
Hmm, tests?
By the way, that second form parses like this:
mailbox =
name-addr =
display-name angle-addr = "User Name (me) <user@host>"
display-name =
phrase = "User Name"
angle-addr = CFWS "<" addr-spec ">" = "(me) <user@host>"
So strictly speaking, shouldn't we be stripping the whole (me) as garbage?
It is not even part of the display-name but is a whitespace equivalent
comment.
^ permalink raw reply
* Re: git status in clean working dir
From: Junio C Hamano @ 2008-07-22 2:48 UTC (permalink / raw)
To: Jeff King; +Cc: git, David Bremner
In-Reply-To: <7vtzeir68z.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Actually, the situation is now even worse than I originally thought
> especially with Jeff's pager.<cmd> patch on 'master' recently. For
> example, you can screw yourself quite badly by forcing diff-files used in
> the scripts you run to page, defeating --exit-code option. Which means
>
> (1) It hurts? Don't do it then; but
>
> (2) Then why are we even allowing to configure the plumbing to page?
>
> Should we maintain a table of commands that we allow paging to be
> customized, and ignore pager.<cmd> for commands that are not in the list?
>
> Which codepath should issue error messages when the user tries to break
> the system by saying "pager.diff-files = true"?
Another possibility is to set up an extra process whose sole purpose is to
wait for the main process that feeds the pager pipe and relay its exit
status to the outside world. But I do not think we would want to go
there...
^ permalink raw reply
* [PATCH] Revert "make git-status use a pager"
From: Junio C Hamano @ 2008-07-22 2:43 UTC (permalink / raw)
To: git
This reverts commit c8af1de9cfa0a5678ae766777e0f905e60b69fda.
The change was immensely unpopular, and poeple who would really want to
page can use pager.status configuration.
---
git.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git.c b/git.c
index 74ea0e6..1bfd271 100644
--- a/git.c
+++ b/git.c
@@ -341,7 +341,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "shortlog", cmd_shortlog, USE_PAGER },
{ "show-branch", cmd_show_branch, RUN_SETUP },
{ "show", cmd_show, RUN_SETUP | USE_PAGER },
- { "status", cmd_status, RUN_SETUP | NEED_WORK_TREE | USE_PAGER },
+ { "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
{ "stripspace", cmd_stripspace },
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
{ "tag", cmd_tag, RUN_SETUP },
--
1.6.0.rc0
^ permalink raw reply related
* Re: git status in clean working dir
From: Junio C Hamano @ 2008-07-22 2:40 UTC (permalink / raw)
To: Jeff King; +Cc: git, David Bremner
In-Reply-To: <7vy73ur6pz.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> git status -a
>>
>> exits with 0
>> ...
>
> Try "git status -a >/dev/null" or "git --no-pager status -a".
>
> I think this is an instance of the c8af1de (make git-status use a pager,
> 2008-04-23) stupidity raising its ugly head again.
>
> Do people mind reverting that patch?
Actually, the situation is now even worse than I originally thought
especially with Jeff's pager.<cmd> patch on 'master' recently. For
example, you can screw yourself quite badly by forcing diff-files used in
the scripts you run to page, defeating --exit-code option. Which means
(1) It hurts? Don't do it then; but
(2) Then why are we even allowing to configure the plumbing to page?
Should we maintain a table of commands that we allow paging to be
customized, and ignore pager.<cmd> for commands that are not in the list?
Which codepath should issue error messages when the user tries to break
the system by saying "pager.diff-files = true"?
^ permalink raw reply
* Re: git status in clean working dir
From: Abhijit Menon-Sen @ 2008-07-22 2:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Bremner, git
In-Reply-To: <7vy73ur6pz.fsf@gitster.siamese.dyndns.org>
At 2008-07-21 19:30:16 -0700, gitster@pobox.com wrote:
>
> I think this is an instance of the c8af1de (make git-status use a
> pager, 2008-04-23) stupidity raising its ugly head again.
>
> Do people mind reverting that patch?
I think that is an excellent idea. I've found myself annoyed time and
again by "git status" starting the pager for just a couple of lines of
output.
-- ams
^ 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