threads / patch / 23065

patchgit checkout: create unparented branch by --orphan

Subject: [PATCH] git checkout: create unparented branch by --orphan

## tl;dr

7 messages between Mar 18, 2010 and Mar 20, 2010. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Erick Mattos· Mar 18, 2010, 16:09 UTC · lore

Similar to -b, --orphan creates a new branch, but it starts without any commit. After running "git checkout --orphan newbranch", you are on a new branch "newbranch", and the first commit you create from this state will start a new history without any ancestry.

"git checkout --orphan" keeps the index and the working tree files intact in order to make it convenient for creating a new history whose trees resemble the ones from the original branch.

When creating a branch whose trees have no resemblance to the ones from the original branch, it may be easier to start work on the new branch by untracking and removing all working tree files that came from the original branch, by running a 'git rm -rf .' immediately after running "checkout --orphan".

Signed-off-by: Erick Mattos <erick.mattos@gmail.com>
---

Complete rewrite of --orphan functionality to fit it in the new design goals set by Junio. Now --orphan is not anymore an option of -b. It is an alternative to it.

Junio:  I ask you to reconsider only the giving of the "short-and-sweet" -o
from beginning because of the new design.

As it is now an alternative to -b, doing similar stuff, I think it will be nice if they look similar too. So the it will be [[-b|-o] <new_branch>] not [[-b|--orphan] <new_branch>].

Please ignore it if you can preview future options to checkout that could take the rest of the 21 remaining letters and 6 digits.

Regards
 Documentation/git-checkout.txt |   21 +++++++++++++++-
 builtin/checkout.c             |   19 +++++++++++++-
 t/t2017-checkout-orphan.sh     |   53 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100755 t/t2017-checkout-orphan.sh
Show changes to 3 files +90 −3

