Re: [PATCH] git checkout: create unparented branch by --orphan
- From
Erick Mattos <erick.mattos@gmail.com>
- Date
- Mar 20, 2010, 19:06 UTC
- Message-ID
- <55bacdd31003201206w6215c6a4qec09797fbe060725@mail.gmail.com>
- In-Reply-To
- <7vvdcrowlc.fsf@alter.siamese.dyndns.org>
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