* 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: [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
* [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
* [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
* 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
* 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: [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: 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] 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: [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: 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
* [PATCH 1/2] run-command: add pre-exec callback
From: Jeff King @ 2008-07-22 7:12 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071009.GA3610@sigill.intra.peff.net>
This is a function provided by the caller which is called
_after_ the process is forked, but before the spawned
program is executed. On platforms (like mingw) where
subprocesses are forked and executed in a single call, the
preexec callback is simply ignored.
This will be used in the following patch to do some setup
for 'less' that must happen in the forked child.
Signed-off-by: Jeff King <peff@peff.net>
---
run-command.c | 2 ++
run-command.h | 1 +
2 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/run-command.c b/run-command.c
index 6e29fdf..73d0c31 100644
--- a/run-command.c
+++ b/run-command.c
@@ -110,6 +110,8 @@ int start_command(struct child_process *cmd)
unsetenv(*cmd->env);
}
}
+ if (cmd->preexec_cb)
+ cmd->preexec_cb();
if (cmd->git_cmd) {
execv_git_cmd(cmd->argv);
} else {
diff --git a/run-command.h b/run-command.h
index 5203a9e..4f2b7d7 100644
--- a/run-command.h
+++ b/run-command.h
@@ -42,6 +42,7 @@ struct child_process {
unsigned no_stderr:1;
unsigned git_cmd:1; /* if this is to be git sub-command */
unsigned stdout_to_stderr:1;
+ void (*preexec_cb)(void);
};
int start_command(struct child_process *);
--
1.6.0.rc0.1.g9291f.dirty
^ permalink raw reply related
* [PATCH 2/2] spawn pager via run_command interface
From: Jeff King @ 2008-07-22 7:14 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071246.GA3584@sigill.intra.peff.net>
This has two important effects:
1. The pager is now the _child_ process, instead of the
parent. This means that whatever spawned git (e.g., the
shell) will see the exit code of the git process, and
not the pager.
2. The mingw and regular code are now unified, which makes
the setup_pager function much simpler.
There are two caveats:
1. We used to call execlp directly on the pager, followed
by trying to exec it via the shall. We now just use the
shell (which is what mingw has always done). This may
have different results for pager names which contain
shell metacharacters.
It is also slightly less efficient because we
unnecessarily run the shell; however, pager spawning is
by definition an interactive task, so it shouldn't be
a huge problem.
2. The git process will remain in memory while the user
looks through the pager. This is potentially wasteful.
We could get around this by turning the parent into a
meta-process which spawns _both_ git and the pager,
collects the exit status from git, waits for both to
end, and then exits with git's exit code.
---
| 49 ++++++++-----------------------------------------
1 files changed, 8 insertions(+), 41 deletions(-)
--git a/pager.c b/pager.c
index 6b5c9e4..7743742 100644
--- a/pager.c
+++ b/pager.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "run-command.h"
/*
* This is split up from the rest of git so that we can do
@@ -8,7 +9,7 @@
static int spawned_pager;
#ifndef __MINGW32__
-static void run_pager(const char *pager)
+static void pager_preexec(void)
{
/*
* Work around bug in "less" by not starting it until we
@@ -20,16 +21,17 @@ static void run_pager(const char *pager)
FD_SET(0, &in);
select(1, &in, NULL, &in, NULL);
- execlp(pager, pager, NULL);
- execl("/bin/sh", "sh", "-c", pager, NULL);
+ setenv("LESS", "FRSX", 0);
}
-#else
-#include "run-command.h"
+#endif
static const char *pager_argv[] = { "sh", "-c", NULL, NULL };
static struct child_process pager_process = {
.argv = pager_argv,
- .in = -1
+ .in = -1,
+#ifndef __MINGW32__
+ .preexec_cb = pager_preexec,
+#endif
};
static void wait_for_pager(void)
{
@@ -40,14 +42,9 @@ static void wait_for_pager(void)
close(2);
finish_command(&pager_process);
}
-#endif
void setup_pager(void)
{
-#ifndef __MINGW32__
- pid_t pid;
- int fd[2];
-#endif
const char *pager = getenv("GIT_PAGER");
if (!isatty(1))
@@ -66,35 +63,6 @@ void setup_pager(void)
spawned_pager = 1; /* means we are emitting to terminal */
-#ifndef __MINGW32__
- if (pipe(fd) < 0)
- return;
- pid = fork();
- if (pid < 0) {
- close(fd[0]);
- close(fd[1]);
- return;
- }
-
- /* return in the child */
- if (!pid) {
- dup2(fd[1], 1);
- dup2(fd[1], 2);
- close(fd[0]);
- close(fd[1]);
- return;
- }
-
- /* The original process turns into the PAGER */
- dup2(fd[0], 0);
- close(fd[0]);
- close(fd[1]);
-
- setenv("LESS", "FRSX", 0);
- run_pager(pager);
- die("unable to execute pager '%s'", pager);
- exit(255);
-#else
/* spawn the pager */
pager_argv[2] = pager;
if (start_command(&pager_process))
@@ -107,7 +75,6 @@ void setup_pager(void)
/* this makes sure that the parent terminates after the pager */
atexit(wait_for_pager);
-#endif
}
int pager_in_use(void)
--
1.6.0.rc0.1.g9291f.dirty
^ permalink raw reply related
* Re: [PATCH 2/2] spawn pager via run_command interface
From: Jeff King @ 2008-07-22 7:16 UTC (permalink / raw)
To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071411.GB3584@sigill.intra.peff.net>
On Tue, Jul 22, 2008 at 03:14:12AM -0400, Jeff King wrote:
> static struct child_process pager_process = {
> .argv = pager_argv,
> - .in = -1
> + .in = -1,
> +#ifndef __MINGW32__
> + .preexec_cb = pager_preexec,
> +#endif
I couldn't recall if this initializer style is portable enough for us.
It was already there wrapped in ifdefs, but perhaps it was only ok
because the mingw version always uses the same compiler?
-Peff
^ permalink raw reply
* Error: "You have some suspicious patch lines"
From: Ben Aurel @ 2008-07-22 7:16 UTC (permalink / raw)
To: git
hi
I working on mac os x 10.5.4 (intel) with git version 1.5.5.3 and I
always get this message for most of my perl scripts and also for
"Makefile.pre" files:
----------- Message ---------------
* You have some suspicious patch lines:
*
* In src/scripts/trunk/3rdparty/file_sanity.pl
* trailing whitespace (line 52)
...
------------------------------------------
Editing '.git/hooks/pre-commit' and comment out the following lines
(around line 58):
--
58 if (/\s$/) {
59 bad_line("trailing whitespace", $_);
60 }
--
I still have this message
----------- Message ---------------
$ git commit .
*
* You have some suspicious patch lines:
*
* In src/scripts/trunk/3rdparty/file_sanity.pl
* indent SP followed by a TAB (line 112)
-----------------------------------------
Editing '.git/hooks/pre-commit' and comment out the following lines
--
61 if (/^\s* \t/) {
62 bad_line("indent SP followed by a TAB", $_);
63 }
--
And finally "git commit" works again.
The question now is: Is it really necessary to edit the git script
everytime? Is there a urgent reason why git refuses to commit because of
"suspicious" lines? Is it really necessary?
Thanks
ben
^ permalink raw reply
* Re: [PATCH] Fix update-index --refresh for submodules if stat(2) returns st_size 0
From: Johannes Sixt @ 2008-07-22 7:17 UTC (permalink / raw)
To: Alex Riesen, Junio C Hamano; +Cc: git
In-Reply-To: <20080721173511.GB5387@steel.home>
Alex Riesen schrieb:
> Can MSys folks please try it? I noticed it when the test
> t2103-update-index-ignore-missing.sh (the 5th case) started failing.
I tested it. mingw.git does suffer from the problem, and this fixes it.
But!
> + if ((changed & DATA_CHANGED) && (ce->ce_size != 0 || S_ISGITLINK(ce->ce_mode)))
Does this mean that ce->ce_size is non-zero for gitlinks, at least on
Unix? Is this value useful in anyway? I don't think so. Then it shouldn't
be a random value that lstat() happens to return.
-- Hannes
^ permalink raw reply
* Re: Git Documentation
From: Johan Herland @ 2008-07-22 7:17 UTC (permalink / raw)
To: Scott Chacon; +Cc: git
In-Reply-To: <d411cc4a0807212035v68c2ed95m93b77c1e61cfec9e@mail.gmail.com>
On Tuesday 22 July 2008, Scott Chacon wrote:
> 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.
You should at least take a look at this thread:
http://thread.gmane.org/gmane.comp.version-control.git/88698
(even though it goes off-topic after a while...)
> If anyone has any tips on how they think git should be taught...
It seems there are primarily two ways to teach Git:
1. Top-down: Start with simple use cases and commands. Teach people a
minimal, but necessary set of porcelain commands to get them started. Stay
_far_ away from plumbing commands and most of the command options.
2. Bottom-up: Start with how Git structures the data. Talk about blobs,
trees, commits, refs, how everything is connected, and how various Git
commands query and manipulate this structure. This _may_ involve a fair
amount of plumbing commands, especially when discovering how the more
complicated high-level commands manipulate the structure.
Some people seem to prefer the first approach, other people prefer the other
approach. Both paths lead to enlightenment ;). In many cases a bit of both
may be useful. HOWEVER, I think it is _very_ important to keep in mind that
these are two _different_ approaches, and the contexts in which they are
taught should be kept separate. I would almost suggest splitting your
website down the middle and make the difference between top-down and
bottom-up immediately visible with, say, a different background color, or
something else that immediately tells the user what "track" they are
following...
BTW, I think what you're doing is good and important (and I've already
enjoyed some of your gitcasts). Keep up the good work! :)
Have fun!
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH 2/9] Makefile: Normalize $(bindir) and $(gitexecdir) before comparing
From: Johannes Sixt @ 2008-07-22 7:25 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0807220147320.3407@eeepc-johanness>
Johannes Schindelin schrieb:
> Hi,
>
> On Mon, 21 Jul 2008, Johannes Sixt wrote:
>
>> + bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
>> + execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \
>
> These lack quotes, no?
No. RHS of an assignment doesn't need quotes as long as the shell syntax
makes clear where the assignment ends, which is the case here because of
the brackets $(...).
-- Hannes
^ permalink raw reply
* Re: Error: "You have some suspicious patch lines"
From: Jeff King @ 2008-07-22 7:26 UTC (permalink / raw)
To: Ben Aurel; +Cc: git
In-Reply-To: <4885895C.5050108@gmail.com>
On Tue, Jul 22, 2008 at 09:16:44AM +0200, Ben Aurel wrote:
> -----------------------------------------
>
> Editing '.git/hooks/pre-commit' and comment out the following lines
> --
> 61 if (/^\s* \t/) {
> 62 bad_line("indent SP followed by a TAB", $_);
> 63 }
> --
>
> And finally "git commit" works again.
>
> The question now is: Is it really necessary to edit the git script
> everytime? Is there a urgent reason why git refuses to commit because of
> "suspicious" lines? Is it really necessary?
The pre-commit hook that ships with git checks whitespace as an example
of what one _could_ do with hooks. It is not meant to be enabled by
default (unless you want that whitespace checking).
So either:
1. You enabled it by setting the execute bit. If so, then don't do
that.
2. Something is broken, and it has caused the hook to be enabled
accidentally. I recall somebody complaining that hooks were enabled
by default under cygwin because the filesystem didn't support the
execute bit. Are you working on an exec-bit challenged filesystem?
In newer versions of git, the hooks actually ship with a .sample
extension so that they will not be used accidentally, regardless of the
executable bit. In the meantime, it is safe to simply delete
.git/hooks/pre-commit if it is bothering you.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] spawn pager via run_command interface
From: Pierre Habouzit @ 2008-07-22 7:31 UTC (permalink / raw)
To: Jeff King; +Cc: Mike Hommey, Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071630.GA3669@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 827 bytes --]
On Tue, Jul 22, 2008 at 07:16:30AM +0000, Jeff King wrote:
> On Tue, Jul 22, 2008 at 03:14:12AM -0400, Jeff King wrote:
>
> > static struct child_process pager_process = {
> > .argv = pager_argv,
> > - .in = -1
> > + .in = -1,
> > +#ifndef __MINGW32__
> > + .preexec_cb = pager_preexec,
> > +#endif
>
> I couldn't recall if this initializer style is portable enough for us.
> It was already there wrapped in ifdefs, but perhaps it was only ok
> because the mingw version always uses the same compiler?
it's not, I asked long time ago, and it's C99, which mingw supports
indeed, and we don't want to require a C99 compiler.
--
·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: Johannes Sixt @ 2008-07-22 7:34 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722044157.GA20787@sigill.intra.peff.net>
Jeff King schrieb:
> @@ -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 },
Every now and then I want to use 'git -p diff-files', and I think that is
a valid use-case. But your suggested patch seems to forbid the pager even
in this case. :-(
-- Hannes
^ permalink raw reply
* Re: [PATCH] builtin-merge: give a proper error message for invalid strategies in config
From: Miklos Vajna @ 2008-07-22 7:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr69mpl5v.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]
On Mon, Jul 21, 2008 at 10:01:16PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Doesn't this make "git merge -s 'recursive resolve'" to misbehave?
Depends on what do we expect it to do. ;-)
My patch unified the handling of pull.twohead / -s option, so it
(actually unintentionally) made -s accepting multiple strategies as a
space separated list. Given that we already accept multiple strategies
as a space separated list in pull.twohead (and we do _not_ use multiple
pull.twohead entries) I think my patch is more logical.
Though, there was already a thread about how should we specify multiple
strategies on the commandline; and you suggested in
http://article.gmane.org/gmane.comp.version-control.git/89208
to use -s strategy1 -s strategy2.
In that case, I think your patch is better, and once it hits git.git, I
would like to send a patch that changes the config parsing as well, so
that pull.twohead "foo bar" would be invalid, and the user would have to
have two pull.twohead entries: one for foo and one for bar.
Does this sound reasonable?
Thanks.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] git-svn: make use of svn auto-props optional
From: Paul Talacko @ 2008-07-22 7:44 UTC (permalink / raw)
To: brad.king; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 558 bytes --]
Hello Brad,
I submitted a patch for this, including a test file a few days ago.
My recommendation was to follow the practice as set out by the svn command line tool, which was that git svn respect auto-props in the config file unless specifically overridden by --auto-props or --no-auto-props.
My patch is attached.
__________________________________________________________
Not happy with your email address?.
Get the one you really want - millions of new email addresses available now at Yahoo! http://uk.docs.yahoo.com/ymail/new.html
[-- Attachment #2: diff --]
[-- Type: application/octet-stream, Size: 8614 bytes --]
diff --git a/git-svn.perl b/git-svn.perl
index a366c89..df06220 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -128,6 +128,8 @@ my %cmd = (
'dry-run|n' => \$_dry_run,
'fetch-all|all' => \$_fetch_all,
'no-rebase' => \$_no_rebase,
+ 'auto-props' => \$SVN::Git::Editor::_auto_props,
+ 'no-auto-props' => \$SVN::Git::Editor::_no_auto_props,
%cmt_opts, %fc_opts } ],
'set-tree' => [ \&cmd_set_tree,
"Set an SVN repository to a git tree-ish",
@@ -448,8 +450,8 @@ sub cmd_dcommit {
log => get_commit_entry($d)->{log},
ra => Git::SVN::Ra->new($gs->full_url),
config => SVN::Core::config_get_config(
- $Git::SVN::Ra::config_dir
- ),
+ $Git::SVN::Ra::config_dir
+ ),
tree_a => "$d~1",
tree_b => $d,
editor_cb => sub {
@@ -3276,7 +3278,7 @@ sub close_edit {
}
package SVN::Git::Editor;
-use vars qw/@ISA $_rmdir $_cp_similarity $_find_copies_harder $_rename_limit/;
+use vars qw/@ISA $_rmdir $_cp_similarity $_find_copies_harder $_rename_limit $_auto_props $_no_auto_props/;
use strict;
use warnings;
use Carp qw/croak/;
@@ -3309,6 +3311,8 @@ sub new {
$self->{rm} = { };
$self->{path_prefix} = length $self->{svn_path} ?
"$self->{svn_path}/" : '';
+ $self->{config} = $opts->{ra}->{config};
+ croak "--auto-props and --no-auto-props are mutually exclusive." if $_auto_props && $_no_auto_props;
return $self;
}
@@ -3497,6 +3501,86 @@ sub ensure_path {
return $bat->{$c};
}
+sub apply_properties {
+ my ( $self, $fbat, $m ) = @_;
+ my $config = $self->{config}->{config};
+ my $svn_auto_prop = {};
+ return if $_no_auto_props;
+ return if ( ! $_auto_props ) && ( ! $config->get_bool ('miscellany', 'enable-auto-props', 0) );
+
+ my $file = $m->{ file_b };
+
+ $config->enumerate(
+ 'auto-props',
+ sub {
+ $svn_auto_prop->{ compile_apr_fnmatch( $_[0] ) } = $_[1];
+ 1;
+ }
+ );
+ my ( $filebase ) = File::Basename::fileparse( $file );
+ while (my ($pattern, $value) = each %$svn_auto_prop ) {
+ next unless $filebase =~ m/$pattern/;
+ for (split (/\s*;\s*/, $value)) {
+ my ($propname, $propvalue) = split (/\s*=\s*/, $_, 2);
+ $self->change_file_prop($fbat, $propname, $propvalue);
+ }
+ }
+
+}
+
+
+## Thanks to SVK::XD and the folks Best Practical Solutions, who in
+## turn based this on Barrie Slaymaker's Regexp::Shellish
+sub compile_apr_fnmatch {
+ my $re = shift;
+
+ $re =~ s@
+ ( \\.
+ | \[ # character class
+ [!^]? # maybe negation (^ and ! are both supported)
+ (?: (?:\\.|[^\\\]]) # one item
+ (?: - # possibly followed by a dash and another
+ (?:\\.|[^\\\]]))? # item
+ )* # 0 or more entries (zero case will be checked specially below)
+ (\]?) # if this ] doesn't match, that means we fell off end of string!
+ | .
+ )
+ @
+ if ( $1 eq '?' ) {
+ '.' ;
+ } elsif ( $1 eq '*' ) {
+ '.*' ;
+ } elsif ( substr($1, 0, 1) eq '[') {
+ if ($1 eq '[]') { # should never match
+ '[^\s\S]';
+ } elsif ($1 eq '[!]' or $1 eq '[^]') { # 0-length match
+ '';
+ } else {
+ my $temp = $1;
+ my $failed = $2 eq '';
+ if ($failed) {
+ '[^\s\S]';
+ } else {
+ $temp =~ s/(\\.|.)/$1 eq '-' ? '-' : quotemeta(substr($1, -1))/ges;
+ # the previous step puts in backslashes at beginning and end; remove them
+ $temp =~ s/^\\\[/[/;
+ $temp =~ s/\\\]$/]/;
+ # if it started with [^ or [!, it now starts with [\^ or [\!; fix.
+ $temp =~ s/^\[ # literal [
+ \\ # literal backslash
+ [!^] # literal ! or ^
+ /[^/x;
+ $temp;
+ }
+ }
+ } else {
+ quotemeta(substr( $1, -1 ) ); # ie, either quote it, or if it's \x, quote x
+ }
+ @gexs ;
+
+ return qr/\A$re\Z/s;
+}
+
sub A {
my ($self, $m) = @_;
my ($dir, $file) = split_path($m->{file_b});
@@ -3505,6 +3589,7 @@ sub A {
undef, -1);
print "\tA\t$m->{file_b}\n" unless $::_q;
$self->chg_file($fbat, $m);
+ $self->apply_properties( $fbat, $m );
$self->close_file($fbat,undef,$self->{pool});
}
diff --git a/t/t9124-git-svn-autoprops.sh b/t/t9124-git-svn-autoprops.sh
new file mode 100644
index 0000000..ed78c2d
--- /dev/null
+++ b/t/t9124-git-svn-autoprops.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+#
+
+
+
+test_description='git-svn dcommit sets autoprops on files'
+
+. ./lib-git-svn.sh
+
+test_expect_success 'make svn repo' '
+ mkdir import &&
+ cd import &&
+ echo first > firstfile &&
+ svn import -m "Import for autoprops test" . "$svnrepo" > /dev/null &&
+ cd .. &&
+ git svn init "$svnrepo" &&
+ git svn fetch
+'
+
+
+mkdir config
+cat > config/config <<EOF
+[miscellany]
+enable-auto-props = yes
+[auto-props]
+*pm = file-type = perl
+*html = svn:mime-type = text/html; encoding = special
+*bar = private = thingy
+EOF
+
+
+
+test_expect_success 'set svn properties on files' '
+ cd "$gittestrepo" &&
+ echo "blah" > a.pm &&
+ echo "foo" > b.html &&
+ echo "data" > foobar &&
+ git add a.pm b.html foobar &&
+ git commit -m files &&
+ git svn dcommit --config-dir=config
+ '
+
+test_expect_success 'export our properties to an svn repo' '
+
+ mkdir testsvnrepo &&
+ cd testsvnrepo &&
+ svn checkout "$svnrepo" &&
+ cd svnrepo
+ '
+
+test_expect_success 'test properties' '
+ test perl = `svn propget file-type a.pm` &&
+ test thingy = `svn propget private foobar` &&
+ test text/html = `svn propget svn:mime-type b.html` &&
+ test special = `svn propget encoding b.html`
+
+ '
+
+cd ../..
+
+test_expect_success 'no-props overrides config file' '
+ touch overriden-b.html &&
+ git add overriden-b.html &&
+ git commit -m "overriden-b" &&
+ git svn dcommit --no-auto-props --config-dir=config &&
+ cd testsvnrepo &&
+ svn checkout "$svnrepo" &&
+ cd svnrepo &&
+ test -z `svn propget file-type overriden-b.html`
+'
+
+cd ../..
+
+cat > config/config <<EOF
+[miscellany]
+enable-auto-props = no
+[auto-props]
+*pm = file-type = perl
+*html = svn:mime-type = text/html; encoding = special
+*bar = private = thingy
+EOF
+
+
+test_expect_success 'test when enable-auto-props is no' '
+ echo "blah" > a_no_props.pm &&
+ echo "foo" > b_no_props.html &&
+ echo "data" > foobar_no_props &&
+ chmod +x foobar_no_props &&
+ git add a_no_props.pm b_no_props.html foobar_no_props &&
+ git commit -m "No props files" &&
+ git svn dcommit --config-dir=config &&
+ cd testsvnrepo &&
+ svn checkout "$svnrepo" &&
+ cd svnrepo &&
+ test -z `svn propget file-type a_no_props.pm` &&
+ test -z `svn propget private foobar_no_props` &&
+ test -z `svn propget svn:mime-type b_no_props.html` &&
+ test -z `svn propget encoding b_no_props.html`
+ '
+
+cd ../..
+
+test_expect_success 'auto-props overrides config file' '
+ touch overriden-auto.pm &&
+ git add overriden-auto.pm &&
+ git commit -m "overriden-auto" &&
+ git svn dcommit --auto-props --config-dir=config &&
+ cd testsvnrepo &&
+ svn checkout "$svnrepo" &&
+ cd svnrepo &&
+ test perl = `svn propget file-type overriden-auto.pm`
+'
+cd ../..
+
+test_expect_success 'auto-props and no-auto-props are exclusive' '
+ touch afile &&
+ git add afile &&
+ git commit -m afile &&
+ test_must_fail git svn dcommit --auto-props --no-auto-props
+'
+
+test_done
+
^ permalink raw reply related
* Re: git status in clean working dir
From: Jeff King @ 2008-07-22 7:46 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <48858D95.7060409@viscovery.net>
On Tue, Jul 22, 2008 at 09:34:45AM +0200, Johannes Sixt wrote:
> > - { "diff-files", cmd_diff_files, RUN_SETUP },
> > + { "diff-files", cmd_diff_files, RUN_SETUP | FORBID_PAGER },
>
> Every now and then I want to use 'git -p diff-files', and I think that is
> a valid use-case. But your suggested patch seems to forbid the pager even
> in this case. :-(
Actually, it doesn't. If you read earlier in the message, this applies
only to pager.* config. That being said, I think Junio's ultimate goal
was to not allow stupid people to accidentally set the pager on
plumbing, at the expense of any smart people who might want to do it for
a good reason.
Though I have to wonder why "git diff --raw" is not enough for you.
At any rate, I think this isn't the right route. We haven't actually
seen evidence of somebody setting pager.diff-files and complaining about
breakage. We have seen people complaining about the lost exit code from
"git status", which is not something that would be on the "forbid" list
anyway. The real solution is to preserve the exit code when spawning the
pager, which I just posted a patch for.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] spawn pager via run_command interface
From: Johannes Sixt @ 2008-07-22 7:48 UTC (permalink / raw)
To: Jeff King; +Cc: Mike Hommey, Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071630.GA3669@sigill.intra.peff.net>
Jeff King schrieb:
> On Tue, Jul 22, 2008 at 03:14:12AM -0400, Jeff King wrote:
>
>> static struct child_process pager_process = {
>> .argv = pager_argv,
>> - .in = -1
>> + .in = -1,
>> +#ifndef __MINGW32__
>> + .preexec_cb = pager_preexec,
>> +#endif
>
> I couldn't recall if this initializer style is portable enough for us.
> It was already there wrapped in ifdefs, but perhaps it was only ok
> because the mingw version always uses the same compiler?
Yes, that's because on mingw we know that we use gcc. This really must be
changed for portability.
BTW, you could remove the #ifndef __MINGW32__ around both the definition
and the use of pager_preexec. We have everything on mingw to compile and
link this function.
-- Hannes
^ 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