Documentation/git-checkout.txt, builtin/checkout.c, t/t2017-checkout-orphan.sh

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 37c1810..18df834 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git checkout' [-q] [-f] [-m] [<branch>]
-'git checkout' [-q] [-f] [-m] [-b <new_branch>] [<start_point>]
+'git checkout' [-q] [-f] [-m] [[-b|--orphan] <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
 'git checkout' --patch [<tree-ish>] [--] [<paths>...]
 
@@ -20,6 +20,19 @@ When <paths> are not given, this command switches branches by
 updating the index, working tree, and HEAD to reflect the specified
 branch.
 
+When you use "--orphan", a new unparented branch is created having the
+index and the working tree intact.  This allows you to start a new
+history that records set of paths similar to that of the start-point
+commit, which is useful when you want to keep different branches for
+different audiences you are working to like when you have an open source
+and commercial versions of a software, for example.
+
+If you want to start a disconnected history that records set of paths
+totally different from the original branch, you may want to first clear
+the index and the working tree, by running "git rm -rf ." from the
+top-level of the working tree, before preparing your files (by copying
+from elsewhere, extracting a tarball, etc.) in the working tree.
+
 If `-b` is given, a new branch is created and checked out, as if
 linkgit:git-branch[1] were called; in this case you can
 use the --track or --no-track options, which will be passed to `git
@@ -63,6 +76,12 @@ entries; instead, unmerged entries are ignored.
 	When checking out paths from the index, check out stage #2
 	('ours') or #3 ('theirs') for unmerged paths.
 
+--orphan::
+	Create a new branch named <new_branch>, unparented to any other
+	branch.  The new branch you switch to does not have any commit
+	and after the first one it will become the root of a new history
+	completely unconnected from all the other branches.
+
 -b::
 	Create a new branch named <new_branch> and start it at
 	<start_point>; see linkgit:git-branch[1] for details.
diff --git a/builtin/checkout.c b/builtin/checkout.c
index acefaaf..9e0af6a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -33,6 +33,7 @@ struct checkout_opts {
 	int writeout_error;
 
 	const char *new_branch;
+	const char *new_orphan_branch;
 	int new_branch_log;
 	enum branch_track track;
 };
@@ -491,8 +492,15 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 	struct strbuf msg = STRBUF_INIT;
 	const char *old_desc;
 	if (opts->new_branch) {
-		create_branch(old->name, opts->new_branch, new->name, 0,
-			      opts->new_branch_log, opts->track);
+		if (opts->new_orphan_branch) {
+			unsigned char rev[20];
+			int flag;
+			if (old->path != resolve_ref("HEAD", rev, 0, &flag))
+				cmd_checkout(1, &old->name, NULL);
+		}
+		else
+			create_branch(old->name, opts->new_branch, new->name, 0,
+				      opts->new_branch_log, opts->track);
 		new->name = opts->new_branch;
 		setup_branch_path(new);
 	}
@@ -629,6 +637,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
+		OPT_STRING(0, "orphan", &opts.new_orphan_branch, "new branch", "new unparented branch"),
 		OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"),
 		OPT_SET_INT('t', "track",  &opts.track, "track",
 			BRANCH_TRACK_EXPLICIT),
@@ -677,6 +686,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		opts.new_branch = argv0 + 1;
 	}
 
+	if (opts.new_orphan_branch) {
+		if (opts.new_branch)
+			die("--orphan and -b are mutually exclusive");
+		opts.new_branch = opts.new_orphan_branch;
+	}
+
 	if (conflict_style) {
 		opts.merge = 1; /* implied */
 		git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
new file mode 100755
index 0000000..e6d88b1
--- /dev/null
+++ b/t/t2017-checkout-orphan.sh
@@ -0,0 +1,53 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Erick Mattos
+#
+
+test_description='git checkout --orphan
+
+Main Tests for --orphan functionality.'
+
+. ./test-lib.sh
+
+TEST_FILE=foo
+
+test_expect_success 'Setup' '
+	echo "Initial" >"$TEST_FILE" &&
+	git add "$TEST_FILE" &&
+	git commit -m "First Commit"
+	test_tick &&
+	echo "State 1" >>"$TEST_FILE" &&
+	git add "$TEST_FILE" &&
+	git commit -m "Second Commit"
+'
+
+test_expect_success '--orphan creates a new orphan branch from HEAD' '
+	git checkout --orphan alpha &&
+	test_must_fail PAGER= git log >/dev/null 2>/dev/null &&
+	test "alpha" = "$(git symbolic-ref HEAD | sed "s,.*/,,")" &&
+	test_tick &&
+	git commit -m "Third Commit" &&
+	test 0 -eq $(git cat-file -p HEAD | grep "^parent" | wc -l) &&
+	git cat-file -p master | grep "^tree" >base &&
+	git cat-file -p HEAD | grep "^tree" >actual &&
+	test_cmp base actual
+'
+
+test_expect_success '--orphan creates a new orphan branch from <start_point>' '
+	git checkout master &&
+	git checkout --orphan beta master^ &&
+	test_must_fail PAGER= git log >/dev/null 2>/dev/null &&
+	test "beta" = "$(git symbolic-ref HEAD | sed "s,.*/,,")" &&
+	test_tick &&
+	git commit -m "Fourth Commit" &&
+	test 0 -eq $(git cat-file -p HEAD | grep "^parent" | wc -l) &&
+	git cat-file -p master^ | grep "^tree" >base &&
+	git cat-file -p HEAD | grep "^tree" >actual &&
+	test_cmp base actual
+'
+
+test_expect_success '--orphan must be rejected with -b' '
+	test_must_fail git checkout --orphan new -b newer
+'
+
+test_done
-- 
1.7.0.2.280.g460a6
Junio C Hamano· Mar 20, 2010, 15:13 UTC · re: Erick Mattos · lore

Re: [PATCH] git checkout: create unparented branch by --orphan

Erick Mattos <erick.mattos@gmail.com> writes:
> Complete rewrite of --orphan functionality to fit it in the new design goals
> set by Junio.  Now --orphan is not anymore an option of -b.  It is an
> alternative to it.

You are giving me too much credit. I just made a suggestion, you had the choice to agree to or disagree with it, and I am hoping that you made the final decision to rewrite the patch to match what I suggested not because you wanted a "commit count" in the project with any form of a change, but because you thought the suggested approach made much more sense than the earlier iterations. And in that case, the final decision to submit the patch in this form was due to _you_. Don't give me undue credit---I was just helping from the sideline.

> Junio:  I ask you to reconsider only the giving of the "short-and-sweet" -o
> from beginning because of the new design.

No. I don't want our hand to be tied with something that is clearly a corner case feature from the beginning. It wouldn't be too late to add one later, after it proves something people do every day (or even every hour).

Show 30 quoted lines
>  Documentation/git-checkout.txt |   21 +++++++++++++++-
>  builtin/checkout.c             |   19 +++++++++++++-
>  t/t2017-checkout-orphan.sh     |   53 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 90 insertions(+), 3 deletions(-)
>  create mode 100755 t/t2017-checkout-orphan.sh
>
> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
> index 37c1810..18df834 100644
> --- a/Documentation/git-checkout.txt
> +++ b/Documentation/git-checkout.txt
> @@ -20,6 +20,19 @@ When <paths> are not given, this command switches branches by
>  updating the index, working tree, and HEAD to reflect the specified
>  branch.
>  
> +When you use "--orphan", a new unparented branch is created having the
> +index and the working tree intact.  This allows you to start a new
> +history that records set of paths similar to that of the start-point
> +commit, which is useful when you want to keep different branches for
> +different audiences you are working to like when you have an open source
> +and commercial versions of a software, for example.
> +
> +If you want to start a disconnected history that records set of paths
> +totally different from the original branch, you may want to first clear
> +the index and the working tree, by running "git rm -rf ." from the
> +top-level of the working tree, before preparing your files (by copying
> +from elsewhere, extracting a tarball, etc.) in the working tree.
> +
>  If `-b` is given, a new branch is created and checked out, as if
>  linkgit:git-branch[1] were called; in this case you can
>  use the --track or --no-track options, which will be passed to `git

It feels wrong to talk about --orphan before readers learned -b. Does the text flow nicely if we just swap the paragraphs? Better yet, perhaps we shouldn't have any of this in the general description section. Move this and consolidate it with the text in "OPTIONS" section under "--orphan"?

Show 13 quoted lines
> @@ -63,6 +76,12 @@ entries; instead, unmerged entries are ignored.
>  	When checking out paths from the index, check out stage #2
>  	('ours') or #3 ('theirs') for unmerged paths.
>  
> +--orphan::
> +	Create a new branch named <new_branch>, unparented to any other
> +	branch.  The new branch you switch to does not have any commit
> +	and after the first one it will become the root of a new history
> +	completely unconnected from all the other branches.
> +
>  -b::
>  	Create a new branch named <new_branch> and start it at
>  	<start_point>; see linkgit:git-branch[1] for details.

Likewise. I think "--orphan" should come after -b/-t/-l (all about the normal cases of switching to a different branch either new or existing).

Oh, by the way, how does this new option work with -t? I think the combination should be rejected, no?

Show 11 quoted lines
> @@ -491,8 +492,15 @@ static void update_refs_for_switch(struct checkout_opts *opts,
>  	struct strbuf msg = STRBUF_INIT;
>  	const char *old_desc;
>  	if (opts->new_branch) {
> -		create_branch(old->name, opts->new_branch, new->name, 0,
> -			      opts->new_branch_log, opts->track);
> +		if (opts->new_orphan_branch) {
> +			unsigned char rev[20];
> +			int flag;
> +			if (old->path != resolve_ref("HEAD", rev, 0, &flag))
> +				cmd_checkout(1, &old->name, NULL);
Can you explain what this is doing?
 - the "if" compares the addresses of two strings; did you mean it?
 - It is like recursively calling main() which is rather unusual and error
   prone;
 - what happens when cmd_checkout() fails, perhaps due to local changes?

I have a suspicion that you are trying "git co-o new old" case where old is not the current branch (so that you would want to adjust the index and the working tree files to that of "old" while carrying local changes forward), _but_ if that is the case that should have been already done with the logic in switch_branches() where it calls merge_working_tree(), and not here in the call chain.

Show 9 quoted lines
> @@ -677,6 +686,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  		opts.new_branch = argv0 + 1;
>  	}
>  
> +	if (opts.new_orphan_branch) {
> +		if (opts.new_branch)
> +			die("--orphan and -b are mutually exclusive");
> +		opts.new_branch = opts.new_orphan_branch;
> +	}

How should this interact with opts.track? I think (and please correct me if you disagree):

 - "git checkout -t --orphan new [old]" should fail; new is rootless and
   does not track either the current branch or "old";
 - opts.track that came from git_branch_track (default read from config)
   should not cause a failure, but be silently ignored.
Show 27 quoted lines
> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
> new file mode 100755
> index 0000000..e6d88b1
> --- /dev/null
> +++ b/t/t2017-checkout-orphan.sh
> @@ -0,0 +1,53 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2010 Erick Mattos
> +#
> +
> +test_description='git checkout --orphan
> +
> +Main Tests for --orphan functionality.'
> +
> +. ./test-lib.sh
> +
> +TEST_FILE=foo
> +
> +test_expect_success 'Setup' '
> +	echo "Initial" >"$TEST_FILE" &&
> +	git add "$TEST_FILE" &&
> +	git commit -m "First Commit"
> +	test_tick &&
> +	echo "State 1" >>"$TEST_FILE" &&
> +	git add "$TEST_FILE" &&
> +	git commit -m "Second Commit"
No tick before this one?
Show 6 quoted lines
> +'
> +
> +test_expect_success '--orphan creates a new orphan branch from HEAD' '
> +	git checkout --orphan alpha &&
> +	test_must_fail PAGER= git log >/dev/null 2>/dev/null &&
> +	test "alpha" = "$(git symbolic-ref HEAD | sed "s,.*/,,")" &&
What is this PAGER= doing here?
I think it is more direct to check that:
 - "rev-parse --verify HEAD" fails; and
 - "symbolic-ref HEAD" says refs/heads/alpha.
> +	test_tick &&
> +	git commit -m "Third Commit" &&
Tricky but correct ;-)
> +	test 0 -eq $(git cat-file -p HEAD | grep "^parent" | wc -l) &&
    test_must_fail git rev-parse --verify HEAD^
> +	git cat-file -p master | grep "^tree" >base &&
    git rev-parse master^{tree} >base
> +	git cat-file -p HEAD | grep "^tree" >actual &&
> +	test_cmp base actual
> +'
Or you can just replace the above three lines with:
	git diff-tree --quiet master alpha
> +test_expect_success '--orphan creates a new orphan branch from <start_point>' '
> ...
> +'
Similar.
> +test_expect_success '--orphan must be rejected with -b' '
> +	test_must_fail git checkout --orphan new -b newer
> +'

Ok. You have the basics covered well; there are a few more things to test, I think.

With local changes in the index/working tree without "start commit" (which should never fail) and with "start commit" (which should fail if HEAD and start commit has differences to the same paths as you have local changes to).

Also you would want to check with -t, --no-t, branch.autosetupmrebe set to always in the configuration with -t and without -t from the command line,

Thanks.
Erick Mattos· Mar 20, 2010, 19:06 UTC · re: Junio C Hamano · lore

Re: [PATCH] git checkout: create unparented branch by --orphan

Hi,
2010/3/20 Junio C Hamano <gitster@pobox.com>:
Show 14 quoted lines
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> Complete rewrite of --orphan functionality to fit it in the new design goals
>> set by Junio.  Now --orphan is not anymore an option of -b.  It is an
>> alternative to it.
>
> You are giving me too much credit.  I just made a suggestion, you had the
> choice to agree to or disagree with it, and I am hoping that you made the
> final decision to rewrite the patch to match what I suggested not because
> you wanted a "commit count" in the project with any form of a change, but
> because you thought the suggested approach made much more sense than the
> earlier iterations.  And in that case, the final decision to submit the
> patch in this form was due to _you_.  Don't give me undue credit---I was
> just helping from the sideline.

If I was going to look for a commit count I would do spelling/grammar corrections. :-)

This is the second feature I am trying to add to git. Both changes of mine are features I use my own. On my compiled and customized version of git. I thought they were good and tried to share following the best free software spirit (I have been always feeling guilty of just using free software). I am not a new developer trying to be known. I am not even "new"... :-)

