Git development
 help / color / mirror / Atom feed
* Re: per-ref data storage
From: martin f krafft @ 2011-08-02 21:12 UTC (permalink / raw)
  To: Jeff King, git discussion list, Petr Baudis, Clemens Buchacher
In-Reply-To: <20110802192727.GB20239@fishbowl.rw.madduck.net>

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

also sprach martin f krafft <madduck@madduck.net> [2011.08.02.2127 +0200]:
> refs/heads/master is a file, containing its payload in the first
> line by format definition, right?
> 
> I mean: the storage is right there, isn't it?
> 
> Of course this opens a whole new can of worms: merging per-ref data.

origin/master can contain a different set of per-ref data than
master, and the consolidation would need to happen during the normal
merge.

But unless there's always a new commit associated with a change of
those data, git-push will happily overwrite those data on the
remote.

… unless the remote refuses to accept a ref update if the data have
changed. Conceivably that's could lead into a control path similar
to what happens on a non-fast-forward push — unless
receive.nonFastForwards is on.

What then?

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
seminars, n.:
  from "semi" and "arse", hence, any half-assed discussion.
 
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

* Re: [PATCH] commit: write out cache-tree information
From: Junio C Hamano @ 2011-08-02 21:15 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Git Mailing List, Carlos Martín Nieto
In-Reply-To: <7vmxfrel63.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>> While write-tree has code to write out the cache-tree information
>> (since we have to compute it anyway if the cache is stale), commit
>> lost this capability when it became a builtin and moved away from
>> using write-tree.
>
> Earlier the code read from the index, made sure that it is not unmerged by
> running cache_tere_update(), before running prepare-commit-msg hook. The
> hook used to see the index that was read in this codepath which is the
> same as what pre-commit left us.
>
> Why run an extra I/O here? The index file could be quite large, and I do
> not want people to writing it out without good reason.

One possible good reason is if you did this only when we are committing
the whole index. Then theoretically the next operation after commit could
start from a fully populated cache-tree in the index.

When we are running a partial commit, the index file you are writing back
is a temporary index only to build a tree object to record in the commit,
which we already have done, and the temporary will be discarded.

^ permalink raw reply

* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
From: Jon Seymour @ 2011-08-02 21:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, chriscool, j6t, jnareb
In-Reply-To: <7vr553elsd.fsf@alter.siamese.dyndns.org>

On Wed, Aug 3, 2011 at 4:00 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jon Seymour <jon.seymour@gmail.com> writes:
>
>> 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>
>> ...
>
> Ok.
>
>>  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."),
>
> I think this is a regression from naming perspective from the previous
> round. You would be either the normal (checkout) mode or no-checkout mode,
> and honestly, --no-checkout would be understood by anybody while update-ref
> would be understood only by Gitz.
>

Is it relevant here that --bisect-mode here is on bisect--helper which
is really part of the plumbing and unlikely to be used by anything
other than git's own porcelain?

I did leave the porcelain option as --no-checkout.

I personally think the git-bisect.sh implementation which uses
BISECT_MODE internally is slightly simpler than the one that uses
BISECT_NO_CHECKOUT.

Are you ok if I keep roughly the v13 git-bisect.sh implementation, but
revert the bisect--helper.c to v11 (adjusting git-bisect.sh to
compensate)?

jon,

^ permalink raw reply

* Re: [PATCH 0/5] bisect: Add support for a --no-checkout option.
From: Jon Seymour @ 2011-08-02 21:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, git, j6t, jnareb
In-Reply-To: <7vvcufem32.fsf@alter.siamese.dyndns.org>

On Wed, Aug 3, 2011 at 3:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jon Seymour <jon.seymour@gmail.com> writes:
>
>>> Then, the next series can add a --bisect-head option (instead of
>>> --update-ref) to allow the head to be varied (defaulting to
>>> BISECT_HEAD) if not specified.
>>
>> To clarify: --bisect-head would default to HEAD if mode is checkout,
>> to BISECT_HEAD if the mode is update-ref.
>
> I am not sure if we ever want to allow updating anything but HEAD under
> checkout mode, so I would rather think --bisect-head is _always_ invalid
> under that mode. IOW, as I said in the previous message, these two are not
> independent options.
>

I agree. In fact, I like your idea of a BISECT_HEAD for --no-checkout,
and I don't see any need (even in the future)
to allow this to be configurable [ except perhaps if someone thinks
doing a --no-checkout with HEAD is actually useful ].

