Git development
 help / color / mirror / Atom feed
* Re: [RFC] Questions for "Git User's Survey 2011"
From: Jakub Narebski @ 2011-08-02 11:41 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: git
In-Reply-To: <20110801234323.GB31614@book.hvoigt.net>

On Tue, 2 Aug 2011, Heiko Voigt wrote:
> On Mon, Jul 25, 2011 at 10:33:01PM +0200, Jakub Narebski wrote:
> > === 17. Which of the following features would you like to see implemented in git? ===
> > (multiple choice)
[...]
> > NOTES:
> > ^^^^^^
> > What features should be mentioned besides those above?  What criteria
> > should we have for including features in this list?
> 
> How about adding:
> 
>  + improved submodule support
> 
>  ?

Added, thanks.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH v13 8/8] bisect: change bisect function to update BISECT_HEAD, rather than HEAD.
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312284545-2426-1-git-send-email-jon.seymour@gmail.com>

This function modifies git-bisect so that the --no-checkout option
uses BISECT_HEAD rather than HEAD to record the current bisection
head.

The intent is to reduce the confusion that uses of --no-checkout
may experience when using the --no-checkout option since the bisection
process will no longer introduce spurious differences between the
HEAD reference and the working tree and index.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 Documentation/git-bisect.txt |    8 ++++----
 bisect.c                     |    2 +-
 git-bisect.sh                |   17 +++++++++++++----
 t/t6030-bisect-porcelain.sh  |   30 +++++++++++++++---------------
 4 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 2014894..a9b217b 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -267,9 +267,9 @@ OPTIONS
 -------
 --no-checkout::
 +
-This option is used to specify that 'git bisect' should not modify the working
-tree or index on each iteration of the bisection process but should
-update HEAD instead.
+This option is used to specify that 'git bisect' should not checkout the
+new working tree at each iteration of the bisection process but should
+instead update BISECT_HEAD.
 +
 This option is useful in circumstances in which checkout is either not
 possible (because of a damaged respository) or is otherwise not required.
@@ -360,7 +360,7 @@ on a single line.
 $ git bisect start HEAD <known-good-commit> [ <boundary-commit> ... ] --no-checkout
 $ git bisect run eval '
 rc=1;
-if git rev-list --objects HEAD >tmp.$$; then
+if git rev-list --objects BISECT_HEAD >tmp.$$; then
    git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
 fi;
 rm tmp.$$;
