Re: [PATCH v15 7/7] bisect: add documentation for --no-checkout option.

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 3, 2011, 20:59 UTC
Message-ID: <7v62meb4a3.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1312383811-7130-8-git-send-email-jon.seymour@gmail.com>

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

> +This option is useful in circumstances in which checkout is either not
> +possible (because the repository is damaged) or is otherwise not required.

I do not think "because the repository is damaged" is a particularly good example to single out here. For one thing, you may not be able to bisect the history in a damaged repository due to unreadable commits. There are other valid and more plausible reasons that the users do not want to update the working tree while bisecting, e.g. they have local changes, they do not want to rebuild everything after bisection is done, etc.

Also the above description muddies what is prerequisite and what is user's wish.

 - "is otherwise not required" is the prerequisite: "if and only if the
   test you would perform during bisection does not need a checkout of the
   version being tested, you can use this option";

 - Even if the user wishes to use this option for various reasons (e.g. so
   that the next "make" wouldn't have to rebuild everything unnecessarily;
   or the repository has corrupt blobs and checkout of some versions may
   fail), if the test _needs_ a checkout, it is not an option to use this
   feature.

So I would suggest toning it down perhaps like this:

    This option may be useful when the test you would perform in each step
    does not require a checked out tree.

Show 7 quoted lines

> +$ 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;'

Hmm, "eval"?

More importantly, aren't these excessive and unnecessary semicolons unsightly?

	$ git bisect run sh -c '
                git rev-list --objects BISECT_HEAD >tmp.$$ &&
		git pack-objects --stdout >/dev/null <tmp.$$
                rc=$?
                rm -f tmp.$$
                test $rc = 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'.

Also "everything reachable from BISECT_HEAD" feels unnecessarily wasteful. By the time you invoke "bisect run", you already know one good commit, everything reachable from which are known to be good.