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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:28

Erick Mattos [off-list ref] 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).
quoted hunk
 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"?
quoted hunk
@@ -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?
quoted hunk
@@ -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.
quoted hunk
@@ -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.
quoted hunk
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?
+'
+
+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.

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

From: Erick Mattos <hidden>
Date: 2016-06-15 22:48:28

Hi,

2010/3/20 Junio C Hamano [off-list ref]:
Erick Mattos [off-list ref] writes:
quoted
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. ;-)
quoted
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! :-) ).
quoted
 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.
quoted
@@ -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.
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. :-\
quoted
@@ -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.
quoted
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.
quoted
+'
+
+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.
quoted
+     test_tick &&
+     git commit -m "Third Commit" &&
Tricky but correct ;-)
Indeed.  Easier your way though.
quoted
+     test 0 -eq $(git cat-file -p HEAD | grep "^parent" | wc -l) &&
   test_must_fail git rev-parse --verify HEAD^
quoted
+     git cat-file -p master | grep "^tree" >base &&
   git rev-parse master^{tree} >base
quoted
+     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
quoted
+test_expect_success '--orphan creates a new orphan branch from <start_point>' '
...
+'
Similar.
quoted
+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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help