Hi,
2010/3/20 Junio C Hamano <gitster@xxxxxxxxx>:
> Erick Mattos <erick.mattos@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html