Anyway I think you will not hear of me anymore after we finish this work. I think I did enough for this project and I really don't feel myself fitted to the task.

I realized your last critic was indeed good and that it worthed the changes.

But I have to confess that I am a little impatient to end this development cycle though. So trying not to argue was a change of behavior. ;-)

Show 7 quoted lines
>> Junio:  I ask you to reconsider only the giving of the "short-and-sweet" -o
>> from beginning because of the new design.
>
> No.  I don't want our hand to be tied with something that is clearly a
> corner case feature from the beginning.  It wouldn't be too late to add
> one later, after it proves something people do every day (or even every
> hour).
If it is because of its importance I think it will never deserve the upgrade.

But I was arguing because I imagine checkout will never need the 21 remaining letters and that it was good for cosmetic and similarity (to his brother -b) reasons.

Also because I am more used to one letter options from previous operational systems experience (let's forget that! :-) ).

Show 35 quoted lines
>>  Documentation/git-checkout.txt |   21 +++++++++++++++-
>>  builtin/checkout.c             |   19 +++++++++++++-
>>  t/t2017-checkout-orphan.sh     |   53 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 90 insertions(+), 3 deletions(-)
>>  create mode 100755 t/t2017-checkout-orphan.sh
>>
>> diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
>> index 37c1810..18df834 100644
>> --- a/Documentation/git-checkout.txt
>> +++ b/Documentation/git-checkout.txt
>> @@ -20,6 +20,19 @@ When <paths> are not given, this command switches branches by
>>  updating the index, working tree, and HEAD to reflect the specified
>>  branch.
>>
>> +When you use "--orphan", a new unparented branch is created having the
>> +index and the working tree intact.  This allows you to start a new
>> +history that records set of paths similar to that of the start-point
>> +commit, which is useful when you want to keep different branches for
>> +different audiences you are working to like when you have an open source
>> +and commercial versions of a software, for example.
>> +
>> +If you want to start a disconnected history that records set of paths
>> +totally different from the original branch, you may want to first clear
>> +the index and the working tree, by running "git rm -rf ." from the
>> +top-level of the working tree, before preparing your files (by copying
>> +from elsewhere, extracting a tarball, etc.) in the working tree.
>> +
>>  If `-b` is given, a new branch is created and checked out, as if
>>  linkgit:git-branch[1] were called; in this case you can
>>  use the --track or --no-track options, which will be passed to `git
>
> It feels wrong to talk about --orphan before readers learned -b.  Does the
> text flow nicely if we just swap the paragraphs?  Better yet, perhaps we
> shouldn't have any of this in the general description section.  Move this
> and consolidate it with the text in "OPTIONS" section under "--orphan"?
As you wish.
Show 16 quoted lines
>> @@ -63,6 +76,12 @@ entries; instead, unmerged entries are ignored.
>>       When checking out paths from the index, check out stage #2
>>       ('ours') or #3 ('theirs') for unmerged paths.
>>
>> +--orphan::
>> +     Create a new branch named <new_branch>, unparented to any other
>> +     branch.  The new branch you switch to does not have any commit
>> +     and after the first one it will become the root of a new history
>> +     completely unconnected from all the other branches.
>> +
>>  -b::
>>       Create a new branch named <new_branch> and start it at
>>       <start_point>; see linkgit:git-branch[1] for details.
>
> Likewise.  I think "--orphan" should come after -b/-t/-l (all about the
> normal cases of switching to a different branch either new or existing).
All right.
> Oh, by the way, how does this new option work with -t?  I think the
> combination should be rejected, no?

Yes it should. And it is. By --track itself which is directly connected to -b. Test done just before --orphan test by someone else.

Show 6 quoted lines
> I have a suspicion that you are trying "git co-o new old" case where old
> is not the current branch (so that you would want to adjust the index and
> the working tree files to that of "old" while carrying local changes
> forward), _but_ if that is the case that should have been already done
> with the logic in switch_branches() where it calls merge_working_tree(),
> and not here in the call chain.
My fault.  Forget that.  Fast unverified work in a hurry. :-\
Show 18 quoted lines
>> @@ -677,6 +686,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>>               opts.new_branch = argv0 + 1;
>>       }
>>
>> +     if (opts.new_orphan_branch) {
>> +             if (opts.new_branch)
>> +                     die("--orphan and -b are mutually exclusive");
>> +             opts.new_branch = opts.new_orphan_branch;
>> +     }
>
> How should this interact with opts.track?  I think (and please correct
> me if you disagree):
>
>  - "git checkout -t --orphan new [old]" should fail; new is rootless and
>   does not track either the current branch or "old";
>
>  - opts.track that came from git_branch_track (default read from config)
>   should not cause a failure, but be silently ignored.
It is just working like you have pictured.
Show 29 quoted lines
>> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
>> new file mode 100755
>> index 0000000..e6d88b1
>> --- /dev/null
>> +++ b/t/t2017-checkout-orphan.sh
>> @@ -0,0 +1,53 @@
>> +#!/bin/sh
>> +#
>> +# Copyright (c) 2010 Erick Mattos
>> +#
>> +
>> +test_description='git checkout --orphan
>> +
>> +Main Tests for --orphan functionality.'
>> +
>> +. ./test-lib.sh
>> +
>> +TEST_FILE=foo
>> +
>> +test_expect_success 'Setup' '
>> +     echo "Initial" >"$TEST_FILE" &&
>> +     git add "$TEST_FILE" &&
>> +     git commit -m "First Commit"
>> +     test_tick &&
>> +     echo "State 1" >>"$TEST_FILE" &&
>> +     git add "$TEST_FILE" &&
>> +     git commit -m "Second Commit"
>
> No tick before this one?
I will add another then.
Show 17 quoted lines
>> +'
>> +
>> +test_expect_success '--orphan creates a new orphan branch from HEAD' '
>> +     git checkout --orphan alpha &&
>> +     test_must_fail PAGER= git log >/dev/null 2>/dev/null &&
>> +     test "alpha" = "$(git symbolic-ref HEAD | sed "s,.*/,,")" &&
>
> What is this PAGER= doing here?
>
> I think it is more direct to check that:
>
>  - "rev-parse --verify HEAD" fails; and
>  - "symbolic-ref HEAD" says refs/heads/alpha.
>> +     test_tick &&
>> +     git commit -m "Third Commit" &&
>
> Tricky but correct ;-)
Indeed.  Easier your way though.
Show 25 quoted lines
>> +     test 0 -eq $(git cat-file -p HEAD | grep "^parent" | wc -l) &&
>
>    test_must_fail git rev-parse --verify HEAD^
>
>> +     git cat-file -p master | grep "^tree" >base &&
>
>    git rev-parse master^{tree} >base
>
>> +     git cat-file -p HEAD | grep "^tree" >actual &&
>> +     test_cmp base actual
>> +'
>
> Or you can just replace the above three lines with:
>
>        git diff-tree --quiet master alpha
>
>> +test_expect_success '--orphan creates a new orphan branch from <start_point>' '
>> ...
>> +'
>
> Similar.
>
>> +test_expect_success '--orphan must be rejected with -b' '
>> +     test_must_fail git checkout --orphan new -b newer
>> +'
All your suggestions are going to be added.
Show 7 quoted lines
> Ok.  You have the basics covered well; there are a few more things to
> test, I think.
>
> With local changes in the index/working tree without "start commit" (which
> should never fail) and with "start commit" (which should fail if HEAD and
> start commit has differences to the same paths as you have local changes
> to).

It is behaving like that already and that is intrinsically a switch_branches() logic, not a special --orphan need. It is not part of this implementation and It is probably tested elsewhere (you probably do know where).

> Also you would want to check with -t, --no-t, branch.autosetupmrebe set to
> always in the configuration with -t and without -t from the command line,

The actual implementation is just ignoring track and -t is not even allowed. So it is pointless.

> Thanks.
>
I will be sending a new patch version soon.
Regards
Junio C Hamano· Mar 20, 2010, 20:30 UTC · re: Erick Mattos · lore

Re: [PATCH] git checkout: create unparented branch by --orphan

Erick Mattos <erick.mattos@gmail.com> writes:
Show 15 quoted lines
>> With local changes in the index/working tree without "start commit" (which
>> should never fail) and with "start commit" (which should fail if HEAD and
>> start commit has differences to the same paths as you have local changes
>> to).
>
> It is behaving like that already and that is intrinsically a
> switch_branches() logic, not a special --orphan need.  It is not part
> of this implementation and It is probably tested elsewhere (you
> probably do know where).
>
>> Also you would want to check with -t, --no-t, branch.autosetupmrebe set to
>> always in the configuration with -t and without -t from the command line,
>
> The actual implementation is just ignoring track and -t is not even
> allowed.  So it is pointless.

I think you misunderstood the point of having tests. It is not about demonstrating that you did a good job implementing the new feature, or your implementation works as advertised in the submitted form. That is the job of the review process before patch acceptance.

Tests are to pretect what you perfected during the patch acceptance review from getting broken by other people in the future, while you are not closely monitoring the mailing list traffic. Many people, me included, tend to concentrate on their own new addition, without being careful enough not to break the existing features. If "-t --orphan" should result in an error, it should result in an error even after somebody restructures the code, so it is not sufficient that it is obvious in the _current_ code structure that breakage of that feature is unlikely.

If you can promise that you will be around on this list forever, and that every time somebody posts patches to the related areas, you will make sure that the changes do not inadvertently break this feature and respond to the patches that do break it before they hit my tree, then theoretically we do not need to have any test to make sure this feature keeps working as advertised. But we cannot ask that kind of time/attention commitment from anybody.

Erick Mattos· Mar 20, 2010, 20:36 UTC · re: Junio C Hamano · lore

Re: [PATCH] git checkout: create unparented branch by --orphan

2010/3/20 Junio C Hamano <gitster@pobox.com>:
Show 40 quoted lines
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>>> With local changes in the index/working tree without "start commit" (which
>>> should never fail) and with "start commit" (which should fail if HEAD and
>>> start commit has differences to the same paths as you have local changes
>>> to).
>>
>> It is behaving like that already and that is intrinsically a
>> switch_branches() logic, not a special --orphan need.  It is not part
>> of this implementation and It is probably tested elsewhere (you
>> probably do know where).
>>
>>> Also you would want to check with -t, --no-t, branch.autosetupmrebe set to
>>> always in the configuration with -t and without -t from the command line,
>>
>> The actual implementation is just ignoring track and -t is not even
>> allowed.  So it is pointless.
>
> I think you misunderstood the point of having tests.  It is not about
> demonstrating that you did a good job implementing the new feature, or
> your implementation works as advertised in the submitted form.  That is
> the job of the review process before patch acceptance.
>
> Tests are to pretect what you perfected during the patch acceptance review
> from getting broken by other people in the future, while you are not
> closely monitoring the mailing list traffic.  Many people, me included,
> tend to concentrate on their own new addition, without being careful
> enough not to break the existing features.  If "-t --orphan" should result
> in an error, it should result in an error even after somebody restructures
> the code, so it is not sufficient that it is obvious in the _current_ code
> structure that breakage of that feature is unlikely.
>
> If you can promise that you will be around on this list forever, and that
> every time somebody posts patches to the related areas, you will make sure
> that the changes do not inadvertently break this feature and respond to
> the patches that do break it before they hit my tree, then theoretically
> we do not need to have any test to make sure this feature keeps working as
> advertised.  But we cannot ask that kind of time/attention commitment from
> anybody.
>

All right then. I am going to check it too. But in this particular case, the track is being ignored completely. So to break this behavior people will need to add code to --orphan.

That's not a break, it's a linkage!  :-)
Junio C Hamano· Mar 20, 2010, 20:54 UTC · re: Erick Mattos · lore

Re: [PATCH] git checkout: create unparented branch by --orphan

I hope I am not giving you undue burden, but here is what I would add. One thing that I am not sure about is what to do with "-l --orhpan".

---
 t/t2017-checkout-orphan.sh |   43 ++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 42 insertions(+), 1 deletions(-)
Show changes to t/t2017-checkout-orphan.sh +42 −1
diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
index 5a9a3fa..e80e167 100755
--- a/t/t2017-checkout-orphan.sh
+++ b/t/t2017-checkout-orphan.sh
@@ -44,7 +44,48 @@ test_expect_success '--orphan creates a new orphan branch from <start_point>' '
 '
 
 test_expect_success '--orphan must be rejected with -b' '
-	test_must_fail git checkout --orphan new -b newer
+	git checkout master &&
+	test_must_fail git checkout --orphan new -b newer &&
+	test refs/heads/master = "$(git symbolic-ref HEAD)"
+'
+
+test_expect_success '--orphan is rejected with an existing name' '
+	git checkout master &&
+	test_must_fail git checkout --orphan master &&
+	test refs/heads/master = "$(git symbolic-ref HEAD)"
+'
+
+test_expect_success '--orhapn refuses to switch if a merge is needed' '
+	git checkout master &&
+	git reset --hard &&
+	echo local >>"$TEST_FILE" &&
+	cat "$TEST_FILE" >"$TEST_FILE.saved" &&
+	test_must_fail git checkout --orphan gamma master^ &&
+	test refs/heads/master = "$(git symbolic-ref HEAD)" &&
+	test_cmp "$TEST_FILE" "$TEST_FILE.saved" &&
+	git diff-index --quiet --cached HEAD &&
+	git reset --hard
+'
+
+test_expect_success '--orphan does not mix well with -t' '
+	git checkout master &&
+	test_must_fail git checkout -t master --orphan gamma &&
+	test refs/heads/master = "$(git symbolic-ref HEAD)"
+'
+
+test_expect_success '--orphan ignores branch.autosetupmerge' '
+	git checkout -f master &&
+	git config branch.autosetupmerge always &&
+	git checkout --orphan delta &&
+	test -z "$(git config branch.delta.merge)" &&
+	test refs/heads/delta = "$(git symbolic-ref HEAD)" &&
+	test_must_fail git rev-parse --verify HEAD^
+'
+
+# This is iffy.
+test_expect_success '--orphan does not mix well with -l' '
+	git checkout -f master &&
+	test_must_fail git checkout -l --orphan gamma
 '
 
 test_done
Erick Mattos· Mar 20, 2010, 21:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] git checkout: create unparented branch by --orphan

2010/3/20 Junio C Hamano <gitster@pobox.com>:
> I hope I am not giving you undue burden, but here is what I would add.
> One thing that I am not sure about is what to do with "-l --orhpan".

No problem. I also like to do things right. And you have made it light with your attached tests.

About -l well... you tell.

← back to recent threads