jon.

^ permalink raw reply

* Re: [PATCH v2] commit: allow partial commits with relative paths
From: Junio C Hamano @ 2011-08-02 21:31 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Michael J Gruber, git, Reuben Thomas
In-Reply-To: <20110730171347.GA10432@toss.lan>

Clemens Buchacher <drizzd@aon.at> writes:

> diff --git a/setup.c b/setup.c
> index 5ea5502..2c51a9a 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -264,6 +264,38 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
>  	return pathspec;
>  }
>  
> +const char *pathspec_prefix(const char *prefix, const char **pathspec)
> +{

As a public function, this sorely needs a comment that describes what it
does. More importantly, when I tried to come up with an example
description, it became very clear that this neither prefixes anything to
pathspec, nor prefixes pathspec to anything else.

As an internal helper in ls-files implementation it was perfectly
fine that the function was slightly misnamed, but if you are making it
into a public API, we should get its name right.

Perhaps "common_prefix()"?

Don't you also want to consolidate dir.c:common_prefix() with this?

What happens when pathspec has the recently introduced magic elements,
e.g. ':/' that widens the match to the whole tree?

^ permalink raw reply

* Re: refs/replace advice
From: Pete Wyckoff @ 2011-08-02 21:54 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt
In-Reply-To: <20110729153122.GA4535@padd.com>

pw@padd.com wrote on Fri, 29 Jul 2011 08:31 -0700:
> I've got two near-identical git repos, both imported from
> gigantic upstream p4 repos.  They started at slightly different
> times so have different commit SHA1s, even though the tree
> contents are the same.  I can't filter-branch either of them; too
> many users already.
>
> I'm trying to use "git replace" to avoid cloning the entire set
> of duplicate commits across a slow inter-site link.  Like this:

To follow-up, I decided this was not the right approach.
Instead, for future git-p4 repo creation, I've patched git-p4
to make sure the timestamps and hence the initial SHA1s are
identical.

Thanks, Hannes, for your suggestion, though.

		-- Pete

^ permalink raw reply

* Re: [PATCH] describe: Refresh the index when run with --dirty
From: Junio C Hamano @ 2011-08-02 21:59 UTC (permalink / raw)
  To: Allan Caffee; +Cc: git, Jeff King, Sverre Rabbelier
In-Reply-To: <1312163561-77072-1-git-send-email-allan.caffee@gmail.com>

Thanks.

Here is a minor fix-up on top, that can be squashed in a re-roll (if you
plan to do one).

diff --git a/builtin/describe.c b/builtin/describe.c
index 792af76..9f63067 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -24,7 +24,6 @@ static int longformat;
 static int abbrev = -1; /* unspecified */
 static int max_candidates = 10;
 static struct hash_table names;
-static struct lock_file index_lock; /* real index */
 static int have_util;
 static const char *pattern;
 static int always;
@@ -400,7 +399,6 @@ static void describe(const char *arg, int last_one)
 int cmd_describe(int argc, const char **argv, const char *prefix)
 {
 	int contains = 0;
-	int fd;
 	struct option options[] = {
 		OPT_BOOLEAN(0, "contains",   &contains, "find the tag that comes after the commit"),
 		OPT_BOOLEAN(0, "debug",      &debug, "debug search strategy on stderr"),
@@ -465,13 +463,18 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 
 	if (argc == 0) {
 		if (dirty) {
-			read_cache();
-			refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
+			static struct lock_file index_lock;
+			int fd;
+
+			read_cache_preload(NULL);
+			refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED,
+				      NULL, NULL, NULL);
 			fd = hold_locked_index(&index_lock, 0);
 			if (0 <= fd)
 				update_index_if_able(&the_index, &index_lock);
 
-			if (!cmd_diff_index(ARRAY_SIZE(diff_index_args) - 1, diff_index_args, prefix))
+			if (!cmd_diff_index(ARRAY_SIZE(diff_index_args) - 1,
+					    diff_index_args, prefix))
 				dirty = NULL;
 		}
 		describe("HEAD", 1);

^ permalink raw reply related

* Re: [PATCH] commit: write out cache-tree information
From: Thomas Rast @ 2011-08-02 22:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Carlos Martín Nieto
In-Reply-To: <7vmxfrel63.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> <trast@student.ethz.ch> writes:
>
> > From: Thomas Rast <trast@student.ethz.ch>
> >
> > While write-tree has code to write out the cache-tree information
> > (since we have to compute it anyway if the cache is stale), commit
> > lost this capability when it became a builtin and moved away from
> > using write-tree.
>
> Earlier the code read from the index, made sure that it is not unmerged by
> running cache_tere_update(), before running prepare-commit-msg hook. The
> hook used to see the index that was read in this codepath which is the
> same as what pre-commit left us.
>
> Why run an extra I/O here? The index file could be quite large, and I do
> not want people to writing it out without good reason.

Ok, so let's run some numbers.  With the first test script below I'm
seeing:

  before patch:
    $ time ./commit-in-large-tree.sh
    Initialized empty Git repository in /dev/shm/commit-in-large-tree.tmp/.git/
    6.9M    .git/index

    real    1m31.607s
    user    0m57.604s
    sys     0m29.976s

  after patch: 14% speedup
    $ time ./commit-in-large-tree.sh
    Initialized empty Git repository in /dev/shm/commit-in-large-tree.tmp/.git/
    7.0M    .git/index

    real    1m18.521s
    user    0m53.430s
    sys     0m22.138s

On the other hand if you touch every file as in the second script:

  before patch:
    $ time ./commit-in-large-tree-2.sh 
    Initialized empty Git repository in /dev/shm/commit-in-large-tree.tmp/.git/
    6.9M    .git/index

    real    1m40.910s
    user    0m58.731s
    sys     0m38.011s

  after patch: 5% slowdown
    $ time ./commit-in-large-tree-2.sh 
    Initialized empty Git repository in /dev/shm/commit-in-large-tree.tmp/.git/
    7.0M    .git/index

    real    1m45.465s
    user    1m2.329s
    sys     0m38.849s

I also ran the latter test where it only touches one file in 100
(instead of all 1000) subdirs, and there the patch is still a speedup.

So I guess it depends whether we expect users to mostly modify a small
part or the whole tree.

Regarding your other email

> When we are running a partial commit, the index file you are writing back
> is a temporary index only to build a tree object to record in the commit,
> which we already have done, and the temporary will be discarded.

that's a valid point that I need to address.



-- 8< --   commit-in-large-tree.sh
#!/bin/sh

set -e

git init /dev/shm/commit-in-large-tree.tmp
cd /dev/shm/commit-in-large-tree.tmp
for i in $(seq 1 1000); do
    mkdir $i
    (
	cd $i
	for j in $(seq 1 100); do
	    echo $j > $j
	done
    )
    git add $i
done
git commit -q -m initial
du -h .git/index

for i in $(seq 1 100); do
    echo "$i changed" > $i/$i
    git add $i/$i
    git commit -q -m $i
done

rm -rf /dev/shm/commit-in-large-tree.tmp
-- >8 --

-- 8< --  commit-in-large-tree-2.sh
#!/bin/sh

set -e

git init /dev/shm/commit-in-large-tree.tmp
cd /dev/shm/commit-in-large-tree.tmp
for i in $(seq 1 1000); do
    mkdir $i
    (
	cd $i
	for j in $(seq 1 100); do
	    echo $j > $j
	done
    )
    git add $i
done
git commit -q -m initial
du -h .git/index

for i in $(seq 1 100); do
    for j in $(seq 1 1000); do
	echo "$i changed" > $j/$i
    done
    git add -u
    git commit -q -m $i
done

rm -rf /dev/shm/commit-in-large-tree.tmp
-- >3 --

--
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [RFC 0/6] git-check-attr should work for relative paths
From: Junio C Hamano @ 2011-08-02 22:02 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git
In-Reply-To: <1311849425-9057-1-git-send-email-mhagger@alum.mit.edu>

A minor compilation fix-up on top, to be squashed when the series is
re-rolled.

diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 23a6e07..8155b3d 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -44,7 +44,7 @@ static void output_attr(int cnt, struct git_attr_value *check,
 static void check_attr(const char *prefix, int cnt,
 	struct git_attr_value *check, const char *file)
 {
-	const char *full_path =
+	char *full_path =
 		prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
 	if (check != NULL) {
 		if (git_checkattr(full_path, cnt, check))
diff --git a/test-path-utils.c b/test-path-utils.c
index 471406c..3bc20e9 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -36,10 +36,10 @@ int main(int argc, char **argv)
 	}
 
 	if (argc >= 4 && !strcmp(argv[1], "prefix_path")) {
-		int nongit_ok;
-		setup_git_directory_gently(&nongit_ok);
 		char *prefix = argv[2];
 		int prefix_len = strlen(prefix);
+		int nongit_ok;
+		setup_git_directory_gently(&nongit_ok);
 		while (argc > 3) {
 			puts(prefix_path(prefix, prefix_len, argv[3]));
 			argc--;

^ permalink raw reply related

* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
From: Junio C Hamano @ 2011-08-02 22:05 UTC (permalink / raw)
  To: Jon Seymour; +Cc: git, chriscool, j6t, jnareb
In-Reply-To: <CAH3Anrrf5aZMEH8KmKtvdaRf=D9fb6OVRJN+VjGM8Cd2wV6rnA@mail.gmail.com>

Jon Seymour <jon.seymour@gmail.com> writes:

>> I think this is a regression from naming perspective from the previous
>> round. You would be either the normal (checkout) mode or no-checkout mode,
>> and honestly, --no-checkout would be understood by anybody while update-ref
>> would be understood only by Gitz.
>>
>
> Is it relevant here that --bisect-mode here is on bisect--helper which
> is really part of the plumbing and unlikely to be used by anything
> other than git's own porcelain?

Ah, Ok, that is what I missed. I also was scratching my head after seeing
that there was not much change in the proposed log messages from the
earlier round.

^ permalink raw reply

* [PATCH v14 0/8] bisect: Add support for --no-checkout option
From: Jon Seymour @ 2011-08-02 22:15 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
----------------
v14:
	Reverted --bisect-mode aspect of v13 change so C code matches v11.
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     |    7 ++-
 git-bisect.sh                |  113 ++++++++++++++++++++++--------------
 t/t6030-bisect-porcelain.sh  |  132 +++++++++++++++++++++++++++++++++++++++++-
 6 files changed, 258 insertions(+), 61 deletions(-)

-- 
1.7.6.353.gb3ae7e

^ permalink raw reply

* [PATCH v14 1/8] bisect: move argument parsing before state modification.
From: Jon Seymour @ 2011-08-02 22:15 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312323362-20096-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.gb3ae7e

^ permalink raw reply related

* [PATCH v14 2/8] bisect: use && to connect statements that are deferred with eval.
From: Jon Seymour @ 2011-08-02 22:15 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312323362-20096-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.gb3ae7e

^ permalink raw reply related

* [PATCH v14 3/8] bisect: add tests to document expected behaviour in presence of broken trees.
From: Jon Seymour @ 2011-08-02 22:15 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312323362-20096-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.gb3ae7e

^ permalink raw reply related

* [PATCH v14 4/8] bisect: introduce support for --no-checkout option.
From: Jon Seymour @ 2011-08-02 22:15 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312323362-20096-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 |    7 +++++--
 3 files changed, 28 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..d96a193 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -4,16 +4,19 @@
 #include "bisect.h"
 
 static const char * const git_bisect_helper_usage[] = {
-	"git bisect--helper --next-all",
+	"git bisect--helper --next-all [--no-checkout]",
 	NULL
 };
 
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	int next_all = 0;
+	int no_checkout = 0;
 	struct option options[] = {
 		OPT_BOOLEAN(0, "next-all", &next_all,
 			    "perform 'git bisect next'"),
+		OPT_BOOLEAN(0, "no-checkout", &no_checkout,
+			    "update HEAD instead of checking out the current commit"),
 		OPT_END()
 	};
 
@@ -24,5 +27,5 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_bisect_helper_usage, options);
 
 	/* next-all */
-	return bisect_next_all(prefix);
+	return bisect_next_all(prefix, no_checkout);
 }
-- 
1.7.6.353.gb3ae7e

^ permalink raw reply related

* [PATCH v14 5/8] bisect: introduce --no-checkout support into porcelain.
From: Jon Seymour @ 2011-08-02 22:15 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312323362-20096-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..99a2637 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=
 	while [ $# -gt 0 ]; do
 	    arg="$1"
 	    case "$arg" in
@@ -76,6 +79,11 @@ bisect_start() {
 		shift
 		break
 		;;
+	    --no-checkout)
+		BISECT_MODE=--no-checkout;
+		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" = "--no-checkout"; 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}
 	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" = "--no-checkout"; 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.gb3ae7e

^ permalink raw reply related

* [PATCH v14 6/8] bisect: add tests for the --no-checkout option.
From: Jon Seymour @ 2011-08-02 22:16 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312323362-20096-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.gb3ae7e

^ permalink raw reply related

* [PATCH v14 7/8] bisect: add documentation for --no-checkout option.
From: Jon Seymour @ 2011-08-02 22:16 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312323362-20096-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.gb3ae7e

^ permalink raw reply related

* [PATCH v14 8/8] bisect: change bisect function to update BISECT_HEAD, rather than HEAD.
From: Jon Seymour @ 2011-08-02 22:16 UTC (permalink / raw)
  To: git; +Cc: chriscool, gitster, j6t, jnareb, Jon Seymour
In-Reply-To: <1312323362-20096-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 +-
 builtin/bisect--helper.c     |    2 +-
 git-bisect.sh                |   17 +++++++++++++----
 t/t6030-bisect-porcelain.sh  |   30 +++++++++++++++---------------
 5 files changed, 34 insertions(+), 25 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/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d96a193..8d325a5 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -16,7 +16,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "next-all", &next_all,
 			    "perform 'git bisect next'"),
 		OPT_BOOLEAN(0, "no-checkout", &no_checkout,
-			    "update HEAD instead of checking out the current commit"),
+			    "update BISECT_HEAD instead of checking out the current commit"),
 		OPT_END()
 	};
 
diff --git a/git-bisect.sh b/git-bisect.sh
index 99a2637..9792a84 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" = "--no-checkout"; 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" = "--no-checkout"; 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" = "--no-checkout"; 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.gb3ae7e

^ permalink raw reply related

* Re: [PATCH v13 4/8] bisect: introduce support for --no-checkout option.
From: Jon Seymour @ 2011-08-02 22:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, chriscool, j6t, jnareb
In-Reply-To: <7vsjpjcvv2.fsf@alter.siamese.dyndns.org>

On Wed, Aug 3, 2011 at 8:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jon Seymour <jon.seymour@gmail.com> writes:
>
>>> I think this is a regression from naming perspective from the previous
>>> round. You would be either the normal (checkout) mode or no-checkout mode,
>>> and honestly, --no-checkout would be understood by anybody while update-ref
>>> would be understood only by Gitz.
>>>
>>
>> Is it relevant here that --bisect-mode here is on bisect--helper which
>> is really part of the plumbing and unlikely to be used by anything
>> other than git's own porcelain?
>
> Ah, Ok, that is what I missed. I also was scratching my head after seeing
> that there was not much change in the proposed log messages from the
> earlier round.
>
>

I've issued a v14 that removes it - I was treating it as a boolean in
the C code anyway.

If anything v14 git-bisect.sh is slightly simpler than v13

jon,

^ permalink raw reply

* [PATCH] git-am: ignore leading whitespace before patch
From: David Barr @ 2011-08-02 22:20 UTC (permalink / raw)
  To: Git Mailing List; +Cc: David Barr

Some web-based email clients prepend whitespace to raw message
transcripts to workaround content-sniffing in some browsers.
Adjust the patch format detection logic to ignore leading
whitespace.

Signed-off-by: David Barr <davidbarr@google.com>
---
 git-am.sh |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 463c741..19b2f0f 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -199,7 +199,11 @@ check_patch_format () {
 	# otherwise, check the first few lines of the first patch to try
 	# to detect its format
 	{
-		read l1
+		# Start from first line containing non-whitespace
+		until [ -n "$l1" ]
+		do
+			read l1
+		done
 		read l2
 		read l3
 		case "$l1" in
-- 
1.7.6

^ permalink raw reply related

* Re: [PATCH] describe: Refresh the index when run with --dirty
From: Allan Caffee @ 2011-08-02 22:38 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King; +Cc: git, Sverre Rabbelier
In-Reply-To: <7v1ux3eapk.fsf@alter.siamese.dyndns.org>

On Tue, Aug 2, 2011 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Thanks.
>
> Here is a minor fix-up on top, that can be squashed in a re-roll (if you
> plan to do one).

Thanks for the patch.  I'll squash this into v2.

On Sun, Jul 31, 2011 at 11:51 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Jul 31, 2011 at 09:52:41PM -0400, Allan Caffee wrote:
>
>> When running git describe --dirty the index should be refreshed.  Previously
>> the cached index would cause describe to think that the index was dirty when,
>> in reality, it was just stale.
>>
>> The issue was exposed by python setuptools which hardlinks files into another
>> directory when building a distribution.
>
> Overall, looks good to me. A few minor nits, though:
>
>> diff --git a/builtin/describe.c b/builtin/describe.c
>> index 66fc291..792af76 100644
>> --- a/builtin/describe.c
>> +++ b/builtin/describe.c
>> @@ -24,6 +24,7 @@ static int longformat;
>>  static int abbrev = -1; /* unspecified */
>>  static int max_candidates = 10;
>>  static struct hash_table names;
>> +static struct lock_file index_lock; /* real index */
>
> This line was presumably copied straight from builtin/commit.c. You can
> drop the "real index" comment here. Commit may deal with multiple
> indices, which is what this comment was clarifying, but here it doesn't
> make any sense.
>
>>  static int always;
>> @@ -399,6 +400,7 @@ static void describe(const char *arg, int last_one)
>>  int cmd_describe(int argc, const char **argv, const char *prefix)
>>  {
>>       int contains = 0;
>> +     int fd;
>
> If a variable is only going to be used for one deep conditional, IMHO
> it's nice to declare it inside the conditional block, so readers of the
> code don't have to wonder under what conditions fd is valid.
>
>> +             if (dirty) {
>> +                     read_cache();
>> +                     refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
>> +                     fd = hold_locked_index(&index_lock, 0);
>> +                     if (0 <= fd)
>> +                             update_index_if_able(&the_index, &index_lock);
>
> A few questions about this read_cache call:
>
>  1. Should this actually be:
>
>          if (read_cache() < 0)
>                  die("unable to read cache");
>
>     ? I notice that cmd_status also does not check the error code. But
>     it seems like if we fail to read, we would then potentially write
>     out a bogus index. Probably unlikely, as failure to read probably
>     implies failure to write.

It definitely seems like writing out a bogus index would be bad, but
even if both the read *and* the write fail we would still be
potentially mislabeling it as "dirty" if we failed to it in the first
place.  It seems like, since they explicitly requested --dirty, we
ought to give up here since we can't accurately respond.

>  2. Should the read and refresh happen while we hold the lock?
>     Otherwise our read-modify-update is not atomic, and we risk
>     overwriting another index writer. Again, cmd_status suffers from
>     the same problem, so this is not something you are introducing.

Yeah it definitely is a race condition as far as I can tell.  Should
this be changed in cmd_status (builtin/commit.c:1227-1232) as well?

--
Allan

^ permalink raw reply

* Re: [PATCH v14 5/8] bisect: introduce --no-checkout support into porcelain.
From: Junio C Hamano @ 2011-08-02 23:01 UTC (permalink / raw)
  To: Jon Seymour; +Cc: git, chriscool, j6t, jnareb
In-Reply-To: <1312323362-20096-6-git-send-email-jon.seymour@gmail.com>

Jon Seymour <jon.seymour@gmail.com> writes:

> +	BISECT_MODE=
>  	while [ $# -gt 0 ]; do
>  	    arg="$1"
>  	    case "$arg" in
> @@ -76,6 +79,11 @@ bisect_start() {
>  		shift
>  		break
>  		;;
> +	    --no-checkout)
> +		BISECT_MODE=--no-checkout;
> +		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" = "--no-checkout"; then
> +		    git update-ref --no-deref HEAD "$start_head"
> +		else
> +		    git checkout "$start_head" --
> +		fi

Just a minor worry but I would not be surprised if somebody's "test"
implementation barfs upon:

	test "--no-checkout" = "--no-checkout"

mistaking the string with a dash at the beginning as an option unknown to
it. That is why we often have "z$variable" in our comparison, like so:

	if test "z$BISECT_MODE" = "z--no-checkout"
        then
        	git update-ref --no-deref BISECT_HEAD "$start_head"
	else
        	git checkout "$start_head" --
	fi

> -	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" &&

&&?

^ permalink raw reply

* Re: [PATCH v14 5/8] bisect: introduce --no-checkout support into porcelain.
From: Junio C Hamano @ 2011-08-02 23:16 UTC (permalink / raw)
  To: Jon Seymour; +Cc: git, chriscool, j6t, jnareb
In-Reply-To: <7voc07ct9z.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Jon Seymour <jon.seymour@gmail.com> writes:
> ...
>> +		if test "$BISECT_MODE" = "--no-checkout"; then
>> +		    git update-ref --no-deref HEAD "$start_head"
>> +		else
>> +		    git checkout "$start_head" --
>> +		fi
>
> Just a minor worry but I would not be surprised if somebody's "test"
> implementation barfs upon:
>
> 	test "--no-checkout" = "--no-checkout"
>
> mistaking the string with a dash at the beginning as an option unknown to
> it. That is why we often have "z$variable" in our comparison, like so:
>
> 	if test "z$BISECT_MODE" = "z--no-checkout"
>         then
>         	git update-ref --no-deref BISECT_HEAD "$start_head"
> 	else
>         	git checkout "$start_head" --
> 	fi
>
>> -	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" &&
>
> &&?

Having said that, other than these minor nits, I think this round is
almost ready. I didn't check how it behaves upon "bisect reset",
though. It shouldn't touch the index, HEAD nor the working tree (it
probably is just the matter of "update-ref -d BISECT_HEAD" and nothing
else, but I haven't thought things through thoroughly).

Further polishing we may want to do while it is still in pu/next I can
think of off the top of my head are:

 - In this mode, I can bisect the history even inside a bare repository,
   as the whole point of --no-checkout is that the mode does not require a
   working tree. I however suspect "git bisect" requires working tree. Is
   this something we want to fix?

 - Further, perhaps should we default to this mode inside a bare
   repository?

Christian, do you think of anything else?  Also do you see flaws in our
reasoning that updating only BISECT_HEAD and doing nothing else is a good
way to do this?

^ permalink raw reply

* Re: Branch dependencies
From: Bert Wesarg @ 2011-08-02 23:25 UTC (permalink / raw)
  To: martin f krafft; +Cc: git discussion list, Petr Baudis
In-Reply-To: <20110802190806.GA16674@fishbowl.rw.madduck.net>

Hi,

On Tue, Aug 2, 2011 at 21:08, martin f krafft <madduck@madduck.net> wrote:
> also sprach Bert Wesarg <bert.wesarg@googlemail.com> [2011.08.02.1506 +0200]:
>> while I appreciate, that you dig this topic up. I think you are trying
>> to solve the wrong problem first. My main problem with the TopGit
>> approach is, that you can't freely change the dependencies of a topic.
>> This may be not the most common case in distro development. But in my
>> eyes more problematic than maintaining the meta data.
>
> Hello Bert, thank you for taking the time to respond!
>
> Could you please try to illuminate me a bit on a use-case of
> changing dependencies? I am aware that TopGit has had a problem with
> changing dependencies due to renamed branches, and I think I have
> a solution to that (encode the dependent ref, not the branch head),
> but I cannot come up with a use case for freely changing
> dependencies just like that.

Not each feature branch may end up in master. And there does not need
to be one feature branch which depends on all other features. For the
latter you have probably an empty feature branch which just depends on
all features. I call this branch mostly 'tip'. Removing a feature
branch from the tips dependency list only with merges can't be done
right now, and the proposed solutions never reached a usable state.

My second usecase is to convert a big quilt patch series into TopGit.
Such big Quilt patches have mostly an artificial dependency to its
predecessors. Removing these artifical dependencies makes it necessary
to remove dependencies from patches.

>
>> For my first mentioned problem, I think a new 'system' needs to be
>> 'rebase' based, not merge based like TopGit.
>
> The problem with rebasing is that you cannot publish the branches.

That doesn't hold you back to publish them. But the other side need to
know how to deal with them.

Saying that doesn't mean I know a good way to deal with them. I mostly
end up using plumbing commands to deal with this.

>
> However, maybe I am simply not seeing the light here. Do you have
> some further ideas about what this would be like? Please keep in
> mind that what I seek is not just a way to bring feature branches
> up-to-date with upstream, but also to have those branches be shared
> among developers.

I think that having the TopGit philosophy of one feature branch is one
patch, you can handle an rebased upstream. Thinking of a feature
branch as a series of patches makes this way harder. But I would like
to have this philosophy.

Bert

>
> Thanks,
>

^ 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