diff --git a/bisect.c b/bisect.c
index 0427117..46874be 100644
--- a/bisect.c
+++ b/bisect.c
@@ -24,7 +24,7 @@ struct argv_array {
 
 static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
 static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
-static const char *argv_update_ref[] = {"update-ref", "--no-deref", "HEAD", NULL, NULL};
+static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD", NULL, NULL};
 
 /* bits #0-15 in revision.h */
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 5f3c2c4..c9aabc0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -36,6 +36,15 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 
 BISECT_MODE=$(test -f "$GIT_DIR/BISECT_MODE" && cat "$GIT_DIR/BISECT_MODE")
 
+bisect_head()
+{
+    if test "$BISECT_MODE" = "update-ref"; then
+	echo BISECT_HEAD;
+    else
+	echo HEAD
+    fi
+}
+
 bisect_autostart() {
 	test -s "$GIT_DIR/BISECT_START" || {
 		(
@@ -116,7 +125,7 @@ bisect_start() {
 		# Reset to the rev from where we started.
 		start_head=$(cat "$GIT_DIR/BISECT_START")
 		if test "$BISECT_MODE" = "update-ref"; then
-		    git update-ref --no-deref HEAD "$start_head"
+		    git update-ref --no-deref $(bisect_head) "$start_head"
 		else
 		    git checkout "$start_head" --
 		fi
@@ -219,8 +228,8 @@ bisect_state() {
 	0,*)
 		die "$(gettext "Please call 'bisect_state' with at least one argument.")" ;;
 	1,bad|1,good|1,skip)
-		rev=$(git rev-parse --verify HEAD) ||
-			die "$(gettext "Bad rev input: HEAD")"
+		rev=$(git rev-parse --verify $(bisect_head)) ||
+			die "$(gettext "Bad rev input: $(bisect_head)")"
 		bisect_write "$state" "$rev"
 		check_expected_revs "$rev" ;;
 	2,bad|*,good|*,skip)
@@ -354,7 +363,7 @@ bisect_reset() {
 	    usage ;;
 	esac
 	if test "$BISECT_MODE" = "update-ref"; then
-		git symbolic-ref HEAD $(git rev-parse --symbolic-full-name "${branch}")
+		git symbolic-ref $(bisect_head) $(git rev-parse --symbolic-full-name "${branch}")
 	else
 		if git checkout "$branch" --; then
 			bisect_clean_state
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index a1e0ddc..ef422a1 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -641,24 +641,24 @@ test_expect_success 'bisect fails if tree is broken on trial commit' '
 test_expect_success 'bisect: --no-checkout - start commit bad' '
 	git bisect reset &&
 	git bisect start BROKEN_HASH7 BROKEN_HASH4 --no-checkout &&
-	check_same BROKEN_HASH6 HEAD &&
+	check_same BROKEN_HASH6 BISECT_HEAD &&
 	git bisect reset
 '
 
 test_expect_success 'bisect: --no-checkout - trial commit bad' '
 	git bisect reset &&
 	git bisect start broken BROKEN_HASH4 --no-checkout &&
-	check_same BROKEN_HASH6 HEAD &&
+	check_same BROKEN_HASH6 BISECT_HEAD &&
 	git bisect reset
 '
 
 test_expect_success 'bisect: --no-checkout - target before breakage' '
 	git bisect reset &&
 	git bisect start broken BROKEN_HASH4 --no-checkout &&
-	check_same BROKEN_HASH6 HEAD &&
-	git bisect bad HEAD &&
-	check_same BROKEN_HASH5 HEAD &&
-	git bisect bad HEAD &&
+	check_same BROKEN_HASH6 BISECT_HEAD &&
+	git bisect bad BISECT_HEAD &&
+	check_same BROKEN_HASH5 BISECT_HEAD &&
+	git bisect bad BISECT_HEAD &&
 	check_same BROKEN_HASH5 bisect/bad &&
 	git bisect reset
 '
@@ -666,10 +666,10 @@ test_expect_success 'bisect: --no-checkout - target before breakage' '
 test_expect_success 'bisect: --no-checkout - target in breakage' '
 	git bisect reset &&
 	git bisect start broken BROKEN_HASH4 --no-checkout &&
-	check_same BROKEN_HASH6 HEAD &&
-	git bisect bad HEAD &&
-	check_same BROKEN_HASH5 HEAD &&
-	git bisect good HEAD &&
+	check_same BROKEN_HASH6 BISECT_HEAD &&
+	git bisect bad BISECT_HEAD &&
+	check_same BROKEN_HASH5 BISECT_HEAD &&
+	git bisect good BISECT_HEAD &&
 	check_same BROKEN_HASH6 bisect/bad &&
 	git bisect reset
 '
@@ -677,10 +677,10 @@ test_expect_success 'bisect: --no-checkout - target in breakage' '
 test_expect_success 'bisect: --no-checkout - target after breakage' '
 	git bisect reset &&
 	git bisect start broken BROKEN_HASH4 --no-checkout &&
-	check_same BROKEN_HASH6 HEAD &&
-	git bisect good HEAD &&
-	check_same BROKEN_HASH8 HEAD &&
-	git bisect good HEAD &&
+	check_same BROKEN_HASH6 BISECT_HEAD &&
+	git bisect good BISECT_HEAD &&
+	check_same BROKEN_HASH8 BISECT_HEAD &&
+	git bisect good BISECT_HEAD &&
 	check_same BROKEN_HASH9 bisect/bad &&
 	git bisect reset
 '
@@ -691,7 +691,7 @@ test_expect_success 'bisect: demonstrate identification of damage boundary' "
 	git bisect start broken master --no-checkout &&
 	git bisect run eval '
 rc=1;
-if git rev-list --objects HEAD >tmp.$$; then
+if git rev-list --objects BISECT_HEAD >tmp.$$; then
    git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
 fi;
 rm tmp.$$;
-- 
1.7.6.353.g3461

^ permalink raw reply related

* [PATCH v13 7/8] bisect: add documentation for --no-checkout option.
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312284545-2426-1-git-send-email-jon.seymour@gmail.com>

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 Documentation/git-bisect.txt |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index ab60a18..2014894 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -17,7 +17,7 @@ The command takes various subcommands, and different options depending
 on the subcommand:
 
  git bisect help
- git bisect start [<bad> [<good>...]] [--] [<paths>...]
+ git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<paths>...]
  git bisect bad [<rev>]
  git bisect good [<rev>...]
  git bisect skip [(<rev>|<range>)...]
@@ -263,6 +263,17 @@ rewind the tree to the pristine state.  Finally the script should exit
 with the status of the real test to let the "git bisect run" command loop
 determine the eventual outcome of the bisect session.
 
+OPTIONS
+-------
+--no-checkout::
++
+This option is used to specify that 'git bisect' should not modify the working
+tree or index on each iteration of the bisection process but should
+update HEAD instead.
++
+This option is useful in circumstances in which checkout is either not
+possible (because of a damaged respository) or is otherwise not required.
+
 EXAMPLES
 --------
 
@@ -343,6 +354,25 @@ $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
 This shows that you can do without a run script if you write the test
 on a single line.
 
+* Locate a good region of the object graph in a damaged repository
++
+------------
+$ git bisect start HEAD <known-good-commit> [ <boundary-commit> ... ] --no-checkout
+$ git bisect run eval '
+rc=1;
+if git rev-list --objects HEAD >tmp.$$; then
+   git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
+fi;
+rm tmp.$$;
+test $rc -eq 0;'
+
+------------
++
+In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit that
+has at least one parent whose reachable graph is fully traversable in the sense
+required by 'git pack objects'.
+
+
 SEE ALSO
 --------
 link:git-bisect-lk2009.html[Fighting regressions with git bisect],
-- 
1.7.6.353.g3461

^ permalink raw reply related

* [PATCH v13 6/8] bisect: add tests for the --no-checkout option.
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312284545-2426-1-git-send-email-jon.seymour@gmail.com>

These tests verify that git-bisect --no-checkout can successfully
bisect commit histories that reference damaged trees.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 t/t6030-bisect-porcelain.sh |   70 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9ae2de8..a1e0ddc 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -616,6 +616,14 @@ cat > expected.missing-tree.default <<EOF
 fatal: unable to read tree 39f7e61a724187ab767d2e08442d9b6b9dab587d
 EOF
 
+check_same()
+{
+    echo "Checking $1 is the same as $2" &&
+    git rev-parse "$1" > expected.same &&
+    git rev-parse "$2" > expected.actual &&
+    test_cmp expected.same expected.actual
+}
+
 test_expect_success 'bisect fails if tree is broken on start commit' '
 	git bisect reset &&
 	test_must_fail git bisect start BROKEN_HASH7 BROKEN_HASH4 2>error.txt &&
@@ -630,4 +638,66 @@ test_expect_success 'bisect fails if tree is broken on trial commit' '
 	test_cmp expected.missing-tree.default error.txt
 '
 
+test_expect_success 'bisect: --no-checkout - start commit bad' '
+	git bisect reset &&
+	git bisect start BROKEN_HASH7 BROKEN_HASH4 --no-checkout &&
+	check_same BROKEN_HASH6 HEAD &&
+	git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - trial commit bad' '
+	git bisect reset &&
+	git bisect start broken BROKEN_HASH4 --no-checkout &&
+	check_same BROKEN_HASH6 HEAD &&
+	git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target before breakage' '
+	git bisect reset &&
+	git bisect start broken BROKEN_HASH4 --no-checkout &&
+	check_same BROKEN_HASH6 HEAD &&
+	git bisect bad HEAD &&
+	check_same BROKEN_HASH5 HEAD &&
+	git bisect bad HEAD &&
+	check_same BROKEN_HASH5 bisect/bad &&
+	git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target in breakage' '
+	git bisect reset &&
+	git bisect start broken BROKEN_HASH4 --no-checkout &&
+	check_same BROKEN_HASH6 HEAD &&
+	git bisect bad HEAD &&
+	check_same BROKEN_HASH5 HEAD &&
+	git bisect good HEAD &&
+	check_same BROKEN_HASH6 bisect/bad &&
+	git bisect reset
+'
+
+test_expect_success 'bisect: --no-checkout - target after breakage' '
+	git bisect reset &&
+	git bisect start broken BROKEN_HASH4 --no-checkout &&
+	check_same BROKEN_HASH6 HEAD &&
+	git bisect good HEAD &&
+	check_same BROKEN_HASH8 HEAD &&
+	git bisect good HEAD &&
+	check_same BROKEN_HASH9 bisect/bad &&
+	git bisect reset
+'
+
+test_expect_success 'bisect: demonstrate identification of damage boundary' "
+	git bisect reset &&
+	git checkout broken &&
+	git bisect start broken master --no-checkout &&
+	git bisect run eval '
+rc=1;
+if git rev-list --objects HEAD >tmp.$$; then
+   git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
+fi;
+rm tmp.$$;
+test \$rc -eq 0;' &&
+	check_same BROKEN_HASH6 bisect/bad &&
+	git bisect reset
+"
+
 test_done
-- 
1.7.6.353.g3461

^ permalink raw reply related

* [PATCH v13 5/8] bisect: introduce --no-checkout support into porcelain.
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312284545-2426-1-git-send-email-jon.seymour@gmail.com>

git-bisect can now perform bisection of a history without performing
a checkout at each stage of the bisection process. Instead, HEAD is updated.

One use-case for this function is allow git bisect to be used with
damaged repositories where git checkout would fail because the tree
referenced by the commit is damaged.

It can also be used in other cases where actual checkout of the tree
is not required to progress the bisection.

Improved-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-bisect.sh |   32 +++++++++++++++++++++++++-------
 1 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index a44ffe1..5f3c2c4 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -3,7 +3,7 @@
 USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
         print this long help message.
-git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
+git bisect start [--no-checkout] [<bad> [<good>...]] [--] [<pathspec>...]
         reset bisect state and start bisection.
 git bisect bad [<rev>]
         mark <rev> a known-bad revision.
@@ -34,6 +34,8 @@ require_work_tree
 _x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
 _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 
+BISECT_MODE=$(test -f "$GIT_DIR/BISECT_MODE" && cat "$GIT_DIR/BISECT_MODE")
+
 bisect_autostart() {
 	test -s "$GIT_DIR/BISECT_START" || {
 		(
@@ -69,6 +71,7 @@ bisect_start() {
 	orig_args=$(git rev-parse --sq-quote "$@")
 	bad_seen=0
 	eval=''
+	BISECT_MODE=checkout
 	while [ $# -gt 0 ]; do
 	    arg="$1"
 	    case "$arg" in
@@ -76,6 +79,11 @@ bisect_start() {
 		shift
 		break
 		;;
+	    --no-checkout)
+		BISECT_MODE=update-ref;
+		shift ;;
+	    --*)
+		die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
 	    *)
 		rev=$(git rev-parse -q --verify "$arg^{commit}") || {
 		    test $has_double_dash -eq 1 &&
@@ -107,7 +115,11 @@ bisect_start() {
 	then
 		# Reset to the rev from where we started.
 		start_head=$(cat "$GIT_DIR/BISECT_START")
-		git checkout "$start_head" -- || exit
+		if test "$BISECT_MODE" = "update-ref"; then
+		    git update-ref --no-deref HEAD "$start_head"
+		else
+		    git checkout "$start_head" --
+		fi
 	else
 		# Get rev from where we start.
 		case "$head" in
@@ -144,7 +156,8 @@ bisect_start() {
 	# Write new start state.
 	#
 	echo "$start_head" >"$GIT_DIR/BISECT_START" &&
-	git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
+	git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES"
+	echo "$BISECT_MODE" > "$GIT_DIR/BISECT_MODE" &&
 	eval "$eval true" &&
 	echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
 	#
@@ -291,7 +304,7 @@ bisect_next() {
 	bisect_next_check good
 
 	# Perform all bisection computation, display and checkout
-	git bisect--helper --next-all
+	git bisect--helper --next-all ${BISECT_MODE:+--bisect-mode=}${BISECT_MODE}
 	res=$?
 
         # Check if we should exit because bisection is finished
@@ -340,11 +353,15 @@ bisect_reset() {
 	*)
 	    usage ;;
 	esac
-	if git checkout "$branch" -- ; then
-		bisect_clean_state
+	if test "$BISECT_MODE" = "update-ref"; then
+		git symbolic-ref HEAD $(git rev-parse --symbolic-full-name "${branch}")
 	else
-		die "$(eval_gettext "Could not check out original HEAD '\$branch'.
+		if git checkout "$branch" --; then
+			bisect_clean_state
+		else
+			die "$(eval_gettext "Could not check out original HEAD '\$branch'.
 Try 'git bisect reset <commit>'.")"
+		fi
 	fi
 }
 
@@ -360,6 +377,7 @@ bisect_clean_state() {
 	rm -f "$GIT_DIR/BISECT_LOG" &&
 	rm -f "$GIT_DIR/BISECT_NAMES" &&
 	rm -f "$GIT_DIR/BISECT_RUN" &&
+	rm -f "$GIT_DIR/BISECT_MODE" &&
 	# Cleanup head-name if it got left by an old version of git-bisect
 	rm -f "$GIT_DIR/head-name" &&
 
-- 
1.7.6.353.g3461

^ permalink raw reply related

* [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312284545-2426-1-git-send-email-jon.seymour@gmail.com>

If --no-checkout is specified, then the bisection process uses:

	git update-ref --no-deref HEAD <trial>

at each trial instead of:

	git checkout <trial>

Improved-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 bisect.c                 |   33 ++++++++++++++++++++++-----------
 bisect.h                 |    2 +-
 builtin/bisect--helper.c |    9 +++++++--
 3 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/bisect.c b/bisect.c
index dd7e8ed..0427117 100644
--- a/bisect.c
+++ b/bisect.c
@@ -24,6 +24,7 @@ struct argv_array {
 
 static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
 static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
+static const char *argv_update_ref[] = {"update-ref", "--no-deref", "HEAD", NULL, NULL};
 
 /* bits #0-15 in revision.h */
 
@@ -707,16 +708,23 @@ static void mark_expected_rev(char *bisect_rev_hex)
 		die("closing file %s: %s", filename, strerror(errno));
 }
 
-static int bisect_checkout(char *bisect_rev_hex)
+static int bisect_checkout(char *bisect_rev_hex, int no_checkout)
 {
 	int res;
 
 	mark_expected_rev(bisect_rev_hex);
 
 	argv_checkout[2] = bisect_rev_hex;
-	res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
-	if (res)
-		exit(res);
+	if (no_checkout) {
+		argv_update_ref[3] = bisect_rev_hex;
+		if (run_command_v_opt(argv_update_ref, RUN_GIT_CMD))
+			die("update-ref --no-deref HEAD failed on %s",
+			    bisect_rev_hex);
+	} else {
+		res = run_command_v_opt(argv_checkout, RUN_GIT_CMD);
+		if (res)
+			exit(res);
+	}
 
 	argv_show_branch[1] = bisect_rev_hex;
 	return run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
@@ -788,7 +796,7 @@ static void handle_skipped_merge_base(const unsigned char *mb)
  * - If one is "skipped", we can't know but we should warn.
  * - If we don't know, we should check it out and ask the user to test.
  */
-static void check_merge_bases(void)
+static void check_merge_bases(int no_checkout)
 {
 	struct commit_list *result;
 	int rev_nr;
@@ -806,7 +814,7 @@ static void check_merge_bases(void)
 			handle_skipped_merge_base(mb);
 		} else {
 			printf("Bisecting: a merge base must be tested\n");
-			exit(bisect_checkout(sha1_to_hex(mb)));
+			exit(bisect_checkout(sha1_to_hex(mb), no_checkout));
 		}
 	}
 
@@ -849,7 +857,7 @@ static int check_ancestors(const char *prefix)
  * If a merge base must be tested by the user, its source code will be
  * checked out to be tested by the user and we will exit.
  */
-static void check_good_are_ancestors_of_bad(const char *prefix)
+static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout)
 {
 	const char *filename = git_path("BISECT_ANCESTORS_OK");
 	struct stat st;
@@ -868,7 +876,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix)
 
 	/* Check if all good revs are ancestor of the bad rev. */
 	if (check_ancestors(prefix))
-		check_merge_bases();
+		check_merge_bases(no_checkout);
 
 	/* Create file BISECT_ANCESTORS_OK. */
 	fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);
@@ -908,8 +916,11 @@ static void show_diff_tree(const char *prefix, struct commit *commit)
  * We use the convention that exiting with an exit code 10 means that
  * the bisection process finished successfully.
  * In this case the calling shell script should exit 0.
+ *
+ * If no_checkout is non-zero, the bisection process does not
+ * checkout the trial commit but instead simply updates HEAD.
  */
-int bisect_next_all(const char *prefix)
+int bisect_next_all(const char *prefix, int no_checkout)
 {
 	struct rev_info revs;
 	struct commit_list *tried;
@@ -920,7 +931,7 @@ int bisect_next_all(const char *prefix)
 	if (read_bisect_refs())
 		die("reading bisect refs failed");
 
-	check_good_are_ancestors_of_bad(prefix);
+	check_good_are_ancestors_of_bad(prefix, no_checkout);
 
 	bisect_rev_setup(&revs, prefix, "%s", "^%s", 1);
 	revs.limited = 1;
@@ -966,6 +977,6 @@ int bisect_next_all(const char *prefix)
 	       "(roughly %d step%s)\n", nr, (nr == 1 ? "" : "s"),
 	       steps, (steps == 1 ? "" : "s"));
 
-	return bisect_checkout(bisect_rev_hex);
+	return bisect_checkout(bisect_rev_hex, no_checkout);
 }
 
diff --git a/bisect.h b/bisect.h
index 0862ce5..22f2e4d 100644
--- a/bisect.h
+++ b/bisect.h
@@ -27,7 +27,7 @@ struct rev_list_info {
 	const char *header_prefix;
 };
 
-extern int bisect_next_all(const char *prefix);
+extern int bisect_next_all(const char *prefix, int no_checkout);
 
 extern int estimate_bisect_steps(int all);
 
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 5b22639..5cfac13 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -4,16 +4,20 @@
 #include "bisect.h"
 
 static const char * const git_bisect_helper_usage[] = {
-	"git bisect--helper --next-all",
+	"git bisect--helper --next-all [--bisect-mode=checkout|update-ref]",
 	NULL
 };
 
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	int next_all = 0;
+	int no_checkout = 0;
+	char *bisect_mode=NULL;
 	struct option options[] = {
 		OPT_BOOLEAN(0, "next-all", &next_all,
 			    "perform 'git bisect next'"),
+		OPT_STRING(0, "bisect-mode", &bisect_mode, "mode",
+			    "the bisection mode either checkout or update-ref. defaults to checkout."),
 		OPT_END()
 	};
 
@@ -23,6 +27,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	if (!next_all)
 		usage_with_options(git_bisect_helper_usage, options);
 
+	no_checkout = bisect_mode && !strcmp(bisect_mode, "update-ref");
 	/* next-all */
-	return bisect_next_all(prefix);
+	return bisect_next_all(prefix, no_checkout);
 }
-- 
1.7.6.353.g3461

^ permalink raw reply related

* [PATCH v13 3/8] bisect: add tests to document expected behaviour in presence of broken trees.
From: Jon Seymour @ 2011-08-02 11:29 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312284545-2426-1-git-send-email-jon.seymour@gmail.com>

If the repo is broken, we expect bisect to fail.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 t/t6030-bisect-porcelain.sh |   48 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index b3d1b14..9ae2de8 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -581,5 +581,53 @@ test_expect_success 'erroring out when using bad path parameters' '
 '
 
 #
+# This creates a broken branch which cannot be checked out because
+# the tree created has been deleted.
 #
+# H1-H2-H3-H4-H5-H6-H7  <--other
+#            \
+#             S5-S6'-S7'-S8'-S9  <--broken
+#
+# Commits marked with ' have a missing tree.
+#
+test_expect_success 'broken branch creation' '
+	git bisect reset &&
+	git checkout -b broken $HASH4 &&
+	git tag BROKEN_HASH4 $HASH4 &&
+	add_line_into_file "5(broken): first line on a broken branch" hello2 &&
+	git tag BROKEN_HASH5 &&
+	mkdir missing &&
+	:> missing/MISSING &&
+	git add missing/MISSING &&
+	git commit -m "6(broken): Added file that will be deleted"
+	git tag BROKEN_HASH6 &&
+	add_line_into_file "7(broken): second line on a broken branch" hello2 &&
+	git tag BROKEN_HASH7 &&
+	add_line_into_file "8(broken): third line on a broken branch" hello2 &&
+	git tag BROKEN_HASH8 &&
+	git rm missing/MISSING &&
+	git commit -m "9(broken): Remove missing file"
+	git tag BROKEN_HASH9 &&
+	rm .git/objects/39/f7e61a724187ab767d2e08442d9b6b9dab587d
+'
+
+echo "" > expected.ok
+cat > expected.missing-tree.default <<EOF
+fatal: unable to read tree 39f7e61a724187ab767d2e08442d9b6b9dab587d
+EOF
+
+test_expect_success 'bisect fails if tree is broken on start commit' '
+	git bisect reset &&
+	test_must_fail git bisect start BROKEN_HASH7 BROKEN_HASH4 2>error.txt &&
+	test_cmp expected.missing-tree.default error.txt
+'
+
+test_expect_success 'bisect fails if tree is broken on trial commit' '
+	git bisect reset &&
+	test_must_fail git bisect start BROKEN_HASH9 BROKEN_HASH4 2>error.txt &&
+	git reset --hard broken &&
+	git checkout broken &&
+	test_cmp expected.missing-tree.default error.txt
+'
+
 test_done
-- 
1.7.6.353.g3461

^ permalink raw reply related

* [PATCH v13 2/8] bisect: use && to connect statements that are deferred with eval.
From: Jon Seymour @ 2011-08-02 11:28 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312284545-2426-1-git-send-email-jon.seymour@gmail.com>

Christian Couder pointed out that the existing eval strategy
swallows an initial non-zero return. Using && to connect
the statements should fix this.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-bisect.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index 20f6dd5..a44ffe1 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -86,7 +86,7 @@ bisect_start() {
 		0) state='bad' ; bad_seen=1 ;;
 		*) state='good' ;;
 		esac
-		eval="$eval bisect_write '$state' '$rev' 'nolog'; "
+		eval="$eval bisect_write '$state' '$rev' 'nolog' &&"
 		shift
 		;;
 	    esac
@@ -145,7 +145,7 @@ bisect_start() {
 	#
 	echo "$start_head" >"$GIT_DIR/BISECT_START" &&
 	git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" &&
-	eval "$eval" &&
+	eval "$eval true" &&
 	echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit
 	#
 	# Check if we can proceed to the next bisect state.
-- 
1.7.6.353.g3461

^ permalink raw reply related

* [PATCH v13 0/8] bisect: Add support for --no-checkout option
From: Jon Seymour @ 2011-08-02 11:28 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour

Motivation
==========
For some bisection tasks, checking out the commit at each stage of the bisection process is unecessary or undesirable.

This series adds support for a --no-checkout option to git-bisect.

If specified on a start command, --no-checkout causes 'git bisect' to update HEAD at each stage of the bisection process instead of checking out the commit at that point. 

One application of the --no-checkout option is to find, within a partially damaged repository, a commit that has at least one parent whose graph is fully reachable in the sense of 'git pack-objects'.

For example:

	git bisect start BISECT_HEAD <some-known-good-commit> <boundary-commits> --no-checkout
	git bisect run eval '
		rc=1;
		if git rev-list --objects BISECT_HEAD >tmp.$$; then
		   git pack-objects --stdout >/dev/null < tmp.$$ && rc=0;
		fi;
		rm tmp.$$;
		test $rc -eq 0;'

<some-known-good-commit> is a known good commit, for which the test passes.
<boundary-commits> are commits chosen to prevent the bisection visiting missing or corrupt commit objects.

Assuming this git bisect run completes successfully, bisect/bad will refer to a commit which has at least one parent that is fully reachable in the sense of 'git pack-objects'.

Patch Synopsis
==============

Remediation
-----------
Patch 1/8 changes existing behaviour in the case that an invalid revision argument is supplied to 'git bisect start'. In particular, in this case, bisection state is neither created or modified if argument validation fails. Previously, existing bisection state would be cleared even if the revision arguments were subsequently determined to be invalid. 	

Patch 2/8 remediates a potential flaw that might hide a failure in a chain of pasted statements.

Patch 3/8 adds a test which documents the existing behaviour of git bisect in the presence of tree damage.

New Function
------------
Patch 4/8 modifies the C code that supports bisection.
Patch 5/8 modifies porcelain to enable option exposed by 4/8.
Patch 6/8 adds some tests.
Patch 7/8 adds some documentation.
Patch 8/8 --no-checkout bisections now update BISECT_HEAD rather than HEAD.

Revision History
----------------
v13:
	Following suggestions from Junio:
	 * Replaced BISECT_NO_CHECKOUT control file with BISECT_MODE. 
	 * Changed name of internal option on bisect--helper from --no-checkout to --bisect-mode=checkout|update-ref.
	 * Changed --no-checkout bisections to update BISECT_HEAD instead of HEAD.	
v11:
	Removed support for --update-ref=<ref>, per Junio's preference.
v10:
	Changed the way deferred statements are connected. Reverted some whitespace minimization.
v8:
	Further feedback from Christian Couder. Support --update-ref <ref>.
v6: 
	This series includes numerous improvements suggested by Christian Couder.
Reworks: 
	"bisect: allow git bisect to be used with repos containing damaged trees." 
	Replaced --ignore-checkout-failure with --no-checkout option suggested by Junio.

Jon Seymour (8):
  bisect: move argument parsing before state modification.
  bisect: use && to connect statements that are deferred with eval.
  bisect: add tests to document expected behaviour in presence of
    broken trees.
  bisect: introduce support for --no-checkout option.
  bisect: introduce --no-checkout support into porcelain.
  bisect: add tests for the --no-checkout option.
  bisect: add documentation for --no-checkout option.
  bisect: change bisect function to update BISECT_HEAD, rather than
    HEAD.

 Documentation/git-bisect.txt |   32 ++++++++++-
 bisect.c                     |   33 +++++++----
 bisect.h                     |    2 +-
 builtin/bisect--helper.c     |    9 ++-
 git-bisect.sh                |  113 ++++++++++++++++++++++--------------
 t/t6030-bisect-porcelain.sh  |  132 +++++++++++++++++++++++++++++++++++++++++-
 6 files changed, 260 insertions(+), 61 deletions(-)

-- 
1.7.6.353.g3461

^ permalink raw reply

* [PATCH v13 1/8] bisect: move argument parsing before state modification.
From: Jon Seymour @ 2011-08-02 11:28 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312284545-2426-1-git-send-email-jon.seymour@gmail.com>

Currently 'git bisect start' modifies some state prior to checking
that its arguments are valid.

This change moves argument validation before state modification
with the effect that state modification does not occur
unless argument validations succeeds.

An existing test is changed to check that new bisect state
is not created if arguments are invalid.

A new test is added to check that existing bisect state
is not modified if arguments are invalid.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-bisect.sh               |   66 +++++++++++++++++++++---------------------
 t/t6030-bisect-porcelain.sh |   14 +++++++--
 2 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/git-bisect.sh b/git-bisect.sh
index b2186a8..20f6dd5 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -60,6 +60,39 @@ bisect_autostart() {
 
 bisect_start() {
 	#
+	# Check for one bad and then some good revisions.
+	#
+	has_double_dash=0
+	for arg; do
+	    case "$arg" in --) has_double_dash=1; break ;; esac
+	done
+	orig_args=$(git rev-parse --sq-quote "$@")
+	bad_seen=0
+	eval=''
+	while [ $# -gt 0 ]; do
+	    arg="$1"
+	    case "$arg" in
+	    --)
+		shift
+		break
+		;;
+	    *)
+		rev=$(git rev-parse -q --verify "$arg^{commit}") || {
+		    test $has_double_dash -eq 1 &&
+			die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
+		    break
+		}
+		case $bad_seen in
+		0) state='bad' ; bad_seen=1 ;;
+		*) state='good' ;;
+		esac
+		eval="$eval bisect_write '$state' '$rev' 'nolog'; "
+		shift
+		;;
+	    esac
+	done
+
+	#
 	# Verify HEAD.
 	#
 	head=$(GIT_DIR="$GIT_DIR" git symbolic-ref -q HEAD) ||
@@ -98,39 +131,6 @@ bisect_start() {
 	bisect_clean_state || exit
 
 	#
-	# Check for one bad and then some good revisions.
-	#
-	has_double_dash=0
-	for arg; do
-	    case "$arg" in --) has_double_dash=1; break ;; esac
-	done
-	orig_args=$(git rev-parse --sq-quote "$@")
-	bad_seen=0
-	eval=''
-	while [ $# -gt 0 ]; do
-	    arg="$1"
-	    case "$arg" in
-	    --)
-		shift
-		break
-		;;
-	    *)
-		rev=$(git rev-parse -q --verify "$arg^{commit}") || {
-		    test $has_double_dash -eq 1 &&
-			die "$(eval_gettext "'\$arg' does not appear to be a valid revision")"
-		    break
-		}
-		case $bad_seen in
-		0) state='bad' ; bad_seen=1 ;;
-		*) state='good' ;;
-		esac
-		eval="$eval bisect_write '$state' '$rev' 'nolog'; "
-		shift
-		;;
-	    esac
-	done
-
-	#
 	# Change state.
 	# In case of mistaken revs or checkout error, or signals received,
 	# "bisect_auto_next" below may exit or misbehave.
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index b5063b6..b3d1b14 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -138,15 +138,23 @@ test_expect_success 'bisect start: back in good branch' '
 	grep "* other" branch.output > /dev/null
 '
 
-test_expect_success 'bisect start: no ".git/BISECT_START" if junk rev' '
-	git bisect start $HASH4 $HASH1 -- &&
-	git bisect good &&
+test_expect_success 'bisect start: no ".git/BISECT_START" created if junk rev' '
+	git bisect reset &&
 	test_must_fail git bisect start $HASH4 foo -- &&
 	git branch > branch.output &&
 	grep "* other" branch.output > /dev/null &&
 	test_must_fail test -e .git/BISECT_START
 '
 
+test_expect_success 'bisect start: existing ".git/BISECT_START" not modified if junk rev' '
+	git bisect start $HASH4 $HASH1 -- &&
+	git bisect good &&
+	cp .git/BISECT_START saved &&
+	test_must_fail git bisect start $HASH4 foo -- &&
+	git branch > branch.output &&
+	grep "* (no branch)" branch.output > /dev/null &&
+	test_cmp saved .git/BISECT_START
+'
 test_expect_success 'bisect start: no ".git/BISECT_START" if mistaken rev' '
 	git bisect start $HASH4 $HASH1 -- &&
 	git bisect good &&
-- 
1.7.6.353.g3461

^ permalink raw reply related

* use remote repository as webroot
From: lunarnet76 @ 2011-08-02 11:17 UTC (permalink / raw)
  To: git

hi,

I have a small issue (I hope), I have a git repository in an apache folder
accessed by http://ip/git, I can clone, pull push and stuff, but I don't get
how to get the remote repository (http://ip/git) to update the files to the
one I have just pushed!

Any help would be greatly appreciated!

Regards,
JB

--
View this message in context: http://git.661346.n2.nabble.com/use-remote-repository-as-webroot-tp6644436p6644436.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: Storing additional information in commit headers
From: martin f krafft @ 2011-08-02  8:28 UTC (permalink / raw)
  To: Jeff King, git discussion list, Petr Baudis, Clemens Buchacher
In-Reply-To: <20110802035056.GB17494@sigill.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 9090 bytes --]

also sprach Jeff King <peff@peff.net> [2011.08.02.0550 +0200]:
> >   2. It is immutable. Ideally, I would like to store extra
> >      information for a ref in ref/heads/*, but there seems to be no
> >      way of doing this. Hence, I need to store it in commits and
> >      backtrack for it. Or so I think, at least…
> 
> Wait, so you want metadata on a _ref_, not on a commit? That is a very
> different thing, I think. We usually accomplish that with data in
> .git/config. Or if you need to push data between repos, or if it's too
> big to easily fit in the config, then put it in a blob and keep a
> parallel ref structure (e.g., refs/topgit/bases/refs/heads/master).
> 
> Or maybe I'm just misunderstanding.

You nailed it perfectly well. Thank you for taking the time again to
reply to me.

TopGit does what you suggest (a parallel ref structure), but there
are three problems with this, which I am trying to address:

  1. you need to ensure that these refs are pushed and fetched,
     which requires set up and possible migration issues when things
     change, and can cause big problems for contributors who just so
     happened to forget.

  2. the additional refs confuse people a lot — and I can attest to
     that because I have also at times found myself overwhelmed by
     them when staring at gitk.

  3. once a ref updates, we need to keep a pointer to the previous
     location, since one of the goals is the ability to be able to
     return to a point in history (e.g. for security updates to
     a stable package, or backports). Additional refs enhance the
     aforementioned two problems.

Therefore I thought it would be sensible to store these data in
commit. When the data change, there will always be a new commit to
store these data, and we do *not* want to update the data in
previous commits. Finding the data then becomes backtracking the
branch history until a commit is found containing them.

> >   In addition to the standard tree and parent pointers, there could
> >   be *-ref and x-*-ref headers, which take a single ref argument,
> >   presumably to a blob containing more data.
>
> I'm not sure how well-defined that is, though. What does the ref
> mean? What does it point to, and what is the meaning with respect
> to the original commit? Or are you suggesting that "*" would be
> "topgit-base" here, and that git core would understand only that
> any header matching the pattern "x-*-ref" should be followed with
> respect to reachability/pruning. Only the owner of the "*" part
> (topgit in this case) would be able to make sense of the meaning
> of the ref.

Exactly the latter. Sorry for my unannounced use of wildcards in
this context.

> If that is the case, that does make sense to me. It's basically an
> immutable version of a note.
>
> However, implementing such a thing would mean you have an awkward
> transition period where some versions of git think the referenced
> object is relevant, and others do not. That's something we can
> overcome, but it's going to require code in git, and possibly
> a dormant introduction period.

Indeed. This could be adressed by letting a tool like TopGit require
a minimum version of Git. For a while, this will burden developers,
but ensure that it works. Over time, this will cease to be
a problem.

> I suspect you would give git people more warm fuzzies about
> implementing this by showing a system that is built on git-notes
> and saying "this works really well, except that the external note
> storage is not a good reason because { it's mutable, it's not
> efficient, whatever other reason you find}". And then we know that
> the system is proven to work, and that migrating the note-like
> structure into the object is sensible.
>
> But I get the impression you're one step back from that now. So it makes
> sense to me to at least prototype it via git-notes, which will give you
> the same semantic storage (a mapping of commits to some blobs, with
> reachability handled automatically).

I appreciate how you are developing your reasoning, and the advice
you give.

Indeed, I am already prototyping using git-notes, and I designed the
datastore to be extensible, so that I can use other ways to find the
data.

Using pseudo-headers is another (temporary) way to prove the concept
works, but I am afraid that it will become standard too quickly
(because it's so easy), essentially preventing progress into x-*-ref
domain, or forcing us to carry compatibility with us forever.

What do you think about using the idea of orphan parent commits
(OPC) for now? These are conceptually closest to the x-*-ref
pointers, do not require extra setup, pollute history only a little
bit (IMHO), and slot in with Git and fsck/gc alright.

Here's the idea again, graphically:

  o--o--o--●
       /
      #

while at HEAD, I would backtrack history until I found HEAD^, which
has a parent with a well-defined commit message and holding the data
I am looking for.

Later, when x-*-ref is mainline, instead of parent pointers, it can
be used in place.

When there is a merge and the TopGit data need updating, a new
OPC is slotted into place, on the merge commit. In
the following graph, the user then decided also at a later point to
update e.g. the TopGit patch description (.topmsg), which is also
stored in this OPC:

       o--o-o
      /      \      maint       master
  o--o--o--o--+--o--O--o--o--o--●
       /     /           /
      #     #           #

To keep things simple, every OPC copies the unchanged data from the
previous one as well (compression will reduce the overhead).

Later, I can use the maint branch just in the same way I could use
master when it was at that age.

> > > Otherwise, the usual recommendation is to use a pseudo-header
> > > within the body of the commit message (i.e., "Topgit-Base: ..." at
> > > the end of the commit message). The upside is that it's easy to
> > > create, manipulate, and examine using existing git tools. The
> > > downside is that it is something that the user is more likely to
> > > see in "git log" or when editing a rebased commit message.
> > 
> > … to see *and to accidentally mess up*. And while that may even be
> > unlikely, it does expose information that really ought to be hidden.
> 
> I'm not quite sure what the information is, so I can't really judge. Do
> you have a concrete example?
> 
> I got the impression earlier you were wanting to store a human-readable
> text string.  That makes a pseudo-header a reasonable choice. But if you
> are going to reference some blob (which it seems from what you wrote
> above), and you are interested in proper reachability analysis, then no,
> it probably isn't a good idea.

I am not yet sure what information needs storing. Right now, I am
keeping five fields:

  Depend-Refs         A list of the most recent branch points from
                      dependency branchs, so that a tool can tell
                      when the dependent branch needs an update
                      (commits following those refs that are not
                      reachable by the branch head).
  Base-Ref            The ref to the most recent merge of all
                      dependencies, used to create diffs.
  Patch-Branch        boolean to suggest whether this branch is
                      designed to develop a single patch for
                      submission or use in a quilt series.
  Patch-Message       Patch description (think git-send-email).
  Integration-Branch  boolean to suggest whether instead this branch
                      is a branch designed to collect features.

At the moment, I do now know which of those are necessary, and which
I am missing.

The flexibility of being able to store as much as I want, in
whatever format I want, without having to fear overloading the
commit message or burdening the user, is what makes me want to use
refs to blobs.

> I think extensibility is welcome. It's just that most discussions
> so far have ended up realizing that a new header would just be
> cruft. Maybe yours is different. I'm still not 100% sure
> I understand what you want to accomplish, but the idea of an
> x-*-ref header is a reasonable thing for git to have.

I think there are two questions:

  1. would x-*-ref be a suitable idea for Git core?

     I think the answer is yes, as (I think) it's well-defined and
     I cannot see any problems with it, really.

  2. can we prevent abuse?

     No, never. But just like you cannot abuse X-* headers in the
     RFC822 format due to their design, x-*-ref abuse would only
     affect those who chose it.

Thank you,

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"the question of whether computers can think
 is like the question of whether submarines can swim."
                                                 -- edsgar w. dijkstra
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/sig-policy/999bbcc4/current) --]
[-- Type: application/pgp-signature, Size: 1124 bytes --]

^ permalink raw reply

* filenames in repo are in CP1251; I have linux and UTF-8
From: Ilya Basin @ 2011-08-02  7:54 UTC (permalink / raw)
  To: git

Hi list! Most of our developers use msysgit which stores filenames in
CP1251 encoding. Two of us use cygwin and linux.

When checkout on both linux and cygwin, Russian filenames are garbled.

On Cygwin it even worse, because the conversion of invalid UTF-8 chars
to Unicode, made by Cygwin is irreversible, causing git to miss
the files it've just checked out. Workaround for Cygwin is to change
LANG from "C.UTF-8" to "ru_RU.CP1251". It fixes filenames, but brakes
everything else: log (fixed by i18n.logOutputEncoding), status, show,
diff

Unlike on Cygwin, LANG has no effect for git filenames on Linux. Good
thing, there's no conversion to unicode, so files aren't lost at
checkout.

Question: is there a way to tell git on Linux to use UTF-8 filenames
for the working tree, while storing them in CP1251?

^ permalink raw reply

* Re: [RFC/ PATCH] revert: Allow arbitrary sequencer instructions
From: Christian Couder @ 2011-08-02  7:52 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Git List, Junio C Hamano, Jonathan Nieder, Christian Couder,
	Daniel Barkalow, Jeff King
In-Reply-To: <1312260884-5087-2-git-send-email-artagnon@gmail.com>

Hi Ram,

On Tue, Aug 2, 2011 at 6:54 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
>
>        if (get_sha1(sha1_abbrev, commit_sha1) < 0)
>                return NULL;
>
> -       return lookup_commit_reference(commit_sha1);
> +       commit = lookup_commit_reference(commit_sha1);
> +       if (commit) {
> +               insn = xmalloc(sizeof(struct replay_insn));
> +               insn->commit = commit;
> +               insn->action = action;
> +               return insn;
> +       }
> +       return NULL;
>  }

I'd suggest:

       commit = lookup_commit_reference(commit_sha1);
       if (!commit)
               return NULL;

       insn = xmalloc(sizeof(struct replay_insn));
       insn->commit = commit;
       insn->action = action;
       return insn;

Thanks,
Christian.

^ permalink raw reply

* Re: help: the question about relative path in the objects/info/alternates
From: Jason Wang @ 2011-08-02  7:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jason Wang, git, Tay Ray Chuan
In-Reply-To: <7vr555hvqf.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jason Wang <jason77.wang@gmail.com> writes:
>
>   
>> From the objects/info/alternates part of this URL
>> http://www.kernel.org/pub/software/scm/git/docs/gitrepository-layout.html,
>> it says it will work if alternates has relative path in it.
>>     
>
> It does not just say "relative path", but "relative to the object
> database".  It expresses where the "objects" directory you are borrowing
> from is, relative to your own "objects" directory.
>   
Understand.
> For example, if you have /src/ib/{objects,refs,HEAD} as your repository
> that borrows from elsewhere, say /src/base/, then objects/info/alternates
> file in the borrowing repository should say either /src/base/objects/, or
> "../../base/objects".
>
> An easy way to make sure you do not make typo is to go to your borrowing
> objects directory, and then let your shell completion to type your path,
> like this:
>
>     $ cd /home/jason/source/incremental-bare
>     $ cd objects
>     $ echo ../../base-bare/objects >info/alternates
>
> While formulating that "echo" command line, you would type ../../ <TAB>
> and pick base-bare, <TAB> and pick objects.
>
>   
At first, thanks very much.

Yes, your concern is right. But here i really put a right relative path 
in the alternates file. Since i can verify it. For example, %> cd 
/home/jason/source/incremental-bare; %> git tag; %>git branch, all 
commands work well, while if i put a wrong relative path in the 
alternates file, those commands will fail.

The weird thing is if i operate git commands in the 
/home/jason/source/incremental-bare, all commands work well, it can 
successfully borrow objects (refs) from base-bare via relative path. But 
if i clone a new project-dir from incremental-bare (use incremental-bare 
as local machine dir), it will fail, from the failure output it seems it 
can't borrow objects (refs) from base-bare. But if i add "file://" or 
"git://" before incremental-bare, it can work successfully. Do you think 
it is a bug?
%> git clone /home/jason/source/incremental-bare project-dir // it will fail
%> git clone file:///home/jason/source/incremental-bare project-dir // 
it will work successfully


To be convenient, i give a reproduce step for this problem:
1. verify git version on my local machine
hwang4@wanghui-desktop:mainline$ git --version
git version 1.7.6

2. basing on linux-2.6, generate a bare repository linux-2.6-bare
hwang4@wanghui-desktop:mainline$ git clone --bare linux-2.6 linux-2.6-bare
Cloning into bare repository linux-2.6-bare...
done.
hwang4@wanghui-desktop:mainline$ du -hms linux-2.6-bare/
950 linux-2.6-bare/

3. make some change in the linux-2.6, and generate a commit
hwang4@wanghui-desktop:mainline$ emacs linux-2.6/Makefile
hwang4@wanghui-desktop:mainline$ cd linux-2.6
hwang4@wanghui-desktop:linux-2.6$ git commit -m "just test" ./Makefile
[master f2c6e4f] just test
1 files changed, 1 insertions(+), 1 deletions(-)

4. let the linux-2.6-bare as a reference, generate a new INCREMENTAL 
repositry from linux-2.6
hwang4@wanghui-desktop:mainline$ git clone --bare --reference 
./linux-2.6-bare/ file:///home/hwang4/work/mainline/linux-2.6 linux-2.6-inc
Cloning into bare repository linux-2.6-inc...
remote: Counting objects: 5, done.
remote: Compressing objects: 100% (3/3), done.
Receiving objects: 100% (3/3), 293 bytes, done.
Resolving deltas: 100% (2/2), completed with 2 local objects.
remote: Total 3 (delta 2), reused 0 (delta 0)
hwang4@wanghui-desktop:mainline$ du -hms linux-2.6-inc/
1 linux-2.6-inc/

{
here i must add "file://" before source repository, otherwise, the 
mini-tree is full repository instead of incremental, E.G.:
hwang4@wanghui-desktop:mainline$ git clone --bare --reference 
./linux-2.6-bare/ linux-2.6 linux-2.6-inc
Cloning into bare repository linux-2.6-inc...
done.
hwang4@wanghui-desktop:mainline$ du -hms linux-2.6-inc/
950 linux-2.6-inc/
}

5. replace the absolute path in the 
linux-2.6-inc/objects/info/alternates to the relative path.
hwang4@wanghui-desktop:mainline$ cd linux-2.6-inc/objects/
hwang4@wanghui-desktop:objects$ cat info/alternates
/home/hwang4/work/mainline/linux-2.6-bare/objects
hwang4@wanghui-desktop:objects$ echo ../../linux-2.6-bare/objects/ > 
info/alternates
hwang4@wanghui-desktop:objects$ cat info/alternates
../../linux-2.6-bare/objects/

6. verify relative path can work
hwang4@wanghui-desktop:objects$ git tag
v2.6.11
v2.6.11-tree
v2.6.12
v2.6.12-rc2
v2.6.12-rc3
<snip>

7. from the linux-2.6-inc, we clone a new project, if we don't add 
"file://", it will fail
hwang4@wanghui-desktop:mainline$ git clone 
/home/hwang4/work/mainline/linux-2.6-inc new-project
Cloning into new-project...
done.
error: object directory 
/home/hwang4/work/mainline/new-project/.git/objects/../../linux-2.6-bare/objects/ 
does not exist; check .git/objects/info/alternates.
error: refs/tags/v2.6.38-rc8 does not point to a valid object!
error: refs/tags/v2.6.38-rc7 does not point to a valid object!
<snip>

8. but if we add "file://" before source repository or change to 
absolute path in the linux-2.6-inc/objects/info/alternates, i will work 
successfully
hwang4@wanghui-desktop:mainline$ git clone 
file:///home/hwang4/work/mainline/linux-2.6-inc new-project
Cloning into new-project...
remote: Counting objects: 1970908, done.
remote: Compressing objects: 100% (370377/370377), done.
Receiving objects: 100% (1970908/1970908), 433.54 MiB | 12.60 MiB/s, done.
remote: Total 1970908 (delta 1644814), reused 1908550 (delta 1582482)
Resolving deltas: 100% (1644814/1644814), done.
hwang4@wanghui-desktop:mainline$ ls new-project/
arch COPYING crypto drivers fs init Kbuild kernel MAINTAINERS mm README 
samples security tools virt
block CREDITS Documentation firmware include ipc Kconfig lib Makefile 
net REPORTING-BUGS scripts sound usr

^ permalink raw reply

* Re: [PATCH 09/18] revert: Don't create invalid replay_opts in parse_args
From: Christian Couder @ 2011-08-02  6:41 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Junio C Hamano, Git List, Jonathan Nieder, Daniel Barkalow,
	Jeff King
In-Reply-To: <201108020828.37904.chriscool@tuxfamily.org>

On Tuesday 02 August 2011 08:28:37 Christian Couder wrote:
> On Monday 01 August 2011 20:06:56 Ramkumar Ramachandra wrote:
> > +static void verify_opt_compatible(const char *me, const char *base_opt,
> > ...) +{
> > +	const char *this_opt;
> > +	va_list ap;
> > +	int set;
> > +
> > +	va_start(ap, base_opt);
> > +	while ((this_opt = va_arg(ap, const char *))) {
> > +		set = va_arg(ap, int);
> > +		if (set) {
> > +			va_end(ap);
> > +			die(_("%s: %s cannot be used with %s"),
> > +				me, this_opt, base_opt);
> > +		}
> > +	}
> > +	va_end(ap);
> > +}
> 
> Here I'd suggest:
> 
> static void verify_opt_compatible(const char *me, const char *base_opt,
> ...) {
> 	const char *this_opt;
> 	va_list ap;
> 
> 	va_start(ap, base_opt);
> 	while ((this_opt = va_arg(ap, const char *))) {
> 		int set = va_arg(ap, int);
> 		if (set)
> 			break;
> 	}
> 	va_end(ap);
> 
> 	if (this_opt)
> 		die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
> }

... and we could remove the "set" variable like this:

	while ((this_opt = va_arg(ap, const char *))) {
		if (va_arg(ap, int))
			break;
	}

This could be done in verify_opt_mutually_compatible() too.

Thanks,
Christian.

^ permalink raw reply

* Re: [PATCH 09/18] revert: Don't create invalid replay_opts in parse_args
From: Christian Couder @ 2011-08-02  6:28 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Junio C Hamano, Git List, Jonathan Nieder, Daniel Barkalow,
	Jeff King
In-Reply-To: <1312222025-28453-10-git-send-email-artagnon@gmail.com>

On Monday 01 August 2011 20:06:56 Ramkumar Ramachandra wrote:
>
> +static void verify_opt_compatible(const char *me, const char *base_opt,
> ...) +{
> +	const char *this_opt;
> +	va_list ap;
> +	int set;
> +
> +	va_start(ap, base_opt);
> +	while ((this_opt = va_arg(ap, const char *))) {
> +		set = va_arg(ap, int);
> +		if (set) {
> +			va_end(ap);
> +			die(_("%s: %s cannot be used with %s"),
> +				me, this_opt, base_opt);
> +		}
> +	}
> +	va_end(ap);
> +}

Here I'd suggest:

static void verify_opt_compatible(const char *me, const char *base_opt, ...)
{
	const char *this_opt;
	va_list ap;

	va_start(ap, base_opt);
	while ((this_opt = va_arg(ap, const char *))) {
		int set = va_arg(ap, int);
		if (set)
			break;
	}
	va_end(ap);

	if (this_opt)
		die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
}

Thanks and sorry for the late suggestions,
Christian.

^ permalink raw reply

* Re: [PATCH 17/18] revert: Introduce --continue to continue the operation
From: Christian Couder @ 2011-08-02  6:24 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: Junio C Hamano, Git List, Jonathan Nieder, Daniel Barkalow,
	Jeff King
In-Reply-To: <1312222025-28453-18-git-send-email-artagnon@gmail.com>

On Monday 01 August 2011 20:07:04 Ramkumar Ramachandra wrote:
> +static void verify_opt_mutually_compatible(const char *me, ...)
> +{
> +	const char *opt1, *opt2;
> +	va_list ap;
> +	int set;
> +
> +	va_start(ap, me);
> +	while ((opt1 = va_arg(ap, const char *))) {
> +		set = va_arg(ap, int);
> +		if (set)
> +			break;
> +	}
> +	if (!opt1)
> +		goto ok;
> +	while ((opt2 = va_arg(ap, const char *))) {
> +		set = va_arg(ap, int);
> +		if (set) {
> +			va_end(ap);
> +			die(_("%s: %s cannot be used with %s"),
> +				me, opt1, opt2);
> +		}
> +	}
> +ok:
> +	va_end(ap);
> +}

I'd suggest something like this:

static void verify_opt_mutually_compatible(const char *me, ...)
{
	const char *opt1, *opt2;
	va_list ap;

	va_start(ap, me);
	while ((opt1 = va_arg(ap, const char *))) {
		int set = va_arg(ap, int);
		if (set)
			break;
	}
	if (opt1) {
		while ((opt2 = va_arg(ap, const char *))) {
			int set = va_arg(ap, int);
			if (set)
				break;
		}
	}
	va_end(ap);

	if (opt1 && opt2)
		die(_("%s: %s cannot be used with %s"), me, opt1, opt2);
}

Thanks,
Christian.

^ permalink raw reply

* Re: [GSoC update] Iterating over a stable series
From: Ramkumar Ramachandra @ 2011-08-02  5:01 UTC (permalink / raw)
  To: Git List
  Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
	Daniel Barkalow, Jeff King
In-Reply-To: <1312260884-5087-1-git-send-email-artagnon@gmail.com>

Hi,

Ramkumar Ramachandra writes:
> Subject: [GSoC update] Iterating over a stable series

Er, I just noticed that another GSoC update from me has a very similar
subject and it refers to something entirely different.  My apologies.
A more appropriate subject: Building on rr/revert-cherry-pick-continue
(or what I call sequencer-stable).

-- Ram

^ permalink raw reply

* [RFC/ PATCH] revert: Allow arbitrary sequencer instructions
From: Ramkumar Ramachandra @ 2011-08-02  4:54 UTC (permalink / raw)
  To: Git List
  Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
	Daniel Barkalow, Jeff King
In-Reply-To: <1312260884-5087-1-git-send-email-artagnon@gmail.com>

Allow arbitrary sequencer instructions in the instruction sheet.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |  101 +++++++++++++++++++++++++++++-------------------------
 sequencer.h      |   10 +++++
 2 files changed, 64 insertions(+), 47 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index bca1490..127b97e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -646,49 +646,54 @@ static void read_and_refresh_cache(struct replay_opts *opts)
  *     assert(commit_list_count(list) == 2);
  *     return list;
  */
-struct commit_list **commit_list_append(struct commit *commit,
-					struct commit_list **next)
+struct replay_insn_list **replay_insn_list_append(struct replay_insn *insn,
+						struct replay_insn_list **next)
 {
-	struct commit_list *new = xmalloc(sizeof(struct commit_list));
-	new->item = commit;
+	struct replay_insn_list *new = xmalloc(sizeof(struct replay_insn_list));
+	new->item = insn;
 	*next = new;
 	new->next = NULL;
 	return &new->next;
 }
 
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
-		struct replay_opts *opts)
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
 {
-	struct commit_list *cur = NULL;
+	struct replay_insn_list *cur = NULL;
+	struct commit *commit;
 	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
 	const char *sha1_abbrev = NULL;
-	const char *action_str = opts->action == REVERT ? "revert" : "pick";
+	const char *action_str;
 
 	for (cur = todo_list; cur; cur = cur->next) {
-		sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
-		if (get_message(cur->item, &msg))
+		commit = cur->item->commit;
+		action_str = cur->item->action == REVERT ? "revert" : "pick";
+
+		sha1_abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+		if (get_message(commit, &msg))
 			return error(_("Cannot get commit message for %s"), sha1_abbrev);
 		strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
 	}
 	return 0;
 }
 
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static struct replay_insn *parse_insn_line(char *start)
 {
 	unsigned char commit_sha1[20];
 	char sha1_abbrev[40];
+	struct commit *commit;
+	struct replay_insn *insn;
 	enum replay_action action;
-	int insn_len = 0;
+	int keyword_len;
 	char *p, *q;
 
 	if (!prefixcmp(start, "pick ")) {
 		action = CHERRY_PICK;
-		insn_len = strlen("pick");
-		p = start + insn_len + 1;
+		keyword_len = strlen("pick");
+		p = start + keyword_len + 1;
 	} else if (!prefixcmp(start, "revert ")) {
 		action = REVERT;
-		insn_len = strlen("revert");
-		p = start + insn_len + 1;
+		keyword_len = strlen("revert");
+		p = start + keyword_len + 1;
 	} else
 		return NULL;
 
@@ -699,30 +704,25 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
 
 	strlcpy(sha1_abbrev, p, q - p);
 
-	/*
-	 * Verify that the action matches up with the one in
-	 * opts; we don't support arbitrary instructions
-	 */
-	if (action != opts->action) {
-		const char *action_str;
-		action_str = action == REVERT ? "revert" : "cherry-pick";
-		error(_("Cannot %s during a %s"), action_str, action_name(opts));
-		return NULL;
-	}
-
 	if (get_sha1(sha1_abbrev, commit_sha1) < 0)
 		return NULL;
 
-	return lookup_commit_reference(commit_sha1);
+	commit = lookup_commit_reference(commit_sha1);
+	if (commit) {
+		insn = xmalloc(sizeof(struct replay_insn));
+		insn->commit = commit;
+		insn->action = action;
+		return insn;
+	}
+	return NULL;
 }
 
-static void read_populate_todo(struct commit_list **todo_list,
-			struct replay_opts *opts)
+static void read_populate_todo(struct replay_insn_list **todo_list)
 {
 	const char *todo_file = git_path(SEQ_TODO_FILE);
 	struct strbuf buf = STRBUF_INIT;
-	struct commit_list **next;
-	struct commit *commit;
+	struct replay_insn *insn;
+	struct replay_insn_list **next;
 	char *p;
 	int fd;
 
@@ -738,10 +738,10 @@ static void read_populate_todo(struct commit_list **todo_list,
 
 	next = todo_list;
 	for (p = buf.buf; *p; p = strchr(p, '\n') + 1) {
-		commit = parse_insn_line(p, opts);
-		if (!commit)
+		insn = parse_insn_line(p);
+		if (!insn)
 			goto error;
-		next = commit_list_append(commit, next);
+		next = replay_insn_list_append(insn, next);
 	}
 	if (!*todo_list)
 		goto error;
@@ -795,18 +795,23 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
 		die(_("Malformed options sheet: %s"), opts_file);
 }
 
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
 				struct replay_opts *opts)
 {
 	struct rev_info revs;
 	struct commit *commit;
-	struct commit_list **next;
+	struct replay_insn *insn;
+	struct replay_insn_list **next;
 
 	prepare_revs(&revs, opts);
 
 	next = todo_list;
-	while ((commit = get_revision(&revs)))
-		next = commit_list_append(commit, next);
+	while ((commit = get_revision(&revs))) {
+		insn = xmalloc(sizeof(struct replay_insn));
+		insn->commit = commit;
+		insn->action = opts->action;
+		next = replay_insn_list_append(insn, next);
+	}
 }
 
 static int create_seq_dir(void)
@@ -835,7 +840,7 @@ static void save_head(const char *head)
 		die(_("Error wrapping up %s."), head_file);
 }
 
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static void save_todo(struct replay_insn_list *todo_list)
 {
 	const char *todo_file = git_path(SEQ_TODO_FILE);
 	static struct lock_file todo_lock;
@@ -843,7 +848,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
 	int fd;
 
 	fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
-	if (format_todo(&buf, todo_list, opts) < 0)
+	if (format_todo(&buf, todo_list) < 0)
 		die(_("Could not format %s."), todo_file);
 	if (write_in_full(fd, buf.buf, buf.len) < 0) {
 		strbuf_release(&buf);
@@ -887,9 +892,10 @@ static void save_opts(struct replay_opts *opts)
 	}
 }
 
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct replay_insn_list *todo_list,
+			struct replay_opts *opts)
 {
-	struct commit_list *cur;
+	struct replay_insn_list *cur;
 	int res;
 
 	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -899,8 +905,9 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 	read_and_refresh_cache(opts);
 
 	for (cur = todo_list; cur; cur = cur->next) {
-		save_todo(cur, opts);
-		res = do_pick_commit(cur->item, opts);
+		save_todo(cur);
+		opts->action = cur->item->action;
+		res = do_pick_commit(cur->item->commit, opts);
 		if (res) {
 			if (!cur->next)
 				/*
@@ -925,7 +932,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 
 static int pick_revisions(struct replay_opts *opts)
 {
-	struct commit_list *todo_list = NULL;
+	struct replay_insn_list *todo_list = NULL;
 	unsigned char sha1[20];
 
 	read_and_refresh_cache(opts);
@@ -942,7 +949,7 @@ static int pick_revisions(struct replay_opts *opts)
 		if (!file_exists(git_path(SEQ_TODO_FILE)))
 			goto error;
 		read_populate_opts(&opts);
-		read_populate_todo(&todo_list, opts);
+		read_populate_todo(&todo_list);
 
 		/* Verify that the conflict has been resolved */
 		if (!index_differs_from("HEAD", 0))
diff --git a/sequencer.h b/sequencer.h
index 931733c..eebee65 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -32,6 +32,16 @@ struct replay_opts {
 	size_t xopts_nr, xopts_alloc;
 };
 
+struct replay_insn {
+	struct commit *commit;
+	enum replay_action action;
+};
+
+struct replay_insn_list {
+	struct replay_insn *item;
+	struct replay_insn_list *next;
+};
+
 /*
  * Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
  * any errors.  Intended to be used by 'git reset'.
-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply related

* [GSoC update] Iterating over a stable series
From: Ramkumar Ramachandra @ 2011-08-02  4:54 UTC (permalink / raw)
  To: Git List
  Cc: Junio C Hamano, Jonathan Nieder, Christian Couder,
	Daniel Barkalow, Jeff King

Hi,

The post midterm work is suffering because I'm constantly rewriting
the sequencer-stable series: I often resort to throwing away big
patches that move functions from builtin/revert.c to sequencer.c.
Hopefully, the latest iteration [1] will not require rewriting.

I'd like some early feedback for one of the "design patches" in my new
series: I've chosen to use a commit + action to represent a todo_list.
I'd initially tried a commit + opts keeping future expantion in mind
(allowing instruction-specific command-line options), but the result
is quite inelegant.  Although commit message/ tests are missing, I'd
like to describe the intent in detail:

This patch is a prerequisite for decoupling todo parsing from opts
parsing.  I want to decouple them so that I can achieve tighter
coupling between "git commit" and the sequencer [2].  After that, I
want to craft a nice API and move/ expose various functions in
builtin/revert.c starting with the parsing functions.

Thanks for reading.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/178372
[2]: http://mid.gmane.org/CALkWK0=9kwgtZB-BA12tOQrQXS8XRbhTg6K=Ak00o2nurX16Fg@mail.gmail.com

Ramkumar Ramachandra (1):
  revert: Allow arbitrary sequencer instructions

 builtin/revert.c |  101 +++++++++++++++++++++++++++++-------------------------
 sequencer.h      |   10 +++++
 2 files changed, 64 insertions(+), 47 deletions(-)

-- 
1.7.6.351.gb35ac.dirty

^ permalink raw reply

* Re: [PATCH 17/18] revert: Introduce --continue to continue the operation
From: Ramkumar Ramachandra @ 2011-08-02  4:41 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, Git List, Jonathan Nieder, Daniel Barkalow,
	Jeff King
In-Reply-To: <201108020451.36617.chriscool@tuxfamily.org>

Hi Christian,

Christian Couder writes:
> On Tuesday 02 August 2011 04:47:01 Christian Couder wrote:
>> We can simplify this loop using strchrnul() like this:
[...]
>        for (i = 1; *p; i++) {
>                struct commit *commit = parse_insn_line(p, opts);
>                if (!commit)
>                        return error(_("Could not parse line %d."), i);
>                next = commit_list_append(commit, next);
>                p = strchrnul(p, '\n');
>                if (*p)
>                        p++;
>        }

Thanks for the valuable review.  I would have preferred to receive it
earlier in the development cycle than later- although I've queued this
along with other similar changes (error reporting/ stylistic changes;
not urgent), I'm not in favor of holding up the series.

Thank you.

-- Ram

^ permalink raw reply

* Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork
From: Jeff King @ 2011-08-02  4:00 UTC (permalink / raw)
  To: René Scharfe; +Cc: Johannes Sixt, Erik Faye-Lund, Junio C Hamano, git
In-Reply-To: <4E37202B.80208@lsrfire.ath.cx>

On Mon, Aug 01, 2011 at 11:52:43PM +0200, René Scharfe wrote:

> Am 01.08.2011 23:20, schrieb Johannes Sixt:
> > Am 01.08.2011 22:48, schrieb René Scharfe:
> >> So git archive gives the right results when writing to a pipe, but
> >> always the same wrong result when writing directly to a file.
> > 
> > This could indeed be a CRLF issue. archive-tar.c runs gzip to let it
> > write to the original fd 1 (stdout). gzip is an MSYS program, and MSYS
> > is "clever" and sets up the channel in text mode (CRLF conversion) if it
> > is a regular file, but in binary mode if it is a pipe.
> > 
> > Without the gzip filter, git-archive writes to stdout itself. Since we
> > have set up all our channels in binary mode, we do not suffer from the
> > same problem for plain tar format.
> > 
> > So, I don't think we can do a lot about it, short of patching MSYS again...
> 
> Or we could pipe the output through us, i.e. attach a builtin version of
> cat at the output end of the called command.  Only on Windows, of
> course.  Better ugly and limping then wrong, right?

Yeah, that would work. But I am confused. If what Johannes says is true,
isn't MSYS gzip totally broken for:

  # works
  echo foo | gzip -c | cat >foo.gz

  # broken; introduces CR
  echo foo | gzip -c >foo.gz

? (The "works" and "broken" there are my guesses; I don't have a Windows
box to test on). IOW, is it simply gzip that is broken, and any fix we
do is simply working around a bug in gzip? And therefore the right
solution is for MSYS people to fix gzip?

-Peff

^ permalink raw reply

* Re: Storing additional information in commit headers
From: Jeff King @ 2011-08-02  3:50 UTC (permalink / raw)
  To: martin f krafft; +Cc: git discussion list, Petr Baudis, Clemens Buchacher
In-Reply-To: <20110801211104.GC15401@fishbowl.rw.madduck.net>

On Mon, Aug 01, 2011 at 11:11:04PM +0200, martin f krafft wrote:

> >   1. Does git actually care about your data? E.g., would it want to use
> >      it for reachability analysis in git-fsck?
> > 
> >   2. Is it an immutable property of a commit, or can it be changed after
> >      the fact?
> 
> Excellent points, and I have answers to both:
> 
>   1. Ideally, I would like to point to another blob containing
>      information. Right now, in order to prevent gc from pruning
>      it, that would have to be a commit pointed to with a parent
>      pointer, which is just not right (it's not a parent) and causes
>      the commit to show up in the history (which it should not, as
>      it's an implementation detail).

In that case, notes sound like a nice solution, as that is exactly what
they do. Yes, they are mutable, but that might not be that big a deal.

>   2. It is immutable. Ideally, I would like to store extra
>      information for a ref in ref/heads/*, but there seems to be no
>      way of doing this. Hence, I need to store it in commits and
>      backtrack for it. Or so I think, at least…

Wait, so you want metadata on a _ref_, not on a commit? That is a very
different thing, I think. We usually accomplish that with data in
.git/config. Or if you need to push data between repos, or if it's too
big to easily fit in the config, then put it in a blob and keep a
parallel ref structure (e.g., refs/topgit/bases/refs/heads/master).

Or maybe I'm just misunderstanding.

> > Otherwise, if (1) is yes, then a commit header makes sense. But
> > then, it should also be something that git is taught about, and
> > your commit header should not be some topgit-specific thing, but
> > a header showing the generalized form.
> 
> I agree entirely and would be all too excited to see this happening.
> I already had ideas too:
> 
>   In addition to the standard tree and parent pointers, there could
>   be *-ref and x-*-ref headers, which take a single ref argument,
>   presumably to a blob containing more data.

I'm not sure how well-defined that is, though. What does the ref mean?
What does it point to, and what is the meaning with respect to the
original commit? Or are you suggesting that "*" would be "topgit-base"
here, and that git core would understand only that any header matching
the pattern "x-*-ref" should be followed with respect to
reachability/pruning. Only the owner of the "*" part (topgit in this
case) would be able to make sense of the meaning of the ref.

If that is the case, that does make sense to me. It's basically an
immutable version of a note.

However, implementing such a thing would mean you have an awkward
transition period where some versions of git think the referenced object
is relevant, and others do not. That's something we can overcome, but
it's going to require code in git, and possibly a dormant introduction
period.

I suspect you would give git people more warm fuzzies about implementing
this by showing a system that is built on git-notes and saying "this
works really well, except that the external note storage is not a good
reason because { it's mutable, it's not efficient, whatever other reason
you find}". And then we know that the system is proven to work, and that
migrating the note-like structure into the object is sensible.

But I get the impression you're one step back from that now. So it makes
sense to me to at least prototype it via git-notes, which will give you
the same semantic storage (a mapping of commits to some blobs, with
reachability handled automatically).

> > Otherwise, the usual recommendation is to use a pseudo-header
> > within the body of the commit message (i.e., "Topgit-Base: ..." at
> > the end of the commit message). The upside is that it's easy to
> > create, manipulate, and examine using existing git tools. The
> > downside is that it is something that the user is more likely to
> > see in "git log" or when editing a rebased commit message.
> 
> … to see *and to accidentally mess up*. And while that may even be
> unlikely, it does expose information that really ought to be hidden.

I'm not quite sure what the information is, so I can't really judge. Do
you have a concrete example?

I got the impression earlier you were wanting to store a human-readable
text string.  That makes a pseudo-header a reasonable choice. But if you
are going to reference some blob (which it seems from what you wrote
above), and you are interested in proper reachability analysis, then no,
it probably isn't a good idea.

> I can see how it's arguable too why one would want to give git
> commit objects the ability to reference arbitrary blobs containing
> additional information. I suppose the answer to this question is
> related to the answer to the question of whether Git is
> a contained/complete tool as-is, or also serves as
> a "framework"/"toolkit" for advanced/creative use.
> 
> The availability of the porcelain commands seems to suggest that
> extensible/flexible additional features should be welcome! ;)

I think extensibility is welcome. It's just that most discussions so far
have ended up realizing that a new header would just be cruft. Maybe
yours is different. I'm still not 100% sure I understand what you want
to accomplish, but the idea of an x-*-ref header is a reasonable thing
for git to have.

-Peff

^ permalink raw reply

* Re: Possibly solved invalid free() in git-remote-http from Git 1.7.2.1
From: Jeff King @ 2011-08-02  3:33 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Ævar Arnfjörð Bjarmason, Git Mailing List
In-Reply-To: <20110802105124.000021c3@unknown>

On Tue, Aug 02, 2011 at 10:51:24AM +0800, Tay Ray Chuan wrote:

> On Mon, 1 Aug 2011 12:00:19 -0600
> Jeff King <peff@peff.net> wrote:
> > Hmm. The memory management in the function seems weird to me.
> 
> How about this?

Yes, that fixes all of the oddities I found (I assume you looked and
agreed with me that they were wrong).

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox