* [PATCH] git checkout: create unparented branch by --orphan
@ 2010-03-18 16:09 Erick Mattos
2010-03-20 15:13 ` Junio C Hamano
0 siblings, 1 reply; 7+ messages in thread
From: Erick Mattos @ 2010-03-18 16:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Erick Mattos
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
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] git checkout: create unparented branch by --orphan
2010-03-18 16:09 [PATCH] git checkout: create unparented branch by --orphan Erick Mattos
@ 2010-03-20 15:13 ` Junio C Hamano
2010-03-20 19:06 ` Erick Mattos
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2010-03-20 15:13 UTC (permalink / raw)
To: Erick Mattos; +Cc: git
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).
> 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"?
> @@ -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?
> @@ -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.
> @@ -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.
> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git checkout: create unparented branch by --orphan
2010-03-20 15:13 ` Junio C Hamano
@ 2010-03-20 19:06 ` Erick Mattos
2010-03-20 20:30 ` Junio C Hamano
0 siblings, 1 reply; 7+ messages in thread
From: Erick Mattos @ 2010-03-20 19:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hi,
2010/3/20 Junio C Hamano <gitster@pobox.com>:
> 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. ;-)
>> 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! :-) ).
>> 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.
>> @@ -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. :-\
>> @@ -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.
>> 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.
>> +'
>> +
>> +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.
>> + 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.
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git checkout: create unparented branch by --orphan
2010-03-20 19:06 ` Erick Mattos
@ 2010-03-20 20:30 ` Junio C Hamano
2010-03-20 20:36 ` Erick Mattos
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2010-03-20 20:30 UTC (permalink / raw)
To: Erick Mattos; +Cc: git
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git checkout: create unparented branch by --orphan
2010-03-20 20:30 ` Junio C Hamano
@ 2010-03-20 20:36 ` Erick Mattos
2010-03-20 20:54 ` Junio C Hamano
0 siblings, 1 reply; 7+ messages in thread
From: Erick Mattos @ 2010-03-20 20:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
2010/3/20 Junio C Hamano <gitster@pobox.com>:
> 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! :-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] git checkout: create unparented branch by --orphan
2010-03-20 20:36 ` Erick Mattos
@ 2010-03-20 20:54 ` Junio C Hamano
2010-03-20 21:37 ` Erick Mattos
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2010-03-20 20:54 UTC (permalink / raw)
To: Erick Mattos; +Cc: git
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(-)
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] git checkout: create unparented branch by --orphan
2010-03-20 20:54 ` Junio C Hamano
@ 2010-03-20 21:37 ` Erick Mattos
0 siblings, 0 replies; 7+ messages in thread
From: Erick Mattos @ 2010-03-20 21:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-03-20 21:38 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-18 16:09 [PATCH] git checkout: create unparented branch by --orphan Erick Mattos
2010-03-20 15:13 ` Junio C Hamano
2010-03-20 19:06 ` Erick Mattos
2010-03-20 20:30 ` Junio C Hamano
2010-03-20 20:36 ` Erick Mattos
2010-03-20 20:54 ` Junio C Hamano
2010-03-20 21:37 ` Erick Mattos
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).