[PATCH/RFC] branch: add tests for new copy branch feature

[PATCH/RFC] branch: add tests for new copy branch feature

[Sahil Dua]

New feature - copying a branch along with its config section.

Aim is to have an option -c for copying a branch just like -m option for
renaming a branch.

This commit adds a few basic tests for getting any suggestions/feedback
about expected behavior for this new feature.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fe62e7c775da..2c95ed6ebf3c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
 	test_must_fail git config branch.s/s/dummy
 '
 
+test_expect_success 'git branch -c dumps usage' '
+	test_expect_code 128 git branch -c 2>err &&
+	test_i18ngrep "branch name required" err
+'
+
+git config branch.d.dummy Hello
+
+test_expect_success 'git branch -c d e should work' '
+	git branch -l d &&
+	git reflog exists refs/heads/d &&
+	git branch -c d e &&
+	git reflog exists refs/heads/d &&
+	git reflog exists refs/heads/e
+'
+
+test_expect_success 'config information was copied, too' '
+	test $(git config branch.e.dummy) = Hello &&
+	test $(git config branch.d.dummy) = Hello
+'
+
+git config branch.f/f.dummy Hello
+
+test_expect_success 'git branch -c f/f g/g should work' '
+	git branch -l f/f &&
+	git reflog exists refs/heads/f/f &&
+	git branch -c f/f g/g &&
+	git reflog exists refs/heads/f/f &&
+	git reflog exists refs/heads/g/g
+'
+
+test_expect_success 'config information was copied, too' '
+	test $(git config branch.f/f.dummy) = Hello &&
+	test $(git config branch.g/g.dummy) = Hello
+'
+
+test_expect_success 'git branch -c m2 m2 should work' '
+	git branch -l m2 &&
+	git reflog exists refs/heads/m2 &&
+	git branch -c m2 m2 &&
+	git reflog exists refs/heads/m2
+'
+
+test_expect_success 'git branch -c a a/a should fail' '
+	git branch -l a &&
+	git reflog exists refs/heads/a &&
+	test_must_fail git branch -c a a/a
+'
+
+test_expect_success 'git branch -c b/b b should fail' '
+	git branch -l b/b &&
+	test_must_fail git branch -c b/b b
+'
+
 test_expect_success 'deleting a symref' '
 	git branch target &&
 	git symbolic-ref refs/heads/symref refs/heads/target &&

--
https://github.com/git/git/pull/363

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Ævar Arnfjörð Bjarmason]

On Mon, May 29, 2017 at 12:56 AM, Sahil Dua <sahildua2305@gmail.com> wrote:
> New feature - copying a branch along with its config section.
>
> Aim is to have an option -c for copying a branch just like -m option for
> renaming a branch.
>
> This commit adds a few basic tests for getting any suggestions/feedback
> about expected behavior for this new feature.
>
> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
> ---
>  t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index fe62e7c775da..2c95ed6ebf3c 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
>         test_must_fail git config branch.s/s/dummy
>  '
>
> +test_expect_success 'git branch -c dumps usage' '
> +       test_expect_code 128 git branch -c 2>err &&
> +       test_i18ngrep "branch name required" err
> +'
> +
> +git config branch.d.dummy Hello
> +
> +test_expect_success 'git branch -c d e should work' '
> +       git branch -l d &&
> +       git reflog exists refs/heads/d &&
> +       git branch -c d e &&
> +       git reflog exists refs/heads/d &&
> +       git reflog exists refs/heads/e
> +'
> +
> +test_expect_success 'config information was copied, too' '
> +       test $(git config branch.e.dummy) = Hello &&
> +       test $(git config branch.d.dummy) = Hello
> +'
> +
> +git config branch.f/f.dummy Hello
> +
> +test_expect_success 'git branch -c f/f g/g should work' '
> +       git branch -l f/f &&
> +       git reflog exists refs/heads/f/f &&
> +       git branch -c f/f g/g &&
> +       git reflog exists refs/heads/f/f &&
> +       git reflog exists refs/heads/g/g
> +'
> +
> +test_expect_success 'config information was copied, too' '
> +       test $(git config branch.f/f.dummy) = Hello &&
> +       test $(git config branch.g/g.dummy) = Hello
> +'
> +
> +test_expect_success 'git branch -c m2 m2 should work' '
> +       git branch -l m2 &&
> +       git reflog exists refs/heads/m2 &&
> +       git branch -c m2 m2 &&
> +       git reflog exists refs/heads/m2
> +'
> +
> +test_expect_success 'git branch -c a a/a should fail' '
> +       git branch -l a &&
> +       git reflog exists refs/heads/a &&
> +       test_must_fail git branch -c a a/a
> +'
> +
> +test_expect_success 'git branch -c b/b b should fail' '
> +       git branch -l b/b &&
> +       test_must_fail git branch -c b/b b
> +'
> +
>  test_expect_success 'deleting a symref' '
>         git branch target &&
>         git symbolic-ref refs/heads/symref refs/heads/target &&
>

This looks good to me. Comments, in no particular order:

* There should be a test for `git branch -c <newbranch>`, i.e. that
should implicitly copy from HEAD just like `git branch -m <newbranch>`
does. However, when looking at this I can see there's actually no test
for one-argument `git branch -m`, i.e. if you apply this:

--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -699,8 +699,6 @@ int cmd_branch(int argc, const char **argv, const
char *prefix)
        } else if (rename) {
                if (!argc)
                        die(_("branch name required"));
-               else if (argc == 1)
-                       rename_branch(head, argv[0], rename > 1);
                else if (argc == 2)
                        rename_branch(argv[0], argv[1], rename > 1);
                else

The only test that fails is a `git branch -M master`, i.e.
one-argument -M is tested for, but not -m, same codepath though, but
while you're at it a patch/series like this could start by adding a
one-arg -m test, then follow-up by copying that for the new `-c`.

* We should have a -C to force -c just like -M forces -m, i.e. a test
where one-arg `git branch -C alreadyexists` clobbers alreadyexists,
and `git branch -C source alreadyexists` does the same for two-arg.

* I know this is just something you're copying, but this `git branch
-l <name>` use gets me every time "wait how does listing it help isn't
that always succeeding ... damnit it's --create-reflog not --list, got
me again" :)

Just noting it in case it confuses other reviewers who are skimming
this. Might be worth it to just use --create-reflog for new tests
instead of the non-obvious -l whatever the existing tests in the file
do, or maybe I'm the only one confused by this :)

* When you use `git branch -m` it adds a note to the reflog, your
patch should have a test like the existing "git branch -M baz bam
should add entries to .git/logs/HEAD" test in this file except
"Branch: copied ..." instead of "Branch: renamed...".

* Should there be tests for `git branch -c master master` like we have
for `git branch -m master master` (see 3f59481e33 ("branch: allow a
no-op "branch -M <current-branch> HEAD"", 2011-11-25)). Allowing this
for -m smells like a bend-over-backwards workaround for some script
Jonathan had, should we be doing this for -c too? I don't know.

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Junio C Hamano]

Sahil Dua <sahildua2305@gmail.com> writes:

> New feature - copying a branch along with its config section.

That's an unusual non-sentence (without a verb) in our commit message.

> Aim is to have an option -c for copying a branch just like -m option for
> renaming a branch.

What should it copy literally, and what should it copy with
adjustment (and adjusting what), and what should it refrain from
copying?  

For example, when you create a branch topic-2 by copying from a
branch topic-1, do you copy the reflog in such a way that topic-2's
reflog contains all the entries of topic-1 before the copy happens,
capped with a new (and not shared with topic-1) entry that says
"Copied from topic-1"?  When topic-1 is set to pull from a custom
upstream 'upstream' (i.e. not "origin")'s 'trunk' branch, by setting
'branch.topic-2.remote' to "upstream", i.e. the same as the value of
'branch.topic-1.remote' and 'branch.topic-2.merge' to "trunk",
i.e. the same as 'branch.topic-1.merge'?  Should a "git push" while
topic-2 is checked out push to the same remote as a "git push" would
push to when topic-1 is checked out?  Should they update the same
branch at the remote?  Why and/or why not? [*1*]

> This commit adds a few basic tests for getting any suggestions/feedback
> about expected behavior for this new feature.

Writing tests is a good way to prepare for an implementation, which
must be done according to the design, but that happens after the
design is polished and judged to be a good one.  While soliciting
comments on the design from others, tests are a bit too low level to
express what you are aiming at.  It is a bit unhelpful to those who
may want to help to figure out answers to questions like the ones in
the previos paragraph (the one that begins with "For example,...")
by telling them to "read my intention from these tests", and when
you want as wide an input as possible, that is counterproductive for
your objective ;-).

> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
> ---
>  t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index fe62e7c775da..2c95ed6ebf3c 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
>  	test_must_fail git config branch.s/s/dummy
>  '
>  
> +test_expect_success 'git branch -c dumps usage' '
> +	test_expect_code 128 git branch -c 2>err &&
> +	test_i18ngrep "branch name required" err
> +'

OK.  Do we want to support a single-name format (i.e. copy from the
current)?

> +git config branch.d.dummy Hello

Write your tests to do as little outside test_expct_success block as
possible, and do a set-up close to where it matters.

> +test_expect_success 'git branch -c d e should work' '
> +	git branch -l d &&
> +	git reflog exists refs/heads/d &&
> +	git branch -c d e &&
> +	git reflog exists refs/heads/d &&
> +	git reflog exists refs/heads/e
> +'

A reasonable design of "copy" is for d's log to be left intact, and
e's log to begin with a single entry "created by copying from d".
Another possible design is to copy the log (making them identical),
and then add one entry to e's log "created by copying from d".

The above test would succeed with either implementation and does not
answer "should reflog be copied?" (see above about why "here are the
tests that shows my thinking--read them and comment" is a bad
approach).

"move" that makes the source of the movement disappear after the
operation has a stronger justification of moving the log under the
new name (the only alternative is to lose the history of the source
forever), it is debatable if "copy" wants to retain a copy of the
history of an otherwise unrelated branch, which has its own history
retained after the operation.

> +
> +test_expect_success 'config information was copied, too' '
> +	test $(git config branch.e.dummy) = Hello &&
> +	test $(git config branch.d.dummy) = Hello
> +'

The expectation is that a configuration like 'dummy' that does not
have any meaning to Git itself will all blindly be copied, which is
similar to what "move" does.

> +git config branch.f/f.dummy Hello
> +
> +test_expect_success 'git branch -c f/f g/g should work' '
> +	git branch -l f/f &&
> +	git reflog exists refs/heads/f/f &&
> +	git branch -c f/f g/g &&
> +	git reflog exists refs/heads/f/f &&
> +	git reflog exists refs/heads/g/g
> +'

Hmph.  What's the reason to expect this not to work?

> +test_expect_success 'config information was copied, too' '
> +	test $(git config branch.f/f.dummy) = Hello &&
> +	test $(git config branch.g/g.dummy) = Hello
> +'

Isn't this part of the "should work" test above?  The definition of
working is not just reflog is created for the new branch without the
old branch losing its reflog, right?

> +test_expect_success 'git branch -c m2 m2 should work' '
> +	git branch -l m2 &&
> +	git reflog exists refs/heads/m2 &&
> +	git branch -c m2 m2 &&
> +	git reflog exists refs/heads/m2
> +'

Is it OK for the configuration be lost silently?

Thanks.


[Footnote]

*1* One way to avoid having to design an elaborate "definition of
    copying" from scratch is to piggy-back on whatever "move" does.
    Your definition of "copy" could be "copying from A to create B,
    followed by deleting A, should leave the identical result as
    moving A to create B".  And ask people if they agree with that
    definition as "the first principle".

    All the questions in the paragraph that begins with "For
    example..." and similar ones can be answered by followingthe
    first principle.

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Sahil Dua]

Thanks, Junio for raising all these important questions.

Indeed, showing tests in order to explain my thinking about the
feature was a bad idea. I realise that I should have explained the
feature first instead of getting the tests reviewed without any
elaboration of the intentions. I will explain it now.

My definition of "copy" for this feature is "copying from A to create
B, keeping A intact". That means "copy branch A to B" should do
whatever "move branch A to B" does except it shouldn't delete A and
should keep A unchanged.

1. When a branch topic-2 is created by copying from branch topic-1,
topic-2 branch reflog should now contain the all the entries of
topic-1 branch (before copying) followed by "Copied from topic-1".
[This is debatable though, I want inputs/suggestions about this.]

2. Copying a branch should also copy the git config section for that
branch. This means if topic-2 branch is created from topic-1,
"branch.topic-2.remote" should now be same as "branch.topic-1.remote",
if set.

3. "git push" to copied branch for example - topic-2 should push a new
branch with the same name in the remote repo. That means if topic-1
was previously pushed and a new branch topic-2 is copied from topic-1,
"git push" on topic-2 branch won't push to the same branch as "git
push on topic-1 branch would.

4. "git branch -c new-branch" should copy the currently checked out
branch and create a new branch with name "new-branch".

On Mon, May 29, 2017 at 4:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Sahil Dua <sahildua2305@gmail.com> writes:
>
> > New feature - copying a branch along with its config section.
>
> That's an unusual non-sentence (without a verb) in our commit message.

Indeed terrible. I will remove it.

>
>
> > Aim is to have an option -c for copying a branch just like -m option for
> > renaming a branch.
>
> What should it copy literally, and what should it copy with
> adjustment (and adjusting what), and what should it refrain from
> copying?
>
> For example, when you create a branch topic-2 by copying from a
> branch topic-1, do you copy the reflog in such a way that topic-2's
> reflog contains all the entries of topic-1 before the copy happens,
> capped with a new (and not shared with topic-1) entry that says
> "Copied from topic-1"?  When topic-1 is set to pull from a custom
> upstream 'upstream' (i.e. not "origin")'s 'trunk' branch, by setting
> 'branch.topic-2.remote' to "upstream", i.e. the same as the value of
> 'branch.topic-1.remote' and 'branch.topic-2.merge' to "trunk",
> i.e. the same as 'branch.topic-1.merge'?  Should a "git push" while
> topic-2 is checked out push to the same remote as a "git push" would
> push to when topic-1 is checked out?  Should they update the same
> branch at the remote?  Why and/or why not? [*1*]
>
> > This commit adds a few basic tests for getting any suggestions/feedback
> > about expected behavior for this new feature.
>
> Writing tests is a good way to prepare for an implementation, which
> must be done according to the design, but that happens after the
> design is polished and judged to be a good one.  While soliciting
> comments on the design from others, tests are a bit too low level to
> express what you are aiming at.  It is a bit unhelpful to those who
> may want to help to figure out answers to questions like the ones in
> the previos paragraph (the one that begins with "For example,...")
> by telling them to "read my intention from these tests", and when
> you want as wide an input as possible, that is counterproductive for
> your objective ;-).
>
> > Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
> > ---
> >  t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >
> > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> > index fe62e7c775da..2c95ed6ebf3c 100755
> > --- a/t/t3200-branch.sh
> > +++ b/t/t3200-branch.sh
> > @@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
> >       test_must_fail git config branch.s/s/dummy
> >  '
> >
> > +test_expect_success 'git branch -c dumps usage' '
> > +     test_expect_code 128 git branch -c 2>err &&
> > +     test_i18ngrep "branch name required" err
> > +'
>
> OK.  Do we want to support a single-name format (i.e. copy from the
> current)?
>
>
>
> > +git config branch.d.dummy Hello
>
> Write your tests to do as little outside test_expct_success block as
> possible, and do a set-up close to where it matters.
>

Regarding putting code outside test_expect_success block, I wrote this
to keep tests consistent within the same file as I saw other test
doing it this way. How can I improve this?

Here, I needed to set branch config before I copy so that I can
confirm that the new "copied" branch has the same config as well. How
can I improve this and take it as close to where-it-matters as
possible?

>
> > +test_expect_success 'git branch -c d e should work' '
> > +     git branch -l d &&
> > +     git reflog exists refs/heads/d &&
> > +     git branch -c d e &&
> > +     git reflog exists refs/heads/d &&
> > +     git reflog exists refs/heads/e
> > +'
>
> A reasonable design of "copy" is for d's log to be left intact, and
> e's log to begin with a single entry "created by copying from d".
> Another possible design is to copy the log (making them identical),
> and then add one entry to e's log "created by copying from d".
>

Yes, this is debatable as I mentioned above. My personal choice will
be to use the second approach mentioned by you here.

>
> The above test would succeed with either implementation and does not
> answer "should reflog be copied?" (see above about why "here are the
> tests that shows my thinking--read them and comment" is a bad
> approach).
>
> "move" that makes the source of the movement disappear after the
> operation has a stronger justification of moving the log under the
> new name (the only alternative is to lose the history of the source
> forever), it is debatable if "copy" wants to retain a copy of the
> history of an otherwise unrelated branch, which has its own history
> retained after the operation.
>

For example, if branch topic-2 is copied from branch topic-1, "copy"
option should make sure the history of branch topic-1 is retained. I
will need suggestions about this too, in case I'm missing some
consequences/corner-cases of this.

>
> > +
> > +test_expect_success 'config information was copied, too' '
> > +     test $(git config branch.e.dummy) = Hello &&
> > +     test $(git config branch.d.dummy) = Hello
> > +'
>
> The expectation is that a configuration like 'dummy' that does not
> have any meaning to Git itself will all blindly be copied, which is
> similar to what "move" does.

Yes, any configuration for topic-1 branch should be copied to topic-2
branch while keeping the configuration for topic-1 branch retained.

>
>
> > +git config branch.f/f.dummy Hello
> > +
> > +test_expect_success 'git branch -c f/f g/g should work' '
> > +     git branch -l f/f &&
> > +     git reflog exists refs/heads/f/f &&
> > +     git branch -c f/f g/g &&
> > +     git reflog exists refs/heads/f/f &&
> > +     git reflog exists refs/heads/g/g
> > +'
>
> Hmph.  What's the reason to expect this not to work?
>
>
>
> > +test_expect_success 'config information was copied, too' '
> > +     test $(git config branch.f/f.dummy) = Hello &&
> > +     test $(git config branch.g/g.dummy) = Hello
> > +'
>
> Isn't this part of the "should work" test above?  The definition of
> working is not just reflog is created for the new branch without the
> old branch losing its reflog, right?
>

Right, I will put them in one test together.

> > +test_expect_success 'git branch -c m2 m2 should work' '
> > +     git branch -l m2 &&
> > +     git reflog exists refs/heads/m2 &&
> > +     git branch -c m2 m2 &&
> > +     git reflog exists refs/heads/m2
> > +'
>
> Is it OK for the configuration be lost silently?

No, I will change this accordingly as well.

>
>
> Thanks.
>
>
> [Footnote]
>
> *1* One way to avoid having to design an elaborate "definition of
>     copying" from scratch is to piggy-back on whatever "move" does.
>     Your definition of "copy" could be "copying from A to create B,
>     followed by deleting A, should leave the identical result as
>     moving A to create B".  And ask people if they agree with that
>     definition as "the first principle".
>
>     All the questions in the paragraph that begins with "For
>     example..." and similar ones can be answered by followingthe
>     first principle.
>
>

Thanks.

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Sahil Dua]

On Mon, May 29, 2017 at 1:30 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, May 29, 2017 at 12:56 AM, Sahil Dua <sahildua2305@gmail.com> wrote:
>> New feature - copying a branch along with its config section.
>>
>> Aim is to have an option -c for copying a branch just like -m option for
>> renaming a branch.
>>
>> This commit adds a few basic tests for getting any suggestions/feedback
>> about expected behavior for this new feature.
>>
>> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
>> ---
>>  t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 53 insertions(+)
>>
>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
>> index fe62e7c775da..2c95ed6ebf3c 100755
>> --- a/t/t3200-branch.sh
>> +++ b/t/t3200-branch.sh
>> @@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
>>         test_must_fail git config branch.s/s/dummy
>>  '
>>
>> +test_expect_success 'git branch -c dumps usage' '
>> +       test_expect_code 128 git branch -c 2>err &&
>> +       test_i18ngrep "branch name required" err
>> +'
>> +
>> +git config branch.d.dummy Hello
>> +
>> +test_expect_success 'git branch -c d e should work' '
>> +       git branch -l d &&
>> +       git reflog exists refs/heads/d &&
>> +       git branch -c d e &&
>> +       git reflog exists refs/heads/d &&
>> +       git reflog exists refs/heads/e
>> +'
>> +
>> +test_expect_success 'config information was copied, too' '
>> +       test $(git config branch.e.dummy) = Hello &&
>> +       test $(git config branch.d.dummy) = Hello
>> +'
>> +
>> +git config branch.f/f.dummy Hello
>> +
>> +test_expect_success 'git branch -c f/f g/g should work' '
>> +       git branch -l f/f &&
>> +       git reflog exists refs/heads/f/f &&
>> +       git branch -c f/f g/g &&
>> +       git reflog exists refs/heads/f/f &&
>> +       git reflog exists refs/heads/g/g
>> +'
>> +
>> +test_expect_success 'config information was copied, too' '
>> +       test $(git config branch.f/f.dummy) = Hello &&
>> +       test $(git config branch.g/g.dummy) = Hello
>> +'
>> +
>> +test_expect_success 'git branch -c m2 m2 should work' '
>> +       git branch -l m2 &&
>> +       git reflog exists refs/heads/m2 &&
>> +       git branch -c m2 m2 &&
>> +       git reflog exists refs/heads/m2
>> +'
>> +
>> +test_expect_success 'git branch -c a a/a should fail' '
>> +       git branch -l a &&
>> +       git reflog exists refs/heads/a &&
>> +       test_must_fail git branch -c a a/a
>> +'
>> +
>> +test_expect_success 'git branch -c b/b b should fail' '
>> +       git branch -l b/b &&
>> +       test_must_fail git branch -c b/b b
>> +'
>> +
>>  test_expect_success 'deleting a symref' '
>>         git branch target &&
>>         git symbolic-ref refs/heads/symref refs/heads/target &&
>>
>
> This looks good to me. Comments, in no particular order:
>
> * There should be a test for `git branch -c <newbranch>`, i.e. that
> should implicitly copy from HEAD just like `git branch -m <newbranch>`
> does. However, when looking at this I can see there's actually no test
> for one-argument `git branch -m`, i.e. if you apply this:
>
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -699,8 +699,6 @@ int cmd_branch(int argc, const char **argv, const
> char *prefix)
>         } else if (rename) {
>                 if (!argc)
>                         die(_("branch name required"));
> -               else if (argc == 1)
> -                       rename_branch(head, argv[0], rename > 1);
>                 else if (argc == 2)
>                         rename_branch(argv[0], argv[1], rename > 1);
>                 else
>
> The only test that fails is a `git branch -M master`, i.e.
> one-argument -M is tested for, but not -m, same codepath though, but
> while you're at it a patch/series like this could start by adding a
> one-arg -m test, then follow-up by copying that for the new `-c`.
>

Thanks for the suggestion. Yes, I will add one-arg test for -c. Is it
ok to send a different patch for adding a one-arg test for existing -m
option?

> * We should have a -C to force -c just like -M forces -m, i.e. a test
> where one-arg `git branch -C alreadyexists` clobbers alreadyexists,
> and `git branch -C source alreadyexists` does the same for two-arg.
>
Yes, I missed this. I will add -C option too.

> * I know this is just something you're copying, but this `git branch
> -l <name>` use gets me every time "wait how does listing it help isn't
> that always succeeding ... damnit it's --create-reflog not --list, got
> me again" :)
>

Yes, it was confusing to me too in the beginning. I will use --create-reflog.

> Just noting it in case it confuses other reviewers who are skimming
> this. Might be worth it to just use --create-reflog for new tests
> instead of the non-obvious -l whatever the existing tests in the file
> do, or maybe I'm the only one confused by this :)
>
> * When you use `git branch -m` it adds a note to the reflog, your
> patch should have a test like the existing "git branch -M baz bam
> should add entries to .git/logs/HEAD" test in this file except
> "Branch: copied ..." instead of "Branch: renamed...".
>

Nice, I will add it. Thanks.

> * Should there be tests for `git branch -c master master` like we have
> for `git branch -m master master` (see 3f59481e33 ("branch: allow a
> no-op "branch -M <current-branch> HEAD"", 2011-11-25)). Allowing this
> for -m smells like a bend-over-backwards workaround for some script
> Jonathan had, should we be doing this for -c too? I don't know.

Not sure I understand this. Can you please elaborate?
Thanks.

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Ævar Arnfjörð Bjarmason]

On Mon, May 29, 2017 at 10:41 PM, Sahil Dua <sahildua2305@gmail.com> wrote:
> On Mon, May 29, 2017 at 1:30 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> On Mon, May 29, 2017 at 12:56 AM, Sahil Dua <sahildua2305@gmail.com> wrote:
>>> New feature - copying a branch along with its config section.
>>>
>>> Aim is to have an option -c for copying a branch just like -m option for
>>> renaming a branch.
>>>
>>> This commit adds a few basic tests for getting any suggestions/feedback
>>> about expected behavior for this new feature.
>>>
>>> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
>>> ---
>>>  t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 53 insertions(+)
>>>
>>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
>>> index fe62e7c775da..2c95ed6ebf3c 100755
>>> --- a/t/t3200-branch.sh
>>> +++ b/t/t3200-branch.sh
>>> @@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
>>>         test_must_fail git config branch.s/s/dummy
>>>  '
>>>
>>> +test_expect_success 'git branch -c dumps usage' '
>>> +       test_expect_code 128 git branch -c 2>err &&
>>> +       test_i18ngrep "branch name required" err
>>> +'
>>> +
>>> +git config branch.d.dummy Hello
>>> +
>>> +test_expect_success 'git branch -c d e should work' '
>>> +       git branch -l d &&
>>> +       git reflog exists refs/heads/d &&
>>> +       git branch -c d e &&
>>> +       git reflog exists refs/heads/d &&
>>> +       git reflog exists refs/heads/e
>>> +'
>>> +
>>> +test_expect_success 'config information was copied, too' '
>>> +       test $(git config branch.e.dummy) = Hello &&
>>> +       test $(git config branch.d.dummy) = Hello
>>> +'
>>> +
>>> +git config branch.f/f.dummy Hello
>>> +
>>> +test_expect_success 'git branch -c f/f g/g should work' '
>>> +       git branch -l f/f &&
>>> +       git reflog exists refs/heads/f/f &&
>>> +       git branch -c f/f g/g &&
>>> +       git reflog exists refs/heads/f/f &&
>>> +       git reflog exists refs/heads/g/g
>>> +'
>>> +
>>> +test_expect_success 'config information was copied, too' '
>>> +       test $(git config branch.f/f.dummy) = Hello &&
>>> +       test $(git config branch.g/g.dummy) = Hello
>>> +'
>>> +
>>> +test_expect_success 'git branch -c m2 m2 should work' '
>>> +       git branch -l m2 &&
>>> +       git reflog exists refs/heads/m2 &&
>>> +       git branch -c m2 m2 &&
>>> +       git reflog exists refs/heads/m2
>>> +'
>>> +
>>> +test_expect_success 'git branch -c a a/a should fail' '
>>> +       git branch -l a &&
>>> +       git reflog exists refs/heads/a &&
>>> +       test_must_fail git branch -c a a/a
>>> +'
>>> +
>>> +test_expect_success 'git branch -c b/b b should fail' '
>>> +       git branch -l b/b &&
>>> +       test_must_fail git branch -c b/b b
>>> +'
>>> +
>>>  test_expect_success 'deleting a symref' '
>>>         git branch target &&
>>>         git symbolic-ref refs/heads/symref refs/heads/target &&
>>>
>>
>> This looks good to me. Comments, in no particular order:
>>
>> * There should be a test for `git branch -c <newbranch>`, i.e. that
>> should implicitly copy from HEAD just like `git branch -m <newbranch>`
>> does. However, when looking at this I can see there's actually no test
>> for one-argument `git branch -m`, i.e. if you apply this:
>>
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -699,8 +699,6 @@ int cmd_branch(int argc, const char **argv, const
>> char *prefix)
>>         } else if (rename) {
>>                 if (!argc)
>>                         die(_("branch name required"));
>> -               else if (argc == 1)
>> -                       rename_branch(head, argv[0], rename > 1);
>>                 else if (argc == 2)
>>                         rename_branch(argv[0], argv[1], rename > 1);
>>                 else
>>
>> The only test that fails is a `git branch -M master`, i.e.
>> one-argument -M is tested for, but not -m, same codepath though, but
>> while you're at it a patch/series like this could start by adding a
>> one-arg -m test, then follow-up by copying that for the new `-c`.
>>
>
> Thanks for the suggestion. Yes, I will add one-arg test for -c. Is it
> ok to send a different patch for adding a one-arg test for existing -m
> option?

Yeah, it makes sense to just make the first patch in the series be
some cleanup / improvement of the existing tests, which the subsequent
tests for -c then make use of / copy. It could even be sent on its
own, but probably makes sense to just bundle them up. Up to you
though, in this case you won't need patch A for patch B to work, so
the that's one argument against bundling them up. Personally I'd do it
if I was hacking this just because it's more convenient to keep track
of fewer things.

>> * We should have a -C to force -c just like -M forces -m, i.e. a test
>> where one-arg `git branch -C alreadyexists` clobbers alreadyexists,
>> and `git branch -C source alreadyexists` does the same for two-arg.
>>
> Yes, I missed this. I will add -C option too.
>
>> * I know this is just something you're copying, but this `git branch
>> -l <name>` use gets me every time "wait how does listing it help isn't
>> that always succeeding ... damnit it's --create-reflog not --list, got
>> me again" :)
>>
>
> Yes, it was confusing to me too in the beginning. I will use --create-reflog.
>
>> Just noting it in case it confuses other reviewers who are skimming
>> this. Might be worth it to just use --create-reflog for new tests
>> instead of the non-obvious -l whatever the existing tests in the file
>> do, or maybe I'm the only one confused by this :)
>>
>> * When you use `git branch -m` it adds a note to the reflog, your
>> patch should have a test like the existing "git branch -M baz bam
>> should add entries to .git/logs/HEAD" test in this file except
>> "Branch: copied ..." instead of "Branch: renamed...".
>>
>
> Nice, I will add it. Thanks.
>
>> * Should there be tests for `git branch -c master master` like we have
>> for `git branch -m master master` (see 3f59481e33 ("branch: allow a
>> no-op "branch -M <current-branch> HEAD"", 2011-11-25)). Allowing this
>> for -m smells like a bend-over-backwards workaround for some script
>> Jonathan had, should we be doing this for -c too? I don't know.
>
> Not sure I understand this. Can you please elaborate?
> Thanks.

So the reason we have this for -m is:

    commit 3f59481e33
    Author: Jonathan Nieder <jrnieder@gmail.com>
    Date:   Fri Nov 25 20:30:02 2011 -0600

    branch: allow a no-op "branch -M <current-branch> HEAD"

    Overwriting the current branch with a different commit is forbidden, as it
    will make the status recorded in the index and the working tree out of
    sync with respect to the HEAD. There however is no reason to forbid it if
    the current branch is renamed to itself, which admittedly is something
    only an insane user would do, but is handy for scripts.

My understanding of that last part is that Jonathan/someone (see
reported-by in that patch) had some script which was renaming
branches, and it was easier for whatever reason to just make it no-op
if the rename would have yielded the same result as doing nothing at
all.

Most likely your implementation will consist of just re-using the
logic in rename_branch() (and renaming it to e.g.
copy_or_rename_branch() ...) so you could just re-use the no-op
behavior we use for -m, or if there's some reason not to no-op and
error instead for -c we could just do that, but in any case this case
of `git branch -c master master` or `git branch -c currentbranch`
should be tested for.

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Sahil Dua]

On Mon, May 29, 2017 at 10:50 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, May 29, 2017 at 10:41 PM, Sahil Dua <sahildua2305@gmail.com> wrote:
>> On Mon, May 29, 2017 at 1:30 AM, Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>> On Mon, May 29, 2017 at 12:56 AM, Sahil Dua <sahildua2305@gmail.com> wrote:
>>>> New feature - copying a branch along with its config section.
>>>>
>>>> Aim is to have an option -c for copying a branch just like -m option for
>>>> renaming a branch.
>>>>
>>>> This commit adds a few basic tests for getting any suggestions/feedback
>>>> about expected behavior for this new feature.
>>>>
>>>> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
>>>> ---
>>>>  t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 53 insertions(+)
>>>>
>>>> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
>>>> index fe62e7c775da..2c95ed6ebf3c 100755
>>>> --- a/t/t3200-branch.sh
>>>> +++ b/t/t3200-branch.sh
>>>> @@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
>>>>         test_must_fail git config branch.s/s/dummy
>>>>  '
>>>>
>>>> +test_expect_success 'git branch -c dumps usage' '
>>>> +       test_expect_code 128 git branch -c 2>err &&
>>>> +       test_i18ngrep "branch name required" err
>>>> +'
>>>> +
>>>> +git config branch.d.dummy Hello
>>>> +
>>>> +test_expect_success 'git branch -c d e should work' '
>>>> +       git branch -l d &&
>>>> +       git reflog exists refs/heads/d &&
>>>> +       git branch -c d e &&
>>>> +       git reflog exists refs/heads/d &&
>>>> +       git reflog exists refs/heads/e
>>>> +'
>>>> +
>>>> +test_expect_success 'config information was copied, too' '
>>>> +       test $(git config branch.e.dummy) = Hello &&
>>>> +       test $(git config branch.d.dummy) = Hello
>>>> +'
>>>> +
>>>> +git config branch.f/f.dummy Hello
>>>> +
>>>> +test_expect_success 'git branch -c f/f g/g should work' '
>>>> +       git branch -l f/f &&
>>>> +       git reflog exists refs/heads/f/f &&
>>>> +       git branch -c f/f g/g &&
>>>> +       git reflog exists refs/heads/f/f &&
>>>> +       git reflog exists refs/heads/g/g
>>>> +'
>>>> +
>>>> +test_expect_success 'config information was copied, too' '
>>>> +       test $(git config branch.f/f.dummy) = Hello &&
>>>> +       test $(git config branch.g/g.dummy) = Hello
>>>> +'
>>>> +
>>>> +test_expect_success 'git branch -c m2 m2 should work' '
>>>> +       git branch -l m2 &&
>>>> +       git reflog exists refs/heads/m2 &&
>>>> +       git branch -c m2 m2 &&
>>>> +       git reflog exists refs/heads/m2
>>>> +'
>>>> +
>>>> +test_expect_success 'git branch -c a a/a should fail' '
>>>> +       git branch -l a &&
>>>> +       git reflog exists refs/heads/a &&
>>>> +       test_must_fail git branch -c a a/a
>>>> +'
>>>> +
>>>> +test_expect_success 'git branch -c b/b b should fail' '
>>>> +       git branch -l b/b &&
>>>> +       test_must_fail git branch -c b/b b
>>>> +'
>>>> +
>>>>  test_expect_success 'deleting a symref' '
>>>>         git branch target &&
>>>>         git symbolic-ref refs/heads/symref refs/heads/target &&
>>>>
>>>
>>> This looks good to me. Comments, in no particular order:
>>>
>>> * There should be a test for `git branch -c <newbranch>`, i.e. that
>>> should implicitly copy from HEAD just like `git branch -m <newbranch>`
>>> does. However, when looking at this I can see there's actually no test
>>> for one-argument `git branch -m`, i.e. if you apply this:
>>>
>>> --- a/builtin/branch.c
>>> +++ b/builtin/branch.c
>>> @@ -699,8 +699,6 @@ int cmd_branch(int argc, const char **argv, const
>>> char *prefix)
>>>         } else if (rename) {
>>>                 if (!argc)
>>>                         die(_("branch name required"));
>>> -               else if (argc == 1)
>>> -                       rename_branch(head, argv[0], rename > 1);
>>>                 else if (argc == 2)
>>>                         rename_branch(argv[0], argv[1], rename > 1);
>>>                 else
>>>
>>> The only test that fails is a `git branch -M master`, i.e.
>>> one-argument -M is tested for, but not -m, same codepath though, but
>>> while you're at it a patch/series like this could start by adding a
>>> one-arg -m test, then follow-up by copying that for the new `-c`.
>>>
>>
>> Thanks for the suggestion. Yes, I will add one-arg test for -c. Is it
>> ok to send a different patch for adding a one-arg test for existing -m
>> option?
>
> Yeah, it makes sense to just make the first patch in the series be
> some cleanup / improvement of the existing tests, which the subsequent
> tests for -c then make use of / copy. It could even be sent on its
> own, but probably makes sense to just bundle them up. Up to you
> though, in this case you won't need patch A for patch B to work, so
> the that's one argument against bundling them up. Personally I'd do it
> if I was hacking this just because it's more convenient to keep track
> of fewer things.
>

Got it. I will submit a patch for the tests for -m option.

>>> * We should have a -C to force -c just like -M forces -m, i.e. a test
>>> where one-arg `git branch -C alreadyexists` clobbers alreadyexists,
>>> and `git branch -C source alreadyexists` does the same for two-arg.
>>>
>> Yes, I missed this. I will add -C option too.
>>
>>> * I know this is just something you're copying, but this `git branch
>>> -l <name>` use gets me every time "wait how does listing it help isn't
>>> that always succeeding ... damnit it's --create-reflog not --list, got
>>> me again" :)
>>>
>>
>> Yes, it was confusing to me too in the beginning. I will use --create-reflog.
>>
>>> Just noting it in case it confuses other reviewers who are skimming
>>> this. Might be worth it to just use --create-reflog for new tests
>>> instead of the non-obvious -l whatever the existing tests in the file
>>> do, or maybe I'm the only one confused by this :)
>>>
>>> * When you use `git branch -m` it adds a note to the reflog, your
>>> patch should have a test like the existing "git branch -M baz bam
>>> should add entries to .git/logs/HEAD" test in this file except
>>> "Branch: copied ..." instead of "Branch: renamed...".
>>>
>>
>> Nice, I will add it. Thanks.
>>
>>> * Should there be tests for `git branch -c master master` like we have
>>> for `git branch -m master master` (see 3f59481e33 ("branch: allow a
>>> no-op "branch -M <current-branch> HEAD"", 2011-11-25)). Allowing this
>>> for -m smells like a bend-over-backwards workaround for some script
>>> Jonathan had, should we be doing this for -c too? I don't know.
>>
>> Not sure I understand this. Can you please elaborate?
>> Thanks.
>
> So the reason we have this for -m is:
>
>     commit 3f59481e33
>     Author: Jonathan Nieder <jrnieder@gmail.com>
>     Date:   Fri Nov 25 20:30:02 2011 -0600
>
>     branch: allow a no-op "branch -M <current-branch> HEAD"
>
>     Overwriting the current branch with a different commit is forbidden, as it
>     will make the status recorded in the index and the working tree out of
>     sync with respect to the HEAD. There however is no reason to forbid it if
>     the current branch is renamed to itself, which admittedly is something
>     only an insane user would do, but is handy for scripts.
>
> My understanding of that last part is that Jonathan/someone (see
> reported-by in that patch) had some script which was renaming
> branches, and it was easier for whatever reason to just make it no-op
> if the rename would have yielded the same result as doing nothing at
> all.
>
> Most likely your implementation will consist of just re-using the
> logic in rename_branch() (and renaming it to e.g.
> copy_or_rename_branch() ...) so you could just re-use the no-op
> behavior we use for -m, or if there's some reason not to no-op and
> error instead for -c we could just do that, but in any case this case
> of `git branch -c master master` or `git branch -c currentbranch`
> should be tested for.

Understood. Thanks. I will add tests for this too.

[PATCH/RFC v2 4/6] config: modify function signature to include copy argument

[Sahil Dua]

Changed git_config_rename_section to git_config_copy_or_rename_section
which will now accept another argument flag "copy" which will determine
if the function will copy the config section or just rename it.

Again, this includes changes at a lot of unrelated other places wherever
the renamed and updated functions were being used. Default value of
copy=0 is passed at all those places in order to make sure the behavior
of the functions doesn't change for those cases.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 builtin/branch.c | 4 ++--
 builtin/config.c | 4 ++--
 builtin/remote.c | 4 ++--
 cache.h          | 4 ++--
 config.c         | 6 +++---
 submodule.c      | 2 +-
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 16d01a100cbb9..f3cd180e8d4cb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -176,7 +176,7 @@ static void delete_branch_config(const char *branchname)
 {
 	struct strbuf buf = STRBUF_INIT;
 	strbuf_addf(&buf, "branch.%s", branchname);
-	if (git_config_copy_or_rename_section(buf.buf, NULL) < 0)
+	if (git_config_copy_or_rename_section(buf.buf, NULL, 0) < 0)
 		warning(_("Update of config-file failed"));
 	strbuf_release(&buf);
 }
@@ -502,7 +502,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 	strbuf_release(&oldref);
 	strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
 	strbuf_release(&newref);
-	if (git_config_copy_or_rename_section(oldsection.buf, newsection.buf) < 0)
+	if (git_config_copy_or_rename_section(oldsection.buf, newsection.buf, copy) < 0)
 		die(_("Branch is %s, but update of config-file failed"),
 			 (copy ? "copied" : "renamed"));
 	strbuf_release(&oldsection);
diff --git a/builtin/config.c b/builtin/config.c
index c72972d731bd1..4f0b3d1595709 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -694,7 +694,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_write();
 		check_argc(argc, 2, 2);
 		ret = git_config_copy_or_rename_section_in_file(given_config_source.file,
-							argv[0], argv[1]);
+							argv[0], argv[1], 0);
 		if (ret < 0)
 			return ret;
 		if (ret == 0)
@@ -705,7 +705,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_write();
 		check_argc(argc, 1, 1);
 		ret = git_config_copy_or_rename_section_in_file(given_config_source.file,
-							argv[0], NULL);
+							argv[0], NULL, 0);
 		if (ret < 0)
 			return ret;
 		if (ret == 0)
diff --git a/builtin/remote.c b/builtin/remote.c
index ade748044b5ab..2abcdfa441599 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -635,7 +635,7 @@ static int mv(int argc, const char **argv)
 	strbuf_reset(&buf);
 	strbuf_addf(&buf, "remote.%s", rename.old);
 	strbuf_addf(&buf2, "remote.%s", rename.new);
-	if (git_config_copy_or_rename_section(buf.buf, buf2.buf) < 1)
+	if (git_config_copy_or_rename_section(buf.buf, buf2.buf, 0) < 1)
 		return error(_("Could not rename config section '%s' to '%s'"),
 				buf.buf, buf2.buf);
 
@@ -804,7 +804,7 @@ static int rm(int argc, const char **argv)
 
 	if (!result) {
 		strbuf_addf(&buf, "remote.%s", remote->name);
-		if (git_config_copy_or_rename_section(buf.buf, NULL) < 1)
+		if (git_config_copy_or_rename_section(buf.buf, NULL, 0) < 1)
 			return error(_("Could not remove config section '%s'"), buf.buf);
 	}
 
diff --git a/cache.h b/cache.h
index b2b043d3505ba..54a7f272bac87 100644
--- a/cache.h
+++ b/cache.h
@@ -1933,8 +1933,8 @@ extern int git_config_set_multivar_gently(const char *, const char *, const char
 extern void git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, int);
 extern void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
-extern int git_config_copy_or_rename_section(const char *, const char *);
-extern int git_config_copy_or_rename_section_in_file(const char *, const char *, const char *);
+extern int git_config_copy_or_rename_section(const char *, const char *, int);
+extern int git_config_copy_or_rename_section_in_file(const char *, const char *, const char *, int);
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
diff --git a/config.c b/config.c
index d3d48bfae3b96..155274f03b2b6 100644
--- a/config.c
+++ b/config.c
@@ -2640,7 +2640,7 @@ static int section_name_is_ok(const char *name)
 
 /* if new_name == NULL, the section is removed instead */
 int git_config_copy_or_rename_section_in_file(const char *config_filename,
-				      const char *old_name, const char *new_name)
+				      const char *old_name, const char *new_name, int copy)
 {
 	int ret = 0, remove = 0;
 	char *filename_buf = NULL;
@@ -2743,9 +2743,9 @@ int git_config_copy_or_rename_section_in_file(const char *config_filename,
 	return ret;
 }
 
-int git_config_copy_or_rename_section(const char *old_name, const char *new_name)
+int git_config_copy_or_rename_section(const char *old_name, const char *new_name, int copy)
 {
-	return git_config_copy_or_rename_section_in_file(NULL, old_name, new_name);
+	return git_config_copy_or_rename_section_in_file(NULL, old_name, new_name, copy);
 }
 
 /*
diff --git a/submodule.c b/submodule.c
index d93f366be31c6..347ff4ca668aa 100644
--- a/submodule.c
+++ b/submodule.c
@@ -107,7 +107,7 @@ int remove_path_from_gitmodules(const char *path)
 	}
 	strbuf_addstr(&sect, "submodule.");
 	strbuf_addstr(&sect, submodule->name);
-	if (git_config_copy_or_rename_section_in_file(".gitmodules", sect.buf, NULL) < 0) {
+	if (git_config_copy_or_rename_section_in_file(".gitmodules", sect.buf, NULL, 0) < 0) {
 		/* Maybe the user already did that, don't error out here */
 		warning(_("Could not remove .gitmodules entry for %s"), path);
 		strbuf_release(&sect);

--
https://github.com/git/git/pull/363

[PATCH/RFC v2 6/6] branch: don't copy or rename config when same branch name

[Sahil Dua]

It doesn't make sense to trigger config section copy or rename method if
both the branch names are same.

For example - git branch -C a a
In such a case, it shouldn't try to copy or rename the git config
section.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 builtin/branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index f3cd180e8d4cb..df82f196a4bba 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -502,7 +502,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 	strbuf_release(&oldref);
 	strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
 	strbuf_release(&newref);
-	if (git_config_copy_or_rename_section(oldsection.buf, newsection.buf, copy) < 0)
+	if (strcmp(oldname, newname) && git_config_copy_or_rename_section(oldsection.buf, newsection.buf, copy) < 0)
 		die(_("Branch is %s, but update of config-file failed"),
 			 (copy ? "copied" : "renamed"));
 	strbuf_release(&oldsection);

--
https://github.com/git/git/pull/363

[PATCH/RFC v2 3/6] config: abstract out create section from key logic

[Sahil Dua]

Abstracts out the logic for creating string buffer from given key for
example - 'branch.b' and returns '[branch "b"]'.

We want to keep the original config section intact in case of copy
operation. For this we need to fetch the section with updated new branch
name so that we can write that to the config file.

For example - git branch -c foo bar
The mentioned/edited function renames and overwrites this part in the
config - [branch "foo"] to [branch "bar"]. However, in case of copy, we
want to keep the original [branch "foo"] intact and get [branch "bar"]
from "branch.bar" key. 'store_create_section' function will return
[branch "bar"] when "branch.bar" is passed.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 config.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 78cf1ffac043e..d3d48bfae3b96 100644
--- a/config.c
+++ b/config.c
@@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
 	return 4;
 }
 
-static int store_write_section(int fd, const char *key)
+struct strbuf store_create_section(const char *key)
 {
 	const char *dot;
-	int i, success;
+	int i;
 	struct strbuf sb = STRBUF_INIT;
 
 	dot = memchr(key, '.', store.baselen);
@@ -2188,6 +2188,16 @@ static int store_write_section(int fd, const char *key)
 		strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
 	}
 
+	return sb;
+}
+
+static int store_write_section(int fd, const char *key)
+{
+	int success;
+
+	/* Create a section with the given key */
+	struct strbuf sb = store_create_section(key);
+
 	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
 	strbuf_release(&sb);
 

--
https://github.com/git/git/pull/363

[PATCH/RFC v2 1/6] branch: add tests for new copy branch feature

[Sahil Dua]

Adds a few basic tests for getting any suggestions/feedback
about expected behavior for this new feature. Aim is to have an option -c
for copying a branch just like -m option for renaming a branch.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 t/t3200-branch.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fe62e7c775da6..2c95ed6ebf3c5 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -341,6 +341,59 @@ test_expect_success 'config information was renamed, too' '
 	test_must_fail git config branch.s/s/dummy
 '
 
+test_expect_success 'git branch -c dumps usage' '
+	test_expect_code 128 git branch -c 2>err &&
+	test_i18ngrep "branch name required" err
+'
+
+git config branch.d.dummy Hello
+
+test_expect_success 'git branch -c d e should work' '
+	git branch -l d &&
+	git reflog exists refs/heads/d &&
+	git branch -c d e &&
+	git reflog exists refs/heads/d &&
+	git reflog exists refs/heads/e
+'
+
+test_expect_success 'config information was copied, too' '
+	test $(git config branch.e.dummy) = Hello &&
+	test $(git config branch.d.dummy) = Hello
+'
+
+git config branch.f/f.dummy Hello
+
+test_expect_success 'git branch -c f/f g/g should work' '
+	git branch -l f/f &&
+	git reflog exists refs/heads/f/f &&
+	git branch -c f/f g/g &&
+	git reflog exists refs/heads/f/f &&
+	git reflog exists refs/heads/g/g
+'
+
+test_expect_success 'config information was copied, too' '
+	test $(git config branch.f/f.dummy) = Hello &&
+	test $(git config branch.g/g.dummy) = Hello
+'
+
+test_expect_success 'git branch -c m2 m2 should work' '
+	git branch -l m2 &&
+	git reflog exists refs/heads/m2 &&
+	git branch -c m2 m2 &&
+	git reflog exists refs/heads/m2
+'
+
+test_expect_success 'git branch -c a a/a should fail' '
+	git branch -l a &&
+	git reflog exists refs/heads/a &&
+	test_must_fail git branch -c a a/a
+'
+
+test_expect_success 'git branch -c b/b b should fail' '
+	git branch -l b/b &&
+	test_must_fail git branch -c b/b b
+'
+
 test_expect_success 'deleting a symref' '
 	git branch target &&
 	git symbolic-ref refs/heads/symref refs/heads/target &&

--
https://github.com/git/git/pull/363

[PATCH/RFC v2 2/6] branch: add copy branch option

[Sahil Dua]

Adds copy branch option available using -c or -C (forcefully).

Includes a lot of function renames and their signature changes in order
to introduce a new function parameter - flag 'copy' which determines
whether those functions should do operation copy or move.

Additionally, this changes a lot of other files wherever the renamed
functions were used. By default copy=0 is passed at all those places so
that they keep behaving the way they were, before these changes.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 builtin/branch.c          | 48 +++++++++++++++++++++++++++++++----------------
 builtin/config.c          |  4 ++--
 builtin/remote.c          |  6 +++---
 cache.h                   |  4 ++--
 config.c                  |  6 +++---
 refs.c                    | 10 +++++-----
 refs.h                    |  7 ++++---
 refs/files-backend.c      | 21 ++++++++++++++-------
 refs/refs-internal.h      |  6 +++---
 submodule.c               |  2 +-
 t/helper/test-ref-store.c |  2 +-
 11 files changed, 70 insertions(+), 46 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 83fcda43dceec..16d01a100cbb9 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-l] [-f] <branch-name> [<start-point>]"),
 	N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
 	N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
+	N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
 	N_("git branch [<options>] [-r | -a] [--points-at]"),
 	N_("git branch [<options>] [-r | -a] [--format]"),
 	NULL
@@ -175,7 +176,7 @@ static void delete_branch_config(const char *branchname)
 {
 	struct strbuf buf = STRBUF_INIT;
 	strbuf_addf(&buf, "branch.%s", branchname);
-	if (git_config_rename_section(buf.buf, NULL) < 0)
+	if (git_config_copy_or_rename_section(buf.buf, NULL) < 0)
 		warning(_("Update of config-file failed"));
 	strbuf_release(&buf);
 }
@@ -449,7 +450,7 @@ static void reject_rebase_or_bisect_branch(const char *target)
 	free_worktrees(worktrees);
 }
 
-static void rename_branch(const char *oldname, const char *newname, int force)
+static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)
 {
 	struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
 	struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
@@ -457,7 +458,8 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	int clobber_head_ok;
 
 	if (!oldname)
-		die(_("cannot rename the current branch while not on any."));
+		die(_("cannot %s the current branch while not on any."),
+			 (copy ? "copy" : "rename"));
 
 	if (strbuf_check_branch_ref(&oldref, oldname)) {
 		/*
@@ -480,17 +482,19 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 
 	reject_rebase_or_bisect_branch(oldref.buf);
 
-	strbuf_addf(&logmsg, "Branch: renamed %s to %s",
-		 oldref.buf, newref.buf);
+	strbuf_addf(&logmsg, "Branch: %s %s to %s",
+		 (copy ? "copied" : "renamed"), oldref.buf, newref.buf);
 
-	if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
-		die(_("Branch rename failed"));
+	if (copy_or_rename_ref(oldref.buf, newref.buf, logmsg.buf, copy))
+		die(_("Branch %s failed"), (copy ? "copy" : "rename"));
 
 	if (recovery)
-		warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);
+		warning(_("%s a misnamed branch '%s' away"),
+			 (copy ? "copied" : "renamed"), oldref.buf + 11);
 
 	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))
-		die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+		die(_("Branch %s to %s, but HEAD is not updated!"),
+			 (copy ? "copied" : "renamed"), newname);
 
 	strbuf_release(&logmsg);
 
@@ -498,8 +502,9 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	strbuf_release(&oldref);
 	strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
 	strbuf_release(&newref);
-	if (git_config_rename_section(oldsection.buf, newsection.buf) < 0)
-		die(_("Branch is renamed, but update of config-file failed"));
+	if (git_config_copy_or_rename_section(oldsection.buf, newsection.buf) < 0)
+		die(_("Branch is %s, but update of config-file failed"),
+			 (copy ? "copied" : "renamed"));
 	strbuf_release(&oldsection);
 	strbuf_release(&newsection);
 }
@@ -537,7 +542,7 @@ static int edit_branch_description(const char *branch_name)
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-	int delete = 0, rename = 0, force = 0, list = 0;
+	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
@@ -574,6 +579,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
 		OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
 		OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
+		OPT_BIT('c', NULL, &copy, N_("copy a branch and its reflog"), 1),
+		OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
 		OPT_BOOL(0, "list", &list, N_("list branch names")),
 		OPT_BOOL('l', "create-reflog", &reflog, N_("create the branch's reflog")),
 		OPT_BOOL(0, "edit-description", &edit_description,
@@ -617,14 +624,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
+	if (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
 	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
 	    filter.no_commit)
 		list = 1;
 
-	if (!!delete + !!rename + !!new_upstream +
+	if (!!delete + !!rename + !!copy + !!new_upstream +
 	    list + unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
@@ -696,13 +703,22 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (edit_branch_description(branch_name))
 			return 1;
+	} else if (copy) {
+		if (!argc)
+			die(_("branch name required"));
+		else if (argc == 1)
+			copy_or_rename_branch(head, argv[0], 1, copy > 1);
+		else if (argc == 2)
+			copy_or_rename_branch(argv[0], argv[1], 1, copy > 1);
+		else
+			die(_("too many branches for a copy operation"));
 	} else if (rename) {
 		if (!argc)
 			die(_("branch name required"));
 		else if (argc == 1)
-			rename_branch(head, argv[0], rename > 1);
+			copy_or_rename_branch(head, argv[0], 0, rename > 1);
 		else if (argc == 2)
-			rename_branch(argv[0], argv[1], rename > 1);
+			copy_or_rename_branch(argv[0], argv[1], 0, rename > 1);
 		else
 			die(_("too many branches for a rename operation"));
 	} else if (new_upstream) {
diff --git a/builtin/config.c b/builtin/config.c
index 7f6c25d4d95b3..c72972d731bd1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -693,7 +693,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		int ret;
 		check_write();
 		check_argc(argc, 2, 2);
-		ret = git_config_rename_section_in_file(given_config_source.file,
+		ret = git_config_copy_or_rename_section_in_file(given_config_source.file,
 							argv[0], argv[1]);
 		if (ret < 0)
 			return ret;
@@ -704,7 +704,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		int ret;
 		check_write();
 		check_argc(argc, 1, 1);
-		ret = git_config_rename_section_in_file(given_config_source.file,
+		ret = git_config_copy_or_rename_section_in_file(given_config_source.file,
 							argv[0], NULL);
 		if (ret < 0)
 			return ret;
diff --git a/builtin/remote.c b/builtin/remote.c
index addf97ad29343..ade748044b5ab 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -635,7 +635,7 @@ static int mv(int argc, const char **argv)
 	strbuf_reset(&buf);
 	strbuf_addf(&buf, "remote.%s", rename.old);
 	strbuf_addf(&buf2, "remote.%s", rename.new);
-	if (git_config_rename_section(buf.buf, buf2.buf) < 1)
+	if (git_config_copy_or_rename_section(buf.buf, buf2.buf) < 1)
 		return error(_("Could not rename config section '%s' to '%s'"),
 				buf.buf, buf2.buf);
 
@@ -706,7 +706,7 @@ static int mv(int argc, const char **argv)
 		strbuf_reset(&buf2);
 		strbuf_addf(&buf2, "remote: renamed %s to %s",
 				item->string, buf.buf);
-		if (rename_ref(item->string, buf.buf, buf2.buf))
+		if (copy_or_rename_ref(item->string, buf.buf, buf2.buf, 0))
 			die(_("renaming '%s' failed"), item->string);
 	}
 	for (i = 0; i < remote_branches.nr; i++) {
@@ -804,7 +804,7 @@ static int rm(int argc, const char **argv)
 
 	if (!result) {
 		strbuf_addf(&buf, "remote.%s", remote->name);
-		if (git_config_rename_section(buf.buf, NULL) < 1)
+		if (git_config_copy_or_rename_section(buf.buf, NULL) < 1)
 			return error(_("Could not remove config section '%s'"), buf.buf);
 	}
 
diff --git a/cache.h b/cache.h
index ae4c45d379d5b..b2b043d3505ba 100644
--- a/cache.h
+++ b/cache.h
@@ -1933,8 +1933,8 @@ extern int git_config_set_multivar_gently(const char *, const char *, const char
 extern void git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, int);
 extern void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
-extern int git_config_rename_section(const char *, const char *);
-extern int git_config_rename_section_in_file(const char *, const char *, const char *);
+extern int git_config_copy_or_rename_section(const char *, const char *);
+extern int git_config_copy_or_rename_section_in_file(const char *, const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
diff --git a/config.c b/config.c
index 146cb3452adab..78cf1ffac043e 100644
--- a/config.c
+++ b/config.c
@@ -2629,7 +2629,7 @@ static int section_name_is_ok(const char *name)
 }
 
 /* if new_name == NULL, the section is removed instead */
-int git_config_rename_section_in_file(const char *config_filename,
+int git_config_copy_or_rename_section_in_file(const char *config_filename,
 				      const char *old_name, const char *new_name)
 {
 	int ret = 0, remove = 0;
@@ -2733,9 +2733,9 @@ int git_config_rename_section_in_file(const char *config_filename,
 	return ret;
 }
 
-int git_config_rename_section(const char *old_name, const char *new_name)
+int git_config_copy_or_rename_section(const char *old_name, const char *new_name)
 {
-	return git_config_rename_section_in_file(NULL, old_name, new_name);
+	return git_config_copy_or_rename_section_in_file(NULL, old_name, new_name);
 }
 
 /*
diff --git a/refs.c b/refs.c
index 8af9641aa17e6..f8fb2577dfa9c 100644
--- a/refs.c
+++ b/refs.c
@@ -1907,13 +1907,13 @@ int delete_refs(struct string_list *refnames, unsigned int flags)
 	return refs_delete_refs(get_main_ref_store(), refnames, flags);
 }
 
-int refs_rename_ref(struct ref_store *refs, const char *oldref,
-		    const char *newref, const char *logmsg)
+int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg, int copy)
 {
-	return refs->be->rename_ref(refs, oldref, newref, logmsg);
+	return refs->be->copy_or_rename_ref(refs, oldref, newref, logmsg, copy);
 }
 
-int rename_ref(const char *oldref, const char *newref, const char *logmsg)
+int copy_or_rename_ref(const char *oldref, const char *newref, const char *logmsg, int copy)
 {
-	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
+	return refs_copy_or_rename_ref(get_main_ref_store(), oldref, newref, logmsg, copy);
 }
diff --git a/refs.h b/refs.h
index 685a979a0eb70..febdb09541813 100644
--- a/refs.h
+++ b/refs.h
@@ -394,9 +394,10 @@ const char *prettify_refname(const char *refname);
 char *shorten_unambiguous_ref(const char *refname, int strict);
 
 /** rename ref, return 0 on success **/
-int refs_rename_ref(struct ref_store *refs, const char *oldref,
-		    const char *newref, const char *logmsg);
-int rename_ref(const char *oldref, const char *newref, const char *logmsg);
+int refs_copy_or_rename_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg, int copy);
+int copy_or_rename_ref(const char *oldref, const char *newref,
+			const char *logmsg, int copy);
 
 int refs_create_symref(struct ref_store *refs, const char *refname,
 		       const char *target, const char *logmsg);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index cb1f528cbeec4..670cc00d3f3e3 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1703,9 +1703,9 @@ static int commit_ref_update(struct files_ref_store *refs,
 			     const struct object_id *oid, const char *logmsg,
 			     struct strbuf *err);
 
-static int files_rename_ref(struct ref_store *ref_store,
+static int files_copy_or_rename_ref(struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
-			    const char *logmsg)
+			    const char *logmsg, int copy)
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
@@ -1746,14 +1746,21 @@ static int files_rename_ref(struct ref_store *ref_store,
 		goto out;
 	}
 
-	if (log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
+	if (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
 		ret = error("unable to move logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
 			    oldrefname, strerror(errno));
 		goto out;
 	}
 
-	if (refs_delete_ref(&refs->base, logmsg, oldrefname,
-			    orig_oid.hash, REF_NODEREF)) {
+	// TODO: merge this block with the rename one above
+	if (copy && log && copy_file(tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
+		ret = error("unable to copy logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
+			    oldrefname, strerror(errno));
+		goto out;
+	}
+
+	if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,
+			    orig_sha1, REF_NODEREF)) {
 		error("unable to delete old %s", oldrefname);
 		goto rollback;
 	}
@@ -1765,7 +1772,7 @@ static int files_rename_ref(struct ref_store *ref_store,
 	 * the safety anyway; we want to delete the reference whatever
 	 * its current value.
 	 */
-	if (!refs_read_ref_full(&refs->base, newrefname,
+	if (!copy && !refs_read_ref_full(&refs->base, newrefname,
 				RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
 				oid.hash, NULL) &&
 	    refs_delete_ref(&refs->base, NULL, newrefname,
@@ -3318,7 +3325,7 @@ struct ref_storage_be refs_be_files = {
 	files_peel_ref,
 	files_create_symref,
 	files_delete_refs,
-	files_rename_ref,
+	files_copy_or_rename_ref,
 
 	files_ref_iterator_begin,
 	files_read_raw_ref,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index b6b291cf00e5c..91d59b01fb570 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -510,9 +510,9 @@ typedef int create_symref_fn(struct ref_store *ref_store,
 			     const char *logmsg);
 typedef int delete_refs_fn(struct ref_store *ref_store,
 			   struct string_list *refnames, unsigned int flags);
-typedef int rename_ref_fn(struct ref_store *ref_store,
+typedef int copy_or_rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
-			  const char *logmsg);
+			  const char *logmsg, int copy);
 
 /*
  * Iterate over the references in the specified ref_store that are
@@ -606,7 +606,7 @@ struct ref_storage_be {
 	peel_ref_fn *peel_ref;
 	create_symref_fn *create_symref;
 	delete_refs_fn *delete_refs;
-	rename_ref_fn *rename_ref;
+	copy_or_rename_ref_fn *copy_or_rename_ref;
 
 	ref_iterator_begin_fn *iterator_begin;
 	read_raw_ref_fn *read_raw_ref;
diff --git a/submodule.c b/submodule.c
index bf5a93d16fb71..d93f366be31c6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -107,7 +107,7 @@ int remove_path_from_gitmodules(const char *path)
 	}
 	strbuf_addstr(&sect, "submodule.");
 	strbuf_addstr(&sect, submodule->name);
-	if (git_config_rename_section_in_file(".gitmodules", sect.buf, NULL) < 0) {
+	if (git_config_copy_or_rename_section_in_file(".gitmodules", sect.buf, NULL) < 0) {
 		/* Maybe the user already did that, don't error out here */
 		warning(_("Could not remove .gitmodules entry for %s"), path);
 		strbuf_release(&sect);
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index fba85e7da58fb..bfa031d77c8f0 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -107,7 +107,7 @@ static int cmd_rename_ref(struct ref_store *refs, const char **argv)
 	const char *newref = notnull(*argv++, "newref");
 	const char *logmsg = *argv++;
 
-	return refs_rename_ref(refs, oldref, newref, logmsg);
+	return refs_copy_or_rename_ref(refs, oldref, newref, logmsg, 0);
 }
 
 static int each_ref(const char *refname, const struct object_id *oid,

--
https://github.com/git/git/pull/363

[PATCH/RFC v2 5/6] config: add copy config section logic

[Sahil Dua]

Adds implementation for copying the config section while copying a
branch.

While we're parsing the config file, we need to make sure we start
copying the config section once we find the matching block for our
branch1 (for example when running 'git branch -c branch1 branch2').

There is one flag used - 'copying_section' which can take 0/1/2 values.
0 - not copying currently
1 - just started copying section
2 - currently copying
I thought of making this flag binary to keep things easier. However,
since there was distinction in behavior(adding to currently copied
section) depending upon whether it's the first line of config section or
not.

The copied section has first line which contains the new branch name
(branch2 in our example). This is achieved using store_create_section
method.

Once we're done with reading the entire config file, we write our copied
section. Hence, literally copying the config section from branch1 to
branch2.

However, there's one case which is not handled by this yet - when
branch2 already has some configuration and -C command is used, operation
should delete the present configuration for branch2.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 config.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/config.c b/config.c
index 155274f03b2b6..2bf711ca3e0da 100644
--- a/config.c
+++ b/config.c
@@ -2642,13 +2642,14 @@ static int section_name_is_ok(const char *name)
 int git_config_copy_or_rename_section_in_file(const char *config_filename,
 				      const char *old_name, const char *new_name, int copy)
 {
-	int ret = 0, remove = 0;
+	int ret = 0, remove = 0, copying_section = 0, copied_section_length;
 	char *filename_buf = NULL;
 	struct lock_file *lock;
 	int out_fd;
 	char buf[1024];
 	FILE *config_file = NULL;
 	struct stat st;
+	struct strbuf copied_section;
 
 	if (new_name && !section_name_is_ok(new_name)) {
 		ret = error("invalid section name: %s", new_name);
@@ -2689,6 +2690,13 @@ int git_config_copy_or_rename_section_in_file(const char *config_filename,
 			; /* do nothing */
 		if (buf[i] == '[') {
 			/* it's a section */
+			if (copying_section) {
+				/* Mark the end of copying the matching
+				 * section, as this is the beginning
+				 * of the new section
+				 */
+				copying_section = 0;
+			}
 			int offset = section_name_match(&buf[i], old_name);
 			if (offset > 0) {
 				ret++;
@@ -2696,26 +2704,41 @@ int git_config_copy_or_rename_section_in_file(const char *config_filename,
 					remove = 1;
 					continue;
 				}
-				store.baselen = strlen(new_name);
-				if (!store_write_section(out_fd, new_name)) {
-					ret = write_error(get_lock_file_path(lock));
-					goto out;
+				if (!copy) {
+					store.baselen = strlen(new_name);
+					if (!store_write_section(out_fd, new_name)) {
+						ret = write_error(get_lock_file_path(lock));
+						goto out;
+					}
+				} else {
+					/* Mark the beginning of copying the matching section */
+					copying_section = 1;
+
+					/* TODO: Make this work for the
+					 * case when there are multiple
+					 * matching sections
+					 */
+					/* Create a section with new branch name */
+					store.baselen = strlen(new_name);
+					copied_section = store_create_section(new_name);
 				}
 				/*
 				 * We wrote out the new section, with
 				 * a newline, now skip the old
 				 * section's length
 				 */
-				output += offset + i;
-				if (strlen(output) > 0) {
-					/*
-					 * More content means there's
-					 * a declaration to put on the
-					 * next line; indent with a
-					 * tab
-					 */
-					output -= 1;
-					output[0] = '\t';
+				if (!copy) {
+					output += offset + i;
+					if (strlen(output) > 0) {
+						/*
+						 * More content means there's
+						 * a declaration to put on the
+						 * next line; indent with a
+						 * tab
+						 */
+						output -= 1;
+						output[0] = '\t';
+					}
 				}
 			}
 			remove = 0;
@@ -2723,11 +2746,25 @@ int git_config_copy_or_rename_section_in_file(const char *config_filename,
 		if (remove)
 			continue;
 		length = strlen(output);
+
+		if (copying_section > 1) {
+			strbuf_addf(&copied_section, "%s", output);
+		} else if (copying_section == 1) {
+			copying_section = 2;
+		}
 		if (write_in_full(out_fd, output, length) != length) {
 			ret = write_error(get_lock_file_path(lock));
 			goto out;
 		}
 	}
+
+	if (copy && copied_section.len > 0) {
+		copied_section_length = strlen(copied_section.buf);
+		if (write_in_full(out_fd, copied_section.buf, copied_section_length) != copied_section_length) {
+			ret = write_error(get_lock_file_path(lock));
+			goto out;
+		}
+	}
 	fclose(config_file);
 	config_file = NULL;
 commit_and_out:

--
https://github.com/git/git/pull/363

Re: [PATCH/RFC v2 2/6] branch: add copy branch option

[Junio C Hamano]

Sahil Dua <sahildua2305@gmail.com> writes:

> Adds copy branch option available using -c or -C (forcefully).
>
> Includes a lot of function renames and their signature changes in order
> to introduce a new function parameter - flag 'copy' which determines
> whether those functions should do operation copy or move.
>
> Additionally, this changes a lot of other files wherever the renamed
> functions were used. By default copy=0 is passed at all those places so
> that they keep behaving the way they were, before these changes.

Things like rename_branch() that is narrowly confined inside a
single program (i.e. builtin/branch.c), if renaming and copying
shares a lot of logic and there is only a single caller to rename,
it may be OK to rename the function to rename_or_copy_branch() and
pass a new "are we doing copy or move?" parameter, but for lower
level infrastructure like config_rename_section(), I am afraid to
say that such a change is totally unacceptable.  When the current
callers are content with rename_section(), and have no need to ever
copy, why should they be forced tocall copy-or-rename with copy set
to 0?

When the original code looks like:


    == caller (there are many) ==

    rename_it(a, b);

    == implementation (only one) ==

    int rename_it(src, dst) {
	... logic to create dst by copying src ...
	... logic to remove src ...
    }

You could introduce a common helper

    == implementation ==

    int rename_or_copy_it(src, dst, copy?) {
	... logic to create dst by copying src ...
	if (!copy?) {
	    ... logic to remove src ...
	}
    }

but to help the current code (and possibly code somebody _else_ is
developing elsewhere), you can also do it in a much less disruptive
way.

    == implementation ==

    static int rename_or_copy_it(src, dst, copy?) {
	... logic to create dst by copying src ...
	if (!copy?) {
	    ... logic to remove src ...
	}
    }

    int rename_it(src, dst) {
	return rename_or_copy_it(src, dst, 0);
    }

    int copy_it(src, dst) {
	return rename_or_copy_it(src, dst, 1);
    }

Existing callers of "rename" that are not interested in your new
"copy" thing can be left oblivious to it if you did it that way.

Re: [PATCH/RFC v2 2/6] branch: add copy branch option

[Sahil Dua]

On Thu, Jun 1, 2017 at 3:50 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Sahil Dua <sahildua2305@gmail.com> writes:
>
>> Adds copy branch option available using -c or -C (forcefully).
>>
>> Includes a lot of function renames and their signature changes in order
>> to introduce a new function parameter - flag 'copy' which determines
>> whether those functions should do operation copy or move.
>>
>> Additionally, this changes a lot of other files wherever the renamed
>> functions were used. By default copy=0 is passed at all those places so
>> that they keep behaving the way they were, before these changes.
>
> Things like rename_branch() that is narrowly confined inside a
> single program (i.e. builtin/branch.c), if renaming and copying
> shares a lot of logic and there is only a single caller to rename,
> it may be OK to rename the function to rename_or_copy_branch() and
> pass a new "are we doing copy or move?" parameter, but for lower
> level infrastructure like config_rename_section(), I am afraid to
> say that such a change is totally unacceptable.  When the current
> callers are content with rename_section(), and have no need to ever
> copy, why should they be forced tocall copy-or-rename with copy set
> to 0?
>
> When the original code looks like:
>
>
>     == caller (there are many) ==
>
>     rename_it(a, b);
>
>     == implementation (only one) ==
>
>     int rename_it(src, dst) {
>         ... logic to create dst by copying src ...
>         ... logic to remove src ...
>     }
>
> You could introduce a common helper
>
>     == implementation ==
>
>     int rename_or_copy_it(src, dst, copy?) {
>         ... logic to create dst by copying src ...
>         if (!copy?) {
>             ... logic to remove src ...
>         }
>     }
>
> but to help the current code (and possibly code somebody _else_ is
> developing elsewhere), you can also do it in a much less disruptive
> way.
>
>     == implementation ==
>
>     static int rename_or_copy_it(src, dst, copy?) {
>         ... logic to create dst by copying src ...
>         if (!copy?) {
>             ... logic to remove src ...
>         }
>     }
>
>     int rename_it(src, dst) {
>         return rename_or_copy_it(src, dst, 0);
>     }
>
>     int copy_it(src, dst) {
>         return rename_or_copy_it(src, dst, 1);
>     }
>
> Existing callers of "rename" that are not interested in your new
> "copy" thing can be left oblivious to it if you did it that way.
>

Thanks for your comments. I was already a little sceptic about making
changes in all the callers for config_rename_section function. I will
make the changes to have a helper function so that the existing
callers of this method aren't affected at all.

[PATCH/RFC v3 1/3] branch: add tests for new copy branch feature

[Sahil Dua]

Adds a few basic tests for the new copy branch option. Aim is to
have an option -c for copying a branch just like -m option for
renaming a branch.

My definition of "copy" for this feature is "copying from A to create
B, keeping A intact". That means "copy branch A to B" should do
whatever "move branch A to B" does except it shouldn't delete A and
should keep A unchanged.

1. When a branch topic-2 is created by copying from branch topic-1,
topic-2 branch reflog should now contain the all the entries of
topic-1 branch (before copying) followed by "Copied from topic-1".
[This is debatable though, I want inputs/suggestions about this.]

2. Copying a branch should also copy the git config section for that
branch. This means if topic-2 branch is created from topic-1,
"branch.topic-2.remote" should now be same as "branch.topic-1.remote",
if set.

3. "git push" to copied branch for example - topic-2 should push a new
branch with the same name in the remote repo. That means if topic-1
was previously pushed and a new branch topic-2 is copied from topic-1,
"git push" on topic-2 branch won't push to the same branch as "git
push on topic-1 branch would.

4. "git branch -c new-branch" should copy the currently checked out
branch and create a new branch with name "new-branch".

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 t/t3200-branch.sh | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fe62e7c775da6..6176a9c5cd85a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -341,6 +341,106 @@ test_expect_success 'config information was renamed, too' '
 	test_must_fail git config branch.s/s/dummy
 '
 
+test_expect_success 'git branch -c dumps usage' '
+	test_expect_code 128 git branch -c 2>err &&
+	test_i18ngrep "branch name required" err
+'
+
+test_expect_success 'git branch -c d e should work' '
+	git branch -l d &&
+	git reflog exists refs/heads/d &&
+	git config branch.d.dummy Hello &&
+	git branch -c d e &&
+	git reflog exists refs/heads/d &&
+	git reflog exists refs/heads/e &&
+	test $(git config branch.e.dummy) = Hello &&
+	test $(git config branch.d.dummy) = Hello
+'
+
+test_expect_success 'git branch -c f/f g/g should work' '
+	git branch -l f/f &&
+	git reflog exists refs/heads/f/f &&
+	git config branch.f/f.dummy Hello &&
+	git branch -c f/f g/g &&
+	git reflog exists refs/heads/f/f &&
+	git reflog exists refs/heads/g/g &&
+	test $(git config branch.f/f.dummy) = Hello &&
+	test $(git config branch.g/g.dummy) = Hello
+'
+
+test_expect_success 'git branch -c m2 m2 should work' '
+	git branch -l m2 &&
+	git reflog exists refs/heads/m2 &&
+	git config branch.m2.dummy Hello &&
+	git branch -c m2 m2 &&
+	git reflog exists refs/heads/m2 &&
+	test $(git config branch.m2.dummy) = Hello
+'
+
+test_expect_success 'git branch -c a a/a should fail' '
+	git branch -l a &&
+	git reflog exists refs/heads/a &&
+	test_must_fail git branch -c a a/a
+'
+
+test_expect_success 'git branch -c b/b b should fail' '
+	git branch -l b/b &&
+	test_must_fail git branch -c b/b b
+'
+
+test_expect_success 'git branch -C o/q o/p should work when o/p exists' '
+	git branch -l o/q &&
+	git reflog exists refs/heads/o/q &&
+	git reflog exists refs/heads/o/p &&
+	git branch -C o/q o/p
+'
+
+test_expect_success 'git branch -c -f o/q o/p should work when o/p exists' '
+	git reflog exists refs/heads/o/q &&
+	git reflog exists refs/heads/o/p &&
+	git branch -m -f o/q o/p
+'
+
+test_expect_success 'git branch -c q r/q should fail when r exists' '
+	git branch q &&
+	git branch r &&
+	test_must_fail git branch -c q r/q
+'
+
+test_expect_success 'git branch -C b1 b2 should fail when b2 is checked out' '
+	git branch b1 &&
+	git checkout -b b2 &&
+	test_must_fail git branch -C b1 b2
+'
+
+test_expect_success 'git branch -C c1 c2 should succeed when c1 is checked out' '
+	git checkout -b c1 &&
+	git branch c2 &&
+	git branch -C c1 c2 &&
+	test $(git rev-parse --abbrev-ref HEAD) = c2
+'
+
+test_expect_success 'git branch -C c1 c2 should add entries to .git/logs/HEAD' '
+	msg="Branch: copied refs/heads/c1 to refs/heads/c2" &&
+	grep "^0\{40\}.*$msg$" .git/logs/HEAD
+'
+
+test_expect_success 'git branch -C master should work when master is checked out' '
+	git checkout master &&
+	git branch -C master
+'
+
+test_expect_success 'git branch -C master master should work when master is checked out' '
+	git checkout master &&
+	git branch -C master master
+'
+
+test_expect_success 'git branch -C master5 master5 should work when master is checked out' '
+	git checkout master &&
+	git branch master5 &&
+	git branch -C master5 master5
+'
+
 test_expect_success 'deleting a symref' '
 	git branch target &&
 	git symbolic-ref refs/heads/symref refs/heads/target &&

--
https://github.com/git/git/pull/363

[PATCH/RFC v3 2/3] config: abstract out create section from key logic

[Sahil Dua]

Abstracts out the logic for creating string buffer from given key for
example - 'branch.b' and returns '[branch "b"]'.

We want to keep the original config section intact in case of copy
operation. For this we need to fetch the section with updated new branch
name so that we can write that to the config file.

For example - git branch -c foo bar
The mentioned/edited function renames and overwrites this part in the
config - [branch "foo"] to [branch "bar"]. However, in case of copy, we
want to keep the original [branch "foo"] intact and get [branch "bar"]
from "branch.bar" key. 'store_create_section' function will return
[branch "bar"] when "branch.bar" is passed.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 config.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 146cb3452adab..721aef1938081 100644
--- a/config.c
+++ b/config.c
@@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
 	return 4;
 }
 
-static int store_write_section(int fd, const char *key)
+struct strbuf store_create_section(const char *key)
 {
 	const char *dot;
-	int i, success;
+	int i;
 	struct strbuf sb = STRBUF_INIT;
 
 	dot = memchr(key, '.', store.baselen);
@@ -2188,6 +2188,16 @@ static int store_write_section(int fd, const char *key)
 		strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
 	}
 
+	return sb;
+}
+
+static int store_write_section(int fd, const char *key)
+{
+	int success;
+
+	/* Create a section with the given key */
+	struct strbuf sb = store_create_section(key);
+
 	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
 	strbuf_release(&sb);
 

--
https://github.com/git/git/pull/363

[PATCH/RFC v3 3/3] branch: add copy branch feature implementation

[Sahil Dua]

Adds copy branch option available using -c or -C (forcefully).

Adds new function copy_existing_ref to copy the ref and
git_config_copy_section to copy the git config section. Under the hood,
both git_config_copy_section and git_config_rename_section functions use
the same function which takes a flag copy? to decide whether it should
copy or move the config section.

Similarly for copy_existing_ref - rename_ref and copy_existing_ref use
same function which takes the copy? flag and work in the same way as
mentioned above.

The new function is named copy_existing_ref instead of copy_ref because
of conflicts with another function copy_ref from remote.h.

The new copied branch logs start with a line mentioning the copy
operation.

git config copy section logic:
While we're parsing the config file, we need to make sure we start
copying the config section once we find the matching block for our
branch1 (for example when running 'git branch -c branch1 branch2').

There is one flag used - 'copying_section' which can take 0/1/2 values.
0 - not copying currently
1 - just started copying section
2 - currently copying
I thought of making this flag binary to keep things easier. However,
since there was distinction in behavior(adding to currently copied
section) depending upon whether it's the first line of config section or
not.

The copied section has first line which contains the new branch name
(branch2 in our example). This is achieved using store_create_section
method.

Once we're done with reading the entire config file, we write our copied
section. Hence, literally copying the config section from branch1 to
branch2.

However, there's one case which is not handled by this yet - when
branch2 already has some configuration and -C command is used, operation
should delete the present configuration for branch2.

Also, it doesn't make sense to trigger config section copy config
section function if both the branch names are same.

For example - git branch -C a a
In such a case, it shouldn't try to copy or rename the git config
section.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 builtin/branch.c     | 45 ++++++++++++++++++--------
 cache.h              |  2 ++
 config.c             | 90 ++++++++++++++++++++++++++++++++++++++++++----------
 refs.c               | 11 +++++++
 refs.h               |  9 +++++-
 refs/files-backend.c | 34 +++++++++++++++++---
 refs/refs-internal.h |  4 +++
 7 files changed, 159 insertions(+), 36 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 83fcda43dceec..8c0cdafda5b70 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-l] [-f] <branch-name> [<start-point>]"),
 	N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
 	N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
+	N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
 	N_("git branch [<options>] [-r | -a] [--points-at]"),
 	N_("git branch [<options>] [-r | -a] [--format]"),
 	NULL
@@ -449,7 +450,7 @@ static void reject_rebase_or_bisect_branch(const char *target)
 	free_worktrees(worktrees);
 }
 
-static void rename_branch(const char *oldname, const char *newname, int force)
+static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)
 {
 	struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
 	struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
@@ -457,7 +458,8 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	int clobber_head_ok;
 
 	if (!oldname)
-		die(_("cannot rename the current branch while not on any."));
+		die(_("cannot %s the current branch while not on any."),
+			 (copy ? "copy" : "rename"));
 
 	if (strbuf_check_branch_ref(&oldref, oldname)) {
 		/*
@@ -480,17 +482,21 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 
 	reject_rebase_or_bisect_branch(oldref.buf);
 
-	strbuf_addf(&logmsg, "Branch: renamed %s to %s",
-		 oldref.buf, newref.buf);
+	strbuf_addf(&logmsg, "Branch: %s %s to %s",
+		 (copy ? "copied" : "renamed"), oldref.buf, newref.buf);
 
-	if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
+	if (!copy && rename_ref(oldref.buf, newref.buf, logmsg.buf))
+		die(_("Branch rename failed"));
+	if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
 		die(_("Branch rename failed"));
 
 	if (recovery)
-		warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);
+		warning(_("%s a misnamed branch '%s' away"),
+			 (copy ? "copied" : "renamed"), oldref.buf + 11);
 
 	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))
-		die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+		die(_("Branch %s to %s, but HEAD is not updated!"),
+			 (copy ? "copied" : "renamed"), newname);
 
 	strbuf_release(&logmsg);
 
@@ -498,8 +504,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	strbuf_release(&oldref);
 	strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
 	strbuf_release(&newref);
-	if (git_config_rename_section(oldsection.buf, newsection.buf) < 0)
+	if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
 		die(_("Branch is renamed, but update of config-file failed"));
+	if (copy && strcmp(oldname, newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
+		die(_("Branch is copied, but update of config-file failed"));
 	strbuf_release(&oldsection);
 	strbuf_release(&newsection);
 }
@@ -537,7 +545,7 @@ static int edit_branch_description(const char *branch_name)
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-	int delete = 0, rename = 0, force = 0, list = 0;
+	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
@@ -574,6 +582,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
 		OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
 		OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
+		OPT_BIT('c', NULL, &copy, N_("copy a branch and its reflog"), 1),
+		OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
 		OPT_BOOL(0, "list", &list, N_("list branch names")),
 		OPT_BOOL('l', "create-reflog", &reflog, N_("create the branch's reflog")),
 		OPT_BOOL(0, "edit-description", &edit_description,
@@ -617,14 +627,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
+	if (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
 	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
 	    filter.no_commit)
 		list = 1;
 
-	if (!!delete + !!rename + !!new_upstream +
+	if (!!delete + !!rename + !!copy + !!new_upstream +
 	    list + unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
@@ -696,13 +706,22 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (edit_branch_description(branch_name))
 			return 1;
+	} else if (copy) {
+		if (!argc)
+			die(_("branch name required"));
+		else if (argc == 1)
+			copy_or_rename_branch(head, argv[0], 1, copy > 1);
+		else if (argc == 2)
+			copy_or_rename_branch(argv[0], argv[1], 1, copy > 1);
+		else
+			die(_("too many branches for a copy operation"));
 	} else if (rename) {
 		if (!argc)
 			die(_("branch name required"));
 		else if (argc == 1)
-			rename_branch(head, argv[0], rename > 1);
+			copy_or_rename_branch(head, argv[0], 0, rename > 1);
 		else if (argc == 2)
-			rename_branch(argv[0], argv[1], rename > 1);
+			copy_or_rename_branch(argv[0], argv[1], 0, rename > 1);
 		else
 			die(_("too many branches for a rename operation"));
 	} else if (new_upstream) {
diff --git a/cache.h b/cache.h
index ae4c45d379d5b..109d91cb5248a 100644
--- a/cache.h
+++ b/cache.h
@@ -1935,6 +1935,8 @@ extern int git_config_set_multivar_in_file_gently(const char *, const char *, co
 extern void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
 extern int git_config_rename_section_in_file(const char *, const char *, const char *);
+extern int git_config_copy_section(const char *, const char *);
+extern int git_config_copy_section_in_file(const char *, const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
diff --git a/config.c b/config.c
index 721aef1938081..9d67fa846ff30 100644
--- a/config.c
+++ b/config.c
@@ -2639,16 +2639,17 @@ static int section_name_is_ok(const char *name)
 }
 
 /* if new_name == NULL, the section is removed instead */
-int git_config_rename_section_in_file(const char *config_filename,
-				      const char *old_name, const char *new_name)
+static int git_config_copy_or_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name, int copy)
 {
-	int ret = 0, remove = 0;
+	int ret = 0, remove = 0, copying_section = 0, copied_section_length;
 	char *filename_buf = NULL;
 	struct lock_file *lock;
 	int out_fd;
 	char buf[1024];
 	FILE *config_file = NULL;
 	struct stat st;
+	struct strbuf copied_section;
 
 	if (new_name && !section_name_is_ok(new_name)) {
 		ret = error("invalid section name: %s", new_name);
@@ -2689,6 +2690,13 @@ int git_config_rename_section_in_file(const char *config_filename,
 			; /* do nothing */
 		if (buf[i] == '[') {
 			/* it's a section */
+			if (copying_section) {
+				/* Mark the end of copying the matching
+				 * section, as this is the beginning
+				 * of the new section
+				 */
+				copying_section = 0;
+			}
 			int offset = section_name_match(&buf[i], old_name);
 			if (offset > 0) {
 				ret++;
@@ -2696,26 +2704,41 @@ int git_config_rename_section_in_file(const char *config_filename,
 					remove = 1;
 					continue;
 				}
-				store.baselen = strlen(new_name);
-				if (!store_write_section(out_fd, new_name)) {
-					ret = write_error(get_lock_file_path(lock));
-					goto out;
+				if (!copy) {
+					store.baselen = strlen(new_name);
+					if (!store_write_section(out_fd, new_name)) {
+						ret = write_error(get_lock_file_path(lock));
+						goto out;
+					}
+				} else {
+					/* Mark the beginning of copying the matching section */
+					copying_section = 1;
+
+					/* TODO: Make this work for the
+					 * case when there are multiple
+					 * matching sections
+					 */
+					/* Create a section with new branch name */
+					store.baselen = strlen(new_name);
+					copied_section = store_create_section(new_name);
 				}
 				/*
 				 * We wrote out the new section, with
 				 * a newline, now skip the old
 				 * section's length
 				 */
-				output += offset + i;
-				if (strlen(output) > 0) {
-					/*
-					 * More content means there's
-					 * a declaration to put on the
-					 * next line; indent with a
-					 * tab
-					 */
-					output -= 1;
-					output[0] = '\t';
+				if (!copy) {
+					output += offset + i;
+					if (strlen(output) > 0) {
+						/*
+						 * More content means there's
+						 * a declaration to put on the
+						 * next line; indent with a
+						 * tab
+						 */
+						output -= 1;
+						output[0] = '\t';
+					}
 				}
 			}
 			remove = 0;
@@ -2723,11 +2746,25 @@ int git_config_rename_section_in_file(const char *config_filename,
 		if (remove)
 			continue;
 		length = strlen(output);
+
+		if (copying_section > 1) {
+			strbuf_addf(&copied_section, "%s", output);
+		} else if (copying_section == 1) {
+			copying_section = 2;
+		}
 		if (write_in_full(out_fd, output, length) != length) {
 			ret = write_error(get_lock_file_path(lock));
 			goto out;
 		}
 	}
+
+	if (copy && copied_section.len > 0) {
+		copied_section_length = strlen(copied_section.buf);
+		if (write_in_full(out_fd, copied_section.buf, copied_section_length) != copied_section_length) {
+			ret = write_error(get_lock_file_path(lock));
+			goto out;
+		}
+	}
 	fclose(config_file);
 	config_file = NULL;
 commit_and_out:
@@ -2743,11 +2780,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 	return ret;
 }
 
+int git_config_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
+{
+	return git_config_copy_or_rename_section_in_file(config_filename,
+					 old_name, new_name, 0);
+}
+
 int git_config_rename_section(const char *old_name, const char *new_name)
 {
 	return git_config_rename_section_in_file(NULL, old_name, new_name);
 }
 
+int git_config_copy_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
+{
+	return git_config_copy_or_rename_section_in_file(config_filename,
+					 old_name, new_name, 1);
+}
+
+int git_config_copy_section(const char *old_name, const char *new_name)
+{
+	return git_config_copy_section_in_file(NULL, old_name, new_name);
+}
+
 /*
  * Call this to report error for your variable that should not
  * get a boolean value (i.e. "[my] var" means "true").
diff --git a/refs.c b/refs.c
index 8af9641aa17e6..42b1abef76b1a 100644
--- a/refs.c
+++ b/refs.c
@@ -1917,3 +1917,14 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
 	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
 }
+
+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg)
+{
+	return refs->be->copy_ref(refs, oldref, newref, logmsg);
+}
+
+int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
+{
+	return refs_copy_existing_ref(get_main_ref_store(), oldref, newref, logmsg);
+}
diff --git a/refs.h b/refs.h
index 685a979a0eb70..df93d5a261b6c 100644
--- a/refs.h
+++ b/refs.h
@@ -396,7 +396,14 @@ char *shorten_unambiguous_ref(const char *refname, int strict);
 /** rename ref, return 0 on success **/
 int refs_rename_ref(struct ref_store *refs, const char *oldref,
 		    const char *newref, const char *logmsg);
-int rename_ref(const char *oldref, const char *newref, const char *logmsg);
+int rename_ref(const char *oldref, const char *newref,
+			const char *logmsg);
+
+/** copy ref, return 0 on success **/
+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg);
+int copy_existing_ref(const char *oldref, const char *newref,
+			const char *logmsg);
 
 int refs_create_symref(struct ref_store *refs, const char *refname,
 		       const char *target, const char *logmsg);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index cb1f528cbeec4..41546d957f2be 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1703,9 +1703,9 @@ static int commit_ref_update(struct files_ref_store *refs,
 			     const struct object_id *oid, const char *logmsg,
 			     struct strbuf *err);
 
-static int files_rename_ref(struct ref_store *ref_store,
+static int files_copy_or_rename_ref(struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
-			    const char *logmsg)
+			    const char *logmsg, int copy)
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
@@ -1746,13 +1746,20 @@ static int files_rename_ref(struct ref_store *ref_store,
 		goto out;
 	}
 
-	if (log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
+	if (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
 		ret = error("unable to move logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
 			    oldrefname, strerror(errno));
 		goto out;
 	}
 
-	if (refs_delete_ref(&refs->base, logmsg, oldrefname,
+	// TODO: merge this block with the rename one above
+	if (copy && log && copy_file(tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
+		ret = error("unable to copy logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
+			    oldrefname, strerror(errno));
+		goto out;
+	}
+
+	if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,
 			    orig_oid.hash, REF_NODEREF)) {
 		error("unable to delete old %s", oldrefname);
 		goto rollback;
@@ -1765,7 +1772,7 @@ static int files_rename_ref(struct ref_store *ref_store,
 	 * the safety anyway; we want to delete the reference whatever
 	 * its current value.
 	 */
-	if (!refs_read_ref_full(&refs->base, newrefname,
+	if (!copy && !refs_read_ref_full(&refs->base, newrefname,
 				RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
 				oid.hash, NULL) &&
 	    refs_delete_ref(&refs->base, NULL, newrefname,
@@ -1847,6 +1854,22 @@ static int files_rename_ref(struct ref_store *ref_store,
 	return ret;
 }
 
+static int files_rename_ref(struct ref_store *ref_store,
+			    const char *oldrefname, const char *newrefname,
+			    const char *logmsg)
+{
+	return files_copy_or_rename_ref(ref_store, oldrefname,
+				 newrefname, logmsg, 0);
+}
+
+static int files_copy_ref(struct ref_store *ref_store,
+			    const char *oldrefname, const char *newrefname,
+			    const char *logmsg)
+{
+	return files_copy_or_rename_ref(ref_store, oldrefname,
+				 newrefname, logmsg, 1);
+}
+
 static int close_ref(struct ref_lock *lock)
 {
 	if (close_lock_file(lock->lk))
@@ -3319,6 +3342,7 @@ struct ref_storage_be refs_be_files = {
 	files_create_symref,
 	files_delete_refs,
 	files_rename_ref,
+	files_copy_ref,
 
 	files_ref_iterator_begin,
 	files_read_raw_ref,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index b6b291cf00e5c..823a1c48af6ec 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -513,6 +513,9 @@ typedef int delete_refs_fn(struct ref_store *ref_store,
 typedef int rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
+typedef int copy_ref_fn(struct ref_store *ref_store,
+			  const char *oldref, const char *newref,
+			  const char *logmsg);
 
 /*
  * Iterate over the references in the specified ref_store that are
@@ -607,6 +610,7 @@ struct ref_storage_be {
 	create_symref_fn *create_symref;
 	delete_refs_fn *delete_refs;
 	rename_ref_fn *rename_ref;
+	copy_ref_fn *copy_ref;
 
 	ref_iterator_begin_fn *iterator_begin;
 	read_raw_ref_fn *read_raw_ref;

--
https://github.com/git/git/pull/363

Re: [PATCH/RFC v3 3/3] branch: add copy branch feature implementation

[Ævar Arnfjörð Bjarmason]

On Thu, Jun 1, 2017 at 8:35 PM, Sahil Dua <sahildua2305@gmail.com> wrote:
> Adds copy branch option available using -c or -C (forcefully).

Commenting on the series in general. I have a fixup branch for you
with commits to squash:
https://github.com/avar/git/tree/avar/sahildua-rename-branch-2 issues:

 - There's a mixed variable declaration  with code, should be predeclared.
 - Stuff like printf(_("% branch), "copied") gives bad translations,
needs to be expanded
 - Fixed up comment style

In addition when I try to run the tests I get:

    fatal: cannot lock ref 'refs/heads/a': 'refs/heads/a/b/c' exists;
cannot create 'refs/heads/a'
    not ok 45 - git branch -c a a/a should fail

And there's 18 other failures in that test script, although some could
be from that first failure, doesn't this fail for you? I can't see why
it wouldn't everywhere, i.e. you're trying to create a "a" ref when
"a/b/c" exists.

Re: [PATCH/RFC v3 3/3] branch: add copy branch feature implementation

[Sahil Dua]

On Thu, Jun 1, 2017 at 8:59 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Thu, Jun 1, 2017 at 8:35 PM, Sahil Dua <sahildua2305@gmail.com> wrote:
> > Adds copy branch option available using -c or -C (forcefully).
>
> Commenting on the series in general. I have a fixup branch for you
> with commits to squash:
> https://github.com/avar/git/tree/avar/sahildua-rename-branch-2 issues:
>
>  - There's a mixed variable declaration  with code, should be predeclared.
>  - Stuff like printf(_("% branch), "copied") gives bad translations,
> needs to be expanded
>  - Fixed up comment style
>

Thanks for fixing the issues.

> In addition when I try to run the tests I get:
>
>     fatal: cannot lock ref 'refs/heads/a': 'refs/heads/a/b/c' exists;
> cannot create 'refs/heads/a'
>     not ok 45 - git branch -c a a/a should fail
>
> And there's 18 other failures in that test script, although some could
> be from that first failure, doesn't this fail for you? I can't see why
> it wouldn't everywhere, i.e. you're trying to create a "a" ref when
> "a/b/c" exists.

Indeed I see 3 of the tests failing. The issue is the existing
branches. I have the fix for them by changing the branch name. I will
push it and send another patch along with the above changes.

However, I am having problems in getting one test pass - "git branch
-C c1 c2 should add entries to .git/logs/HEAD". I'm trying to figure
it out.

"Branch copied" log is being added to .git/logs/HEAD. However, it
doesn't work in a similar way as -M option. Hence, it's not being
returned when we grep by ^0\{40\}.*$msg$.

[PATCH/RFC v4 1/3] branch: add tests for new copy branch feature

[Sahil Dua]

Adds a few basic tests for the new copy branch option. Aim is to
have an option -c for copying a branch just like -m option for
renaming a branch.

My definition of "copy" for this feature is "copying from A to create
B, keeping A intact". That means "copy branch A to B" should do
whatever "move branch A to B" does except it shouldn't delete A and
should keep A unchanged.

1. When a branch topic-2 is created by copying from branch topic-1,
topic-2 branch reflog should now contain the all the entries of
topic-1 branch (before copying) followed by "Copied from topic-1".
[This is debatable though, I want inputs/suggestions about this.]

2. Copying a branch should also copy the git config section for that
branch. This means if topic-2 branch is created from topic-1,
"branch.topic-2.remote" should now be same as "branch.topic-1.remote",
if set.

3. "git push" to copied branch for example - topic-2 should push a new
branch with the same name in the remote repo. That means if topic-1
was previously pushed and a new branch topic-2 is copied from topic-1,
"git push" on topic-2 branch won't push to the same branch as "git
push on topic-1 branch would.

4. "git branch -c new-branch" should copy the currently checked out
branch and create a new branch with name "new-branch".

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 t/t3200-branch.sh | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index fe62e7c775da6..91b9d07539bec 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -341,6 +341,106 @@ test_expect_success 'config information was renamed, too' '
 	test_must_fail git config branch.s/s/dummy
 '
 
+test_expect_success 'git branch -c dumps usage' '
+	test_expect_code 128 git branch -c 2>err &&
+	test_i18ngrep "branch name required" err
+'
+
+test_expect_success 'git branch -c d e should work' '
+	git branch -l d &&
+	git reflog exists refs/heads/d &&
+	git config branch.d.dummy Hello &&
+	git branch -c d e &&
+	git reflog exists refs/heads/d &&
+	git reflog exists refs/heads/e &&
+	test $(git config branch.e.dummy) = Hello &&
+	test $(git config branch.d.dummy) = Hello
+'
+
+test_expect_success 'git branch -c f/f g/g should work' '
+	git branch -l f/f &&
+	git reflog exists refs/heads/f/f &&
+	git config branch.f/f.dummy Hello &&
+	git branch -c f/f g/g &&
+	git reflog exists refs/heads/f/f &&
+	git reflog exists refs/heads/g/g &&
+	test $(git config branch.f/f.dummy) = Hello &&
+	test $(git config branch.g/g.dummy) = Hello
+'
+
+test_expect_success 'git branch -c m2 m2 should work' '
+	git branch -l m2 &&
+	git reflog exists refs/heads/m2 &&
+	git config branch.m2.dummy Hello &&
+	git branch -c m2 m2 &&
+	git reflog exists refs/heads/m2 &&
+	test $(git config branch.m2.dummy) = Hello
+'
+
+test_expect_success 'git branch -c zz zz/zz should fail' '
+	git branch -l zz &&
+	git reflog exists refs/heads/zz &&
+	test_must_fail git branch -c zz zz/zz
+'
+
+test_expect_success 'git branch -c b/b b should fail' '
+	git branch -l b/b &&
+	test_must_fail git branch -c b/b b
+'
+
+test_expect_success 'git branch -C o/q o/p should work when o/p exists' '
+	git branch -l o/q &&
+	git reflog exists refs/heads/o/q &&
+	git reflog exists refs/heads/o/p &&
+	git branch -C o/q o/p
+'
+
+test_expect_success 'git branch -c -f o/q o/p should work when o/p exists' '
+	git reflog exists refs/heads/o/q &&
+	git reflog exists refs/heads/o/p &&
+	git branch -m -f o/q o/p
+'
+
+test_expect_success 'git branch -c qq rr/qq should fail when r exists' '
+	git branch qq &&
+	git branch rr &&
+	test_must_fail git branch -c qq rr/qq
+'
+
+test_expect_success 'git branch -C b1 b2 should fail when b2 is checked out' '
+	git branch b1 &&
+	git checkout -b b2 &&
+	test_must_fail git branch -C b1 b2
+'
+
+test_expect_success 'git branch -C c1 c2 should succeed when c1 is checked out' '
+	git checkout -b c1 &&
+	git branch c2 &&
+	git branch -C c1 c2 &&
+	test $(git rev-parse --abbrev-ref HEAD) = c2
+'
+
+test_expect_success 'git branch -C c1 c2 should add entries to .git/logs/HEAD' '
+	msg="Branch: copied refs/heads/c1 to refs/heads/c2" &&
+	grep "$msg$" .git/logs/HEAD
+'
+
+test_expect_success 'git branch -C master should work when master is checked out' '
+	git checkout master &&
+	git branch -C master
+'
+
+test_expect_success 'git branch -C master master should work when master is checked out' '
+	git checkout master &&
+	git branch -C master master
+'
+
+test_expect_success 'git branch -C master5 master5 should work when master is checked out' '
+	git checkout master &&
+	git branch master5 &&
+	git branch -C master5 master5
+'
+
 test_expect_success 'deleting a symref' '
 	git branch target &&
 	git symbolic-ref refs/heads/symref refs/heads/target &&

--
https://github.com/git/git/pull/363

[PATCH/RFC v4 2/3] config: abstract out create section from key logic

[Sahil Dua]

Abstracts out the logic for creating string buffer from given key for
example - 'branch.b' and returns '[branch "b"]'.

We want to keep the original config section intact in case of copy
operation. For this we need to fetch the section with updated new branch
name so that we can write that to the config file.

For example - git branch -c foo bar
The mentioned/edited function renames and overwrites this part in the
config - [branch "foo"] to [branch "bar"]. However, in case of copy, we
want to keep the original [branch "foo"] intact and get [branch "bar"]
from "branch.bar" key. 'store_create_section' function will return
[branch "bar"] when "branch.bar" is passed.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 config.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 146cb3452adab..721aef1938081 100644
--- a/config.c
+++ b/config.c
@@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
 	return 4;
 }
 
-static int store_write_section(int fd, const char *key)
+struct strbuf store_create_section(const char *key)
 {
 	const char *dot;
-	int i, success;
+	int i;
 	struct strbuf sb = STRBUF_INIT;
 
 	dot = memchr(key, '.', store.baselen);
@@ -2188,6 +2188,16 @@ static int store_write_section(int fd, const char *key)
 		strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
 	}
 
+	return sb;
+}
+
+static int store_write_section(int fd, const char *key)
+{
+	int success;
+
+	/* Create a section with the given key */
+	struct strbuf sb = store_create_section(key);
+
 	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
 	strbuf_release(&sb);
 

--
https://github.com/git/git/pull/363

[PATCH/RFC v4 3/3] branch: add copy branch feature implementation

[Sahil Dua]

Adds copy branch option available using -c or -C (forcefully).

Adds new function copy_existing_ref to copy the ref and
git_config_copy_section to copy the git config section. Under the hood,
both git_config_copy_section and git_config_rename_section functions use
the same function which takes a flag copy? to decide whether it should
copy or move the config section.

Similarly for copy_existing_ref - rename_ref and copy_existing_ref use
same function which takes the copy? flag and work in the same way as
mentioned above.

The new function is named copy_existing_ref instead of copy_ref because
of conflicts with another function copy_ref from remote.h.

The new copied branch logs start with a line mentioning the copy
operation.

git config copy section logic:
While we're parsing the config file, we need to make sure we start
copying the config section once we find the matching block for our
branch1 (for example when running 'git branch -c branch1 branch2').

There is one flag used - 'copying_section' which can take 0/1/2 values.
0 - not copying currently
1 - just started copying section
2 - currently copying
I thought of making this flag binary to keep things easier. However,
since there was distinction in behavior(adding to currently copied
section) depending upon whether it's the first line of config section or
not.

The copied section has first line which contains the new branch name
(branch2 in our example). This is achieved using store_create_section
method.

Once we're done with reading the entire config file, we write our copied
section. Hence, literally copying the config section from branch1 to
branch2.

However, there's one case which is not handled by this yet - when
branch2 already has some configuration and -C command is used, operation
should delete the present configuration for branch2.

Also, it doesn't make sense to trigger config section copy config
section function if both the branch names are same.

For example - git branch -C a a
In such a case, it shouldn't try to copy or rename the git config
section.

fixups by avar:

-   fixup: use git comment style, and not C++ comments...
-   fixup bad i18n usage
    This interpolation of English words with sprintf resulted in
    untranslatable strings.
-   fixup mixed code/decl warning

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 builtin/branch.c     | 66 ++++++++++++++++++++++++++++----------
 cache.h              |  2 ++
 config.c             | 89 +++++++++++++++++++++++++++++++++++++++++-----------
 refs.c               | 11 +++++++
 refs.h               |  9 +++++-
 refs/files-backend.c | 46 ++++++++++++++++++++++-----
 refs/refs-internal.h |  4 +++
 7 files changed, 184 insertions(+), 43 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 83fcda43dceec..8ae87b1d6e68e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-l] [-f] <branch-name> [<start-point>]"),
 	N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
 	N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
+	N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
 	N_("git branch [<options>] [-r | -a] [--points-at]"),
 	N_("git branch [<options>] [-r | -a] [--format]"),
 	NULL
@@ -449,15 +450,19 @@ static void reject_rebase_or_bisect_branch(const char *target)
 	free_worktrees(worktrees);
 }
 
-static void rename_branch(const char *oldname, const char *newname, int force)
+static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)
 {
 	struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
 	struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
 	int recovery = 0;
 	int clobber_head_ok;
 
-	if (!oldname)
-		die(_("cannot rename the current branch while not on any."));
+	if (!oldname) {
+		if (copy)
+			die(_("cannot copy the current branch while not on any."));
+		else
+			die(_("cannot rename the current branch while not on any."));
+	}
 
 	if (strbuf_check_branch_ref(&oldref, oldname)) {
 		/*
@@ -480,17 +485,33 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 
 	reject_rebase_or_bisect_branch(oldref.buf);
 
-	strbuf_addf(&logmsg, "Branch: renamed %s to %s",
-		 oldref.buf, newref.buf);
+	if (copy)
+		strbuf_addf(&logmsg, "Branch: copied %s to %s",
+			    oldref.buf, newref.buf);
+	else
+		strbuf_addf(&logmsg, "Branch: renamed %s to %s",
+			    oldref.buf, newref.buf);
 
-	if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
+	if (!copy && rename_ref(oldref.buf, newref.buf, logmsg.buf))
 		die(_("Branch rename failed"));
+	if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
+		die(_("Branch copy failed"));
 
-	if (recovery)
-		warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);
+	if (recovery) {
+		if (copy)
+			warning(_("Copied a misnamed branch '%s' away"),
+				oldref.buf + 11);
+		else
+			warning(_("Renamed a misnamed branch '%s' away"),
+				oldref.buf + 11);
+	}
 
-	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))
-		die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf)) {
+		if (copy)
+			die(_("Branch copied to %s, but HEAD is not updated!"), newname);
+		else
+			die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+	}
 
 	strbuf_release(&logmsg);
 
@@ -498,8 +519,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	strbuf_release(&oldref);
 	strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
 	strbuf_release(&newref);
-	if (git_config_rename_section(oldsection.buf, newsection.buf) < 0)
+	if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
 		die(_("Branch is renamed, but update of config-file failed"));
+	if (copy && strcmp(oldname, newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
+		die(_("Branch is copied, but update of config-file failed"));
 	strbuf_release(&oldsection);
 	strbuf_release(&newsection);
 }
@@ -537,7 +560,7 @@ static int edit_branch_description(const char *branch_name)
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-	int delete = 0, rename = 0, force = 0, list = 0;
+	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
@@ -574,6 +597,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
 		OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
 		OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
+		OPT_BIT('c', NULL, &copy, N_("copy a branch and its reflog"), 1),
+		OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
 		OPT_BOOL(0, "list", &list, N_("list branch names")),
 		OPT_BOOL('l', "create-reflog", &reflog, N_("create the branch's reflog")),
 		OPT_BOOL(0, "edit-description", &edit_description,
@@ -617,14 +642,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
+	if (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
 	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
 	    filter.no_commit)
 		list = 1;
 
-	if (!!delete + !!rename + !!new_upstream +
+	if (!!delete + !!rename + !!copy + !!new_upstream +
 	    list + unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
@@ -696,13 +721,22 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (edit_branch_description(branch_name))
 			return 1;
+	} else if (copy) {
+		if (!argc)
+			die(_("branch name required"));
+		else if (argc == 1)
+			copy_or_rename_branch(head, argv[0], 1, copy > 1);
+		else if (argc == 2)
+			copy_or_rename_branch(argv[0], argv[1], 1, copy > 1);
+		else
+			die(_("too many branches for a copy operation"));
 	} else if (rename) {
 		if (!argc)
 			die(_("branch name required"));
 		else if (argc == 1)
-			rename_branch(head, argv[0], rename > 1);
+			copy_or_rename_branch(head, argv[0], 0, rename > 1);
 		else if (argc == 2)
-			rename_branch(argv[0], argv[1], rename > 1);
+			copy_or_rename_branch(argv[0], argv[1], 0, rename > 1);
 		else
 			die(_("too many branches for a rename operation"));
 	} else if (new_upstream) {
diff --git a/cache.h b/cache.h
index ae4c45d379d5b..109d91cb5248a 100644
--- a/cache.h
+++ b/cache.h
@@ -1935,6 +1935,8 @@ extern int git_config_set_multivar_in_file_gently(const char *, const char *, co
 extern void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
 extern int git_config_rename_section_in_file(const char *, const char *, const char *);
+extern int git_config_copy_section(const char *, const char *);
+extern int git_config_copy_section_in_file(const char *, const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
diff --git a/config.c b/config.c
index 721aef1938081..53b154b2cfcb1 100644
--- a/config.c
+++ b/config.c
@@ -2639,16 +2639,18 @@ static int section_name_is_ok(const char *name)
 }
 
 /* if new_name == NULL, the section is removed instead */
-int git_config_rename_section_in_file(const char *config_filename,
-				      const char *old_name, const char *new_name)
+static int git_config_copy_or_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name, int copy)
 {
-	int ret = 0, remove = 0;
+	int ret = 0, remove = 0, copying_section = 0, copied_section_length;
 	char *filename_buf = NULL;
 	struct lock_file *lock;
 	int out_fd;
 	char buf[1024];
 	FILE *config_file = NULL;
 	struct stat st;
+	struct strbuf copied_section = STRBUF_INIT;
+	int offset;
 
 	if (new_name && !section_name_is_ok(new_name)) {
 		ret = error("invalid section name: %s", new_name);
@@ -2689,33 +2691,51 @@ int git_config_rename_section_in_file(const char *config_filename,
 			; /* do nothing */
 		if (buf[i] == '[') {
 			/* it's a section */
-			int offset = section_name_match(&buf[i], old_name);
+			if (copying_section) {
+				/*
+				 * Mark the end of copying the matching
+				 * section, as this is the beginning
+				 * of the new section
+				 */
+				copying_section = 0;
+			}
+			offset = section_name_match(&buf[i], old_name);
 			if (offset > 0) {
 				ret++;
 				if (new_name == NULL) {
 					remove = 1;
 					continue;
 				}
-				store.baselen = strlen(new_name);
-				if (!store_write_section(out_fd, new_name)) {
-					ret = write_error(get_lock_file_path(lock));
-					goto out;
+				if (!copy) {
+					store.baselen = strlen(new_name);
+					if (!store_write_section(out_fd, new_name)) {
+						ret = write_error(get_lock_file_path(lock));
+						goto out;
+					}
+				} else {
+					/* Mark the beginning of copying the matching section */
+					copying_section = 1;
+					/* Create a section with new branch name */
+					store.baselen = strlen(new_name);
+					copied_section = store_create_section(new_name);
 				}
 				/*
 				 * We wrote out the new section, with
 				 * a newline, now skip the old
 				 * section's length
 				 */
-				output += offset + i;
-				if (strlen(output) > 0) {
-					/*
-					 * More content means there's
-					 * a declaration to put on the
-					 * next line; indent with a
-					 * tab
-					 */
-					output -= 1;
-					output[0] = '\t';
+				if (!copy) {
+					output += offset + i;
+					if (strlen(output) > 0) {
+						/*
+						 * More content means there's
+						 * a declaration to put on the
+						 * next line; indent with a
+						 * tab
+						 */
+						output -= 1;
+						output[0] = '\t';
+					}
 				}
 			}
 			remove = 0;
@@ -2723,11 +2743,25 @@ int git_config_rename_section_in_file(const char *config_filename,
 		if (remove)
 			continue;
 		length = strlen(output);
+
+		if (copying_section > 1) {
+			strbuf_addf(&copied_section, "%s", output);
+		} else if (copying_section == 1) {
+			copying_section = 2;
+		}
 		if (write_in_full(out_fd, output, length) != length) {
 			ret = write_error(get_lock_file_path(lock));
 			goto out;
 		}
 	}
+
+	if (copy && copied_section.len > 0) {
+		copied_section_length = strlen(copied_section.buf);
+		if (write_in_full(out_fd, copied_section.buf, copied_section_length) != copied_section_length) {
+			ret = write_error(get_lock_file_path(lock));
+			goto out;
+		}
+	}
 	fclose(config_file);
 	config_file = NULL;
 commit_and_out:
@@ -2743,11 +2777,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 	return ret;
 }
 
+int git_config_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
+{
+	return git_config_copy_or_rename_section_in_file(config_filename,
+					 old_name, new_name, 0);
+}
+
 int git_config_rename_section(const char *old_name, const char *new_name)
 {
 	return git_config_rename_section_in_file(NULL, old_name, new_name);
 }
 
+int git_config_copy_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
+{
+	return git_config_copy_or_rename_section_in_file(config_filename,
+					 old_name, new_name, 1);
+}
+
+int git_config_copy_section(const char *old_name, const char *new_name)
+{
+	return git_config_copy_section_in_file(NULL, old_name, new_name);
+}
+
 /*
  * Call this to report error for your variable that should not
  * get a boolean value (i.e. "[my] var" means "true").
diff --git a/refs.c b/refs.c
index 8af9641aa17e6..42b1abef76b1a 100644
--- a/refs.c
+++ b/refs.c
@@ -1917,3 +1917,14 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
 	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
 }
+
+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg)
+{
+	return refs->be->copy_ref(refs, oldref, newref, logmsg);
+}
+
+int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
+{
+	return refs_copy_existing_ref(get_main_ref_store(), oldref, newref, logmsg);
+}
diff --git a/refs.h b/refs.h
index 685a979a0eb70..df93d5a261b6c 100644
--- a/refs.h
+++ b/refs.h
@@ -396,7 +396,14 @@ char *shorten_unambiguous_ref(const char *refname, int strict);
 /** rename ref, return 0 on success **/
 int refs_rename_ref(struct ref_store *refs, const char *oldref,
 		    const char *newref, const char *logmsg);
-int rename_ref(const char *oldref, const char *newref, const char *logmsg);
+int rename_ref(const char *oldref, const char *newref,
+			const char *logmsg);
+
+/** copy ref, return 0 on success **/
+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg);
+int copy_existing_ref(const char *oldref, const char *newref,
+			const char *logmsg);
 
 int refs_create_symref(struct ref_store *refs, const char *refname,
 		       const char *target, const char *logmsg);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index cb1f528cbeec4..82d6692584c9c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1703,9 +1703,9 @@ static int commit_ref_update(struct files_ref_store *refs,
 			     const struct object_id *oid, const char *logmsg,
 			     struct strbuf *err);
 
-static int files_rename_ref(struct ref_store *ref_store,
+static int files_copy_or_rename_ref(struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
-			    const char *logmsg)
+			    const char *logmsg, int copy)
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
@@ -1737,8 +1737,12 @@ static int files_rename_ref(struct ref_store *ref_store,
 	}
 
 	if (flag & REF_ISSYMREF) {
-		ret = error("refname %s is a symbolic ref, renaming it is not supported",
-			    oldrefname);
+		if (copy)
+			ret = error("refname %s is a symbolic ref, copying it is not supported",
+				    oldrefname);
+		else
+			ret = error("refname %s is a symbolic ref, renaming it is not supported",
+				    oldrefname);
 		goto out;
 	}
 	if (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {
@@ -1746,13 +1750,19 @@ static int files_rename_ref(struct ref_store *ref_store,
 		goto out;
 	}
 
-	if (log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
+	if (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
 		ret = error("unable to move logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
 			    oldrefname, strerror(errno));
 		goto out;
 	}
 
-	if (refs_delete_ref(&refs->base, logmsg, oldrefname,
+	if (copy && log && copy_file(tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
+		ret = error("unable to copy logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
+			    oldrefname, strerror(errno));
+		goto out;
+	}
+
+	if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,
 			    orig_oid.hash, REF_NODEREF)) {
 		error("unable to delete old %s", oldrefname);
 		goto rollback;
@@ -1765,7 +1775,7 @@ static int files_rename_ref(struct ref_store *ref_store,
 	 * the safety anyway; we want to delete the reference whatever
 	 * its current value.
 	 */
-	if (!refs_read_ref_full(&refs->base, newrefname,
+	if (!copy && !refs_read_ref_full(&refs->base, newrefname,
 				RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
 				oid.hash, NULL) &&
 	    refs_delete_ref(&refs->base, NULL, newrefname,
@@ -1796,7 +1806,10 @@ static int files_rename_ref(struct ref_store *ref_store,
 	lock = lock_ref_sha1_basic(refs, newrefname, NULL, NULL, NULL,
 				   REF_NODEREF, NULL, &err);
 	if (!lock) {
-		error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
+		if (copy)
+			error("unable to copy '%s' to '%s': %s", oldrefname, newrefname, err.buf);
+		else
+			error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
 		strbuf_release(&err);
 		goto rollback;
 	}
@@ -1847,6 +1860,22 @@ static int files_rename_ref(struct ref_store *ref_store,
 	return ret;
 }
 
+static int files_rename_ref(struct ref_store *ref_store,
+			    const char *oldrefname, const char *newrefname,
+			    const char *logmsg)
+{
+	return files_copy_or_rename_ref(ref_store, oldrefname,
+				 newrefname, logmsg, 0);
+}
+
+static int files_copy_ref(struct ref_store *ref_store,
+			    const char *oldrefname, const char *newrefname,
+			    const char *logmsg)
+{
+	return files_copy_or_rename_ref(ref_store, oldrefname,
+				 newrefname, logmsg, 1);
+}
+
 static int close_ref(struct ref_lock *lock)
 {
 	if (close_lock_file(lock->lk))
@@ -3319,6 +3348,7 @@ struct ref_storage_be refs_be_files = {
 	files_create_symref,
 	files_delete_refs,
 	files_rename_ref,
+	files_copy_ref,
 
 	files_ref_iterator_begin,
 	files_read_raw_ref,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index b6b291cf00e5c..823a1c48af6ec 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -513,6 +513,9 @@ typedef int delete_refs_fn(struct ref_store *ref_store,
 typedef int rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
+typedef int copy_ref_fn(struct ref_store *ref_store,
+			  const char *oldref, const char *newref,
+			  const char *logmsg);
 
 /*
  * Iterate over the references in the specified ref_store that are
@@ -607,6 +610,7 @@ struct ref_storage_be {
 	create_symref_fn *create_symref;
 	delete_refs_fn *delete_refs;
 	rename_ref_fn *rename_ref;
+	copy_ref_fn *copy_ref;
 
 	ref_iterator_begin_fn *iterator_begin;
 	read_raw_ref_fn *read_raw_ref;

--
https://github.com/git/git/pull/363

Re: [PATCH/RFC v4 3/3] branch: add copy branch feature implementation

[Sahil Dua]

I want suggestions about one logical point raised by Evar.

Let's consider a case that I'm on branch maint and then I do 'git
checkout master' followed by 'git branch -m feature', it will rename
master branch to feature. Now if I do 'git checkout -' to go to the
last branch, it will take me to maint since master branch doesn't
exist in this case.

Now, for this copy operation - if I'm on branch maint and then I do
'git checkout master' followed by 'git branch -c feature', it will
copy master branch to feature. Now if I do 'git checkout -' to go to
the last branch, it will again go to maint (according to the current
implementation). What do you think it should do? Is this the desired
behavior? Or should it go to master branch since that was the branch
checked out before copying.

Also, in case this needs to be changed, can someone please point me to
how it's being handled so that I can change the behavior.

On Mon, Jun 5, 2017 at 10:40 PM, Sahil Dua <sahildua2305@gmail.com> wrote:
> Adds copy branch option available using -c or -C (forcefully).
>
> Adds new function copy_existing_ref to copy the ref and
> git_config_copy_section to copy the git config section. Under the hood,
> both git_config_copy_section and git_config_rename_section functions use
> the same function which takes a flag copy? to decide whether it should
> copy or move the config section.
>
> Similarly for copy_existing_ref - rename_ref and copy_existing_ref use
> same function which takes the copy? flag and work in the same way as
> mentioned above.
>
> The new function is named copy_existing_ref instead of copy_ref because
> of conflicts with another function copy_ref from remote.h.
>
> The new copied branch logs start with a line mentioning the copy
> operation.
>
> git config copy section logic:
> While we're parsing the config file, we need to make sure we start
> copying the config section once we find the matching block for our
> branch1 (for example when running 'git branch -c branch1 branch2').
>
> There is one flag used - 'copying_section' which can take 0/1/2 values.
> 0 - not copying currently
> 1 - just started copying section
> 2 - currently copying
> I thought of making this flag binary to keep things easier. However,
> since there was distinction in behavior(adding to currently copied
> section) depending upon whether it's the first line of config section or
> not.
>
> The copied section has first line which contains the new branch name
> (branch2 in our example). This is achieved using store_create_section
> method.
>
> Once we're done with reading the entire config file, we write our copied
> section. Hence, literally copying the config section from branch1 to
> branch2.
>
> However, there's one case which is not handled by this yet - when
> branch2 already has some configuration and -C command is used, operation
> should delete the present configuration for branch2.
>
> Also, it doesn't make sense to trigger config section copy config
> section function if both the branch names are same.
>
> For example - git branch -C a a
> In such a case, it shouldn't try to copy or rename the git config
> section.
>
> fixups by avar:
>
> -   fixup: use git comment style, and not C++ comments...
> -   fixup bad i18n usage
>     This interpolation of English words with sprintf resulted in
>     untranslatable strings.
> -   fixup mixed code/decl warning
>
> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
> ---
>  builtin/branch.c     | 66 ++++++++++++++++++++++++++++----------
>  cache.h              |  2 ++
>  config.c             | 89 +++++++++++++++++++++++++++++++++++++++++-----------
>  refs.c               | 11 +++++++
>  refs.h               |  9 +++++-
>  refs/files-backend.c | 46 ++++++++++++++++++++++-----
>  refs/refs-internal.h |  4 +++
>  7 files changed, 184 insertions(+), 43 deletions(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 83fcda43dceec..8ae87b1d6e68e 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
>         N_("git branch [<options>] [-l] [-f] <branch-name> [<start-point>]"),
>         N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
>         N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
> +       N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
>         N_("git branch [<options>] [-r | -a] [--points-at]"),
>         N_("git branch [<options>] [-r | -a] [--format]"),
>         NULL
> @@ -449,15 +450,19 @@ static void reject_rebase_or_bisect_branch(const char *target)
>         free_worktrees(worktrees);
>  }
>
> -static void rename_branch(const char *oldname, const char *newname, int force)
> +static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)
>  {
>         struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
>         struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
>         int recovery = 0;
>         int clobber_head_ok;
>
> -       if (!oldname)
> -               die(_("cannot rename the current branch while not on any."));
> +       if (!oldname) {
> +               if (copy)
> +                       die(_("cannot copy the current branch while not on any."));
> +               else
> +                       die(_("cannot rename the current branch while not on any."));
> +       }
>
>         if (strbuf_check_branch_ref(&oldref, oldname)) {
>                 /*
> @@ -480,17 +485,33 @@ static void rename_branch(const char *oldname, const char *newname, int force)
>
>         reject_rebase_or_bisect_branch(oldref.buf);
>
> -       strbuf_addf(&logmsg, "Branch: renamed %s to %s",
> -                oldref.buf, newref.buf);
> +       if (copy)
> +               strbuf_addf(&logmsg, "Branch: copied %s to %s",
> +                           oldref.buf, newref.buf);
> +       else
> +               strbuf_addf(&logmsg, "Branch: renamed %s to %s",
> +                           oldref.buf, newref.buf);
>
> -       if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
> +       if (!copy && rename_ref(oldref.buf, newref.buf, logmsg.buf))
>                 die(_("Branch rename failed"));
> +       if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
> +               die(_("Branch copy failed"));
>
> -       if (recovery)
> -               warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);
> +       if (recovery) {
> +               if (copy)
> +                       warning(_("Copied a misnamed branch '%s' away"),
> +                               oldref.buf + 11);
> +               else
> +                       warning(_("Renamed a misnamed branch '%s' away"),
> +                               oldref.buf + 11);
> +       }
>
> -       if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))
> -               die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
> +       if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf)) {
> +               if (copy)
> +                       die(_("Branch copied to %s, but HEAD is not updated!"), newname);
> +               else
> +                       die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
> +       }
>
>         strbuf_release(&logmsg);
>
> @@ -498,8 +519,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)
>         strbuf_release(&oldref);
>         strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
>         strbuf_release(&newref);
> -       if (git_config_rename_section(oldsection.buf, newsection.buf) < 0)
> +       if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
>                 die(_("Branch is renamed, but update of config-file failed"));
> +       if (copy && strcmp(oldname, newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
> +               die(_("Branch is copied, but update of config-file failed"));
>         strbuf_release(&oldsection);
>         strbuf_release(&newsection);
>  }
> @@ -537,7 +560,7 @@ static int edit_branch_description(const char *branch_name)
>
>  int cmd_branch(int argc, const char **argv, const char *prefix)
>  {
> -       int delete = 0, rename = 0, force = 0, list = 0;
> +       int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
>         int reflog = 0, edit_description = 0;
>         int quiet = 0, unset_upstream = 0;
>         const char *new_upstream = NULL;
> @@ -574,6 +597,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>                 OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
>                 OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
>                 OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
> +               OPT_BIT('c', NULL, &copy, N_("copy a branch and its reflog"), 1),
> +               OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
>                 OPT_BOOL(0, "list", &list, N_("list branch names")),
>                 OPT_BOOL('l', "create-reflog", &reflog, N_("create the branch's reflog")),
>                 OPT_BOOL(0, "edit-description", &edit_description,
> @@ -617,14 +642,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>         argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
>                              0);
>
> -       if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
> +       if (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)
>                 list = 1;
>
>         if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
>             filter.no_commit)
>                 list = 1;
>
> -       if (!!delete + !!rename + !!new_upstream +
> +       if (!!delete + !!rename + !!copy + !!new_upstream +
>             list + unset_upstream > 1)
>                 usage_with_options(builtin_branch_usage, options);
>
> @@ -696,13 +721,22 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>
>                 if (edit_branch_description(branch_name))
>                         return 1;
> +       } else if (copy) {
> +               if (!argc)
> +                       die(_("branch name required"));
> +               else if (argc == 1)
> +                       copy_or_rename_branch(head, argv[0], 1, copy > 1);
> +               else if (argc == 2)
> +                       copy_or_rename_branch(argv[0], argv[1], 1, copy > 1);
> +               else
> +                       die(_("too many branches for a copy operation"));
>         } else if (rename) {
>                 if (!argc)
>                         die(_("branch name required"));
>                 else if (argc == 1)
> -                       rename_branch(head, argv[0], rename > 1);
> +                       copy_or_rename_branch(head, argv[0], 0, rename > 1);
>                 else if (argc == 2)
> -                       rename_branch(argv[0], argv[1], rename > 1);
> +                       copy_or_rename_branch(argv[0], argv[1], 0, rename > 1);
>                 else
>                         die(_("too many branches for a rename operation"));
>         } else if (new_upstream) {
> diff --git a/cache.h b/cache.h
> index ae4c45d379d5b..109d91cb5248a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1935,6 +1935,8 @@ extern int git_config_set_multivar_in_file_gently(const char *, const char *, co
>  extern void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
>  extern int git_config_rename_section(const char *, const char *);
>  extern int git_config_rename_section_in_file(const char *, const char *, const char *);
> +extern int git_config_copy_section(const char *, const char *);
> +extern int git_config_copy_section_in_file(const char *, const char *, const char *);
>  extern const char *git_etc_gitconfig(void);
>  extern int git_env_bool(const char *, int);
>  extern unsigned long git_env_ulong(const char *, unsigned long);
> diff --git a/config.c b/config.c
> index 721aef1938081..53b154b2cfcb1 100644
> --- a/config.c
> +++ b/config.c
> @@ -2639,16 +2639,18 @@ static int section_name_is_ok(const char *name)
>  }
>
>  /* if new_name == NULL, the section is removed instead */
> -int git_config_rename_section_in_file(const char *config_filename,
> -                                     const char *old_name, const char *new_name)
> +static int git_config_copy_or_rename_section_in_file(const char *config_filename,
> +                                     const char *old_name, const char *new_name, int copy)
>  {
> -       int ret = 0, remove = 0;
> +       int ret = 0, remove = 0, copying_section = 0, copied_section_length;
>         char *filename_buf = NULL;
>         struct lock_file *lock;
>         int out_fd;
>         char buf[1024];
>         FILE *config_file = NULL;
>         struct stat st;
> +       struct strbuf copied_section = STRBUF_INIT;
> +       int offset;
>
>         if (new_name && !section_name_is_ok(new_name)) {
>                 ret = error("invalid section name: %s", new_name);
> @@ -2689,33 +2691,51 @@ int git_config_rename_section_in_file(const char *config_filename,
>                         ; /* do nothing */
>                 if (buf[i] == '[') {
>                         /* it's a section */
> -                       int offset = section_name_match(&buf[i], old_name);
> +                       if (copying_section) {
> +                               /*
> +                                * Mark the end of copying the matching
> +                                * section, as this is the beginning
> +                                * of the new section
> +                                */
> +                               copying_section = 0;
> +                       }
> +                       offset = section_name_match(&buf[i], old_name);
>                         if (offset > 0) {
>                                 ret++;
>                                 if (new_name == NULL) {
>                                         remove = 1;
>                                         continue;
>                                 }
> -                               store.baselen = strlen(new_name);
> -                               if (!store_write_section(out_fd, new_name)) {
> -                                       ret = write_error(get_lock_file_path(lock));
> -                                       goto out;
> +                               if (!copy) {
> +                                       store.baselen = strlen(new_name);
> +                                       if (!store_write_section(out_fd, new_name)) {
> +                                               ret = write_error(get_lock_file_path(lock));
> +                                               goto out;
> +                                       }
> +                               } else {
> +                                       /* Mark the beginning of copying the matching section */
> +                                       copying_section = 1;
> +                                       /* Create a section with new branch name */
> +                                       store.baselen = strlen(new_name);
> +                                       copied_section = store_create_section(new_name);
>                                 }
>                                 /*
>                                  * We wrote out the new section, with
>                                  * a newline, now skip the old
>                                  * section's length
>                                  */
> -                               output += offset + i;
> -                               if (strlen(output) > 0) {
> -                                       /*
> -                                        * More content means there's
> -                                        * a declaration to put on the
> -                                        * next line; indent with a
> -                                        * tab
> -                                        */
> -                                       output -= 1;
> -                                       output[0] = '\t';
> +                               if (!copy) {
> +                                       output += offset + i;
> +                                       if (strlen(output) > 0) {
> +                                               /*
> +                                                * More content means there's
> +                                                * a declaration to put on the
> +                                                * next line; indent with a
> +                                                * tab
> +                                                */
> +                                               output -= 1;
> +                                               output[0] = '\t';
> +                                       }
>                                 }
>                         }
>                         remove = 0;
> @@ -2723,11 +2743,25 @@ int git_config_rename_section_in_file(const char *config_filename,
>                 if (remove)
>                         continue;
>                 length = strlen(output);
> +
> +               if (copying_section > 1) {
> +                       strbuf_addf(&copied_section, "%s", output);
> +               } else if (copying_section == 1) {
> +                       copying_section = 2;
> +               }
>                 if (write_in_full(out_fd, output, length) != length) {
>                         ret = write_error(get_lock_file_path(lock));
>                         goto out;
>                 }
>         }
> +
> +       if (copy && copied_section.len > 0) {
> +               copied_section_length = strlen(copied_section.buf);
> +               if (write_in_full(out_fd, copied_section.buf, copied_section_length) != copied_section_length) {
> +                       ret = write_error(get_lock_file_path(lock));
> +                       goto out;
> +               }
> +       }
>         fclose(config_file);
>         config_file = NULL;
>  commit_and_out:
> @@ -2743,11 +2777,30 @@ int git_config_rename_section_in_file(const char *config_filename,
>         return ret;
>  }
>
> +int git_config_rename_section_in_file(const char *config_filename,
> +                                     const char *old_name, const char *new_name)
> +{
> +       return git_config_copy_or_rename_section_in_file(config_filename,
> +                                        old_name, new_name, 0);
> +}
> +
>  int git_config_rename_section(const char *old_name, const char *new_name)
>  {
>         return git_config_rename_section_in_file(NULL, old_name, new_name);
>  }
>
> +int git_config_copy_section_in_file(const char *config_filename,
> +                                     const char *old_name, const char *new_name)
> +{
> +       return git_config_copy_or_rename_section_in_file(config_filename,
> +                                        old_name, new_name, 1);
> +}
> +
> +int git_config_copy_section(const char *old_name, const char *new_name)
> +{
> +       return git_config_copy_section_in_file(NULL, old_name, new_name);
> +}
> +
>  /*
>   * Call this to report error for your variable that should not
>   * get a boolean value (i.e. "[my] var" means "true").
> diff --git a/refs.c b/refs.c
> index 8af9641aa17e6..42b1abef76b1a 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1917,3 +1917,14 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
>  {
>         return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
>  }
> +
> +int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
> +                   const char *newref, const char *logmsg)
> +{
> +       return refs->be->copy_ref(refs, oldref, newref, logmsg);
> +}
> +
> +int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
> +{
> +       return refs_copy_existing_ref(get_main_ref_store(), oldref, newref, logmsg);
> +}
> diff --git a/refs.h b/refs.h
> index 685a979a0eb70..df93d5a261b6c 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -396,7 +396,14 @@ char *shorten_unambiguous_ref(const char *refname, int strict);
>  /** rename ref, return 0 on success **/
>  int refs_rename_ref(struct ref_store *refs, const char *oldref,
>                     const char *newref, const char *logmsg);
> -int rename_ref(const char *oldref, const char *newref, const char *logmsg);
> +int rename_ref(const char *oldref, const char *newref,
> +                       const char *logmsg);
> +
> +/** copy ref, return 0 on success **/
> +int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
> +                   const char *newref, const char *logmsg);
> +int copy_existing_ref(const char *oldref, const char *newref,
> +                       const char *logmsg);
>
>  int refs_create_symref(struct ref_store *refs, const char *refname,
>                        const char *target, const char *logmsg);
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index cb1f528cbeec4..82d6692584c9c 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1703,9 +1703,9 @@ static int commit_ref_update(struct files_ref_store *refs,
>                              const struct object_id *oid, const char *logmsg,
>                              struct strbuf *err);
>
> -static int files_rename_ref(struct ref_store *ref_store,
> +static int files_copy_or_rename_ref(struct ref_store *ref_store,
>                             const char *oldrefname, const char *newrefname,
> -                           const char *logmsg)
> +                           const char *logmsg, int copy)
>  {
>         struct files_ref_store *refs =
>                 files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
> @@ -1737,8 +1737,12 @@ static int files_rename_ref(struct ref_store *ref_store,
>         }
>
>         if (flag & REF_ISSYMREF) {
> -               ret = error("refname %s is a symbolic ref, renaming it is not supported",
> -                           oldrefname);
> +               if (copy)
> +                       ret = error("refname %s is a symbolic ref, copying it is not supported",
> +                                   oldrefname);
> +               else
> +                       ret = error("refname %s is a symbolic ref, renaming it is not supported",
> +                                   oldrefname);
>                 goto out;
>         }
>         if (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {
> @@ -1746,13 +1750,19 @@ static int files_rename_ref(struct ref_store *ref_store,
>                 goto out;
>         }
>
> -       if (log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
> +       if (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
>                 ret = error("unable to move logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
>                             oldrefname, strerror(errno));
>                 goto out;
>         }
>
> -       if (refs_delete_ref(&refs->base, logmsg, oldrefname,
> +       if (copy && log && copy_file(tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
> +               ret = error("unable to copy logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
> +                           oldrefname, strerror(errno));
> +               goto out;
> +       }
> +
> +       if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,
>                             orig_oid.hash, REF_NODEREF)) {
>                 error("unable to delete old %s", oldrefname);
>                 goto rollback;
> @@ -1765,7 +1775,7 @@ static int files_rename_ref(struct ref_store *ref_store,
>          * the safety anyway; we want to delete the reference whatever
>          * its current value.
>          */
> -       if (!refs_read_ref_full(&refs->base, newrefname,
> +       if (!copy && !refs_read_ref_full(&refs->base, newrefname,
>                                 RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
>                                 oid.hash, NULL) &&
>             refs_delete_ref(&refs->base, NULL, newrefname,
> @@ -1796,7 +1806,10 @@ static int files_rename_ref(struct ref_store *ref_store,
>         lock = lock_ref_sha1_basic(refs, newrefname, NULL, NULL, NULL,
>                                    REF_NODEREF, NULL, &err);
>         if (!lock) {
> -               error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
> +               if (copy)
> +                       error("unable to copy '%s' to '%s': %s", oldrefname, newrefname, err.buf);
> +               else
> +                       error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
>                 strbuf_release(&err);
>                 goto rollback;
>         }
> @@ -1847,6 +1860,22 @@ static int files_rename_ref(struct ref_store *ref_store,
>         return ret;
>  }
>
> +static int files_rename_ref(struct ref_store *ref_store,
> +                           const char *oldrefname, const char *newrefname,
> +                           const char *logmsg)
> +{
> +       return files_copy_or_rename_ref(ref_store, oldrefname,
> +                                newrefname, logmsg, 0);
> +}
> +
> +static int files_copy_ref(struct ref_store *ref_store,
> +                           const char *oldrefname, const char *newrefname,
> +                           const char *logmsg)
> +{
> +       return files_copy_or_rename_ref(ref_store, oldrefname,
> +                                newrefname, logmsg, 1);
> +}
> +
>  static int close_ref(struct ref_lock *lock)
>  {
>         if (close_lock_file(lock->lk))
> @@ -3319,6 +3348,7 @@ struct ref_storage_be refs_be_files = {
>         files_create_symref,
>         files_delete_refs,
>         files_rename_ref,
> +       files_copy_ref,
>
>         files_ref_iterator_begin,
>         files_read_raw_ref,
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index b6b291cf00e5c..823a1c48af6ec 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -513,6 +513,9 @@ typedef int delete_refs_fn(struct ref_store *ref_store,
>  typedef int rename_ref_fn(struct ref_store *ref_store,
>                           const char *oldref, const char *newref,
>                           const char *logmsg);
> +typedef int copy_ref_fn(struct ref_store *ref_store,
> +                         const char *oldref, const char *newref,
> +                         const char *logmsg);
>
>  /*
>   * Iterate over the references in the specified ref_store that are
> @@ -607,6 +610,7 @@ struct ref_storage_be {
>         create_symref_fn *create_symref;
>         delete_refs_fn *delete_refs;
>         rename_ref_fn *rename_ref;
> +       copy_ref_fn *copy_ref;
>
>         ref_iterator_begin_fn *iterator_begin;
>         read_raw_ref_fn *read_raw_ref;
>
> --
> https://github.com/git/git/pull/363

Re: [PATCH/RFC v4 3/3] branch: add copy branch feature implementation

[Junio C Hamano]

Sahil Dua <sahildua2305@gmail.com> writes:

> I want suggestions about one logical point raised by Evar.
>
> Let's consider a case that I'm on branch maint and then I do 'git
> checkout master' followed by 'git branch -m feature', it will rename
> master branch to feature. Now if I do 'git checkout -' to go to the
> last branch, it will take me to maint since master branch doesn't
> exist in this case.
>
> Now, for this copy operation - if I'm on branch maint and then I do
> 'git checkout master' followed by 'git branch -c feature', it will
> copy master branch to feature. Now if I do 'git checkout -' to go to
> the last branch, it will again go to maint (according to the current
> implementation). What do you think it should do? Is this the desired
> behavior? Or should it go to master branch since that was the branch
> checked out before copying.
>
> Also, in case this needs to be changed, can someone please point me to
> how it's being handled so that I can change the behavior.

When somebody says "I want to rename my current branch to X", it is
clear that the person wants to end up being on a branch called X.

To me, "I want to copy my current branch to Y" sounds more like "I
want to create another Y that looks just like the current branch,
but I want stay on my current branch".

If you think copy makes @{-1} problematic, perhaps your copy is
doing more than it should (e.g. switching the current branch at the
same time, or something).

Re: [PATCH/RFC v4 3/3] branch: add copy branch feature implementation

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> When somebody says "I want to rename my current branch to X", it is
> clear that the person wants to end up being on a branch called X.
>
> To me, "I want to copy my current branch to Y" sounds more like "I
> want to create another Y that looks just like the current branch,
> but I want stay on my current branch".
>
> If you think copy makes @{-1} problematic, perhaps your copy is
> doing more than it should (e.g. switching the current branch at the
> same time, or something).

A related tangent.  Git does not assign any identity to a branch, so
"the branch we were previously on" goes strictly by name.  If you
are on branch X, switched to branch Y, and then renamed branch X
(which is not current) to branch Z, Git will still think that the
previous branch is X, so an attempt to switch back to @{-1} may do
unexpected things.

Re: [PATCH/RFC v4 3/3] branch: add copy branch feature implementation

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 6, 2017 at 2:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Sahil Dua <sahildua2305@gmail.com> writes:
>
>> I want suggestions about one logical point raised by Evar.
>>
>> Let's consider a case that I'm on branch maint and then I do 'git
>> checkout master' followed by 'git branch -m feature', it will rename
>> master branch to feature. Now if I do 'git checkout -' to go to the
>> last branch, it will take me to maint since master branch doesn't
>> exist in this case.
>>
>> Now, for this copy operation - if I'm on branch maint and then I do
>> 'git checkout master' followed by 'git branch -c feature', it will
>> copy master branch to feature. Now if I do 'git checkout -' to go to
>> the last branch, it will again go to maint (according to the current
>> implementation). What do you think it should do? Is this the desired
>> behavior? Or should it go to master branch since that was the branch
>> checked out before copying.
>>
>> Also, in case this needs to be changed, can someone please point me to
>> how it's being handled so that I can change the behavior.
>
> When somebody says "I want to rename my current branch to X", it is
> clear that the person wants to end up being on a branch called X.
>
> To me, "I want to copy my current branch to Y" sounds more like "I
> want to create another Y that looks just like the current branch,
> but I want stay on my current branch".

This would be more useful to me if the semantics were copy & checkout
instead of just copy, since when I'd like to copy branches it's almost
always because I'm on some topic branch and want to create & work on a
new copy of that topic branch.

It would also be consistent with "git branch -m" and easier to
explain, i.e. "git branch -c just like -m except it doesn't delete the
branch name/config you moved away from".

Like with -m, you can still move around random other branches, e.g.:

    # While on master
    $ git branch -m some-other new-some-other

This will just move some-other to new-some-other without checkout out
new-some-other, it's only when the source name is the same as your
currently checked out branch that you checkout a new branch,

Now, of course the difference is that when you -m your current branch
it doesn't really have a choice of whether to move your checkout as
well (although I suppose it could leave you in a detached HEAD..) so
it *could* be done differently with -c, but the current behavior makes
more sense to me and matches the common case I'd use it for.

> If you think copy makes @{-1} problematic, perhaps your copy is
> doing more than it should (e.g. switching the current branch at the
> same time, or something).

I think what Sahil is getting at is asking where the @{-N} info is
stored and why this isn't equivalent to:

    $ git checkout -b one master
    $ git checkout -b two master
    $ git checkout master
    $ git checkout one
    $ git checkout two
    $ git checkout - # Goes to "one", not "master"

Which is in analogous flow without this feature that switches to the
last branch, but with "git branch -c" if you were on 'one' and copied
& checked out 'two' doing 'git checkout -' would bring you back to
'master', not 'one'.

Re: [PATCH/RFC v4 3/3] branch: add copy branch feature implementation

[Sahil Dua]

On Tue, Jun 6, 2017 at 9:39 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Tue, Jun 6, 2017 at 2:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Sahil Dua <sahildua2305@gmail.com> writes:
>>
>>> I want suggestions about one logical point raised by Evar.
>>>
>>> Let's consider a case that I'm on branch maint and then I do 'git
>>> checkout master' followed by 'git branch -m feature', it will rename
>>> master branch to feature. Now if I do 'git checkout -' to go to the
>>> last branch, it will take me to maint since master branch doesn't
>>> exist in this case.
>>>
>>> Now, for this copy operation - if I'm on branch maint and then I do
>>> 'git checkout master' followed by 'git branch -c feature', it will
>>> copy master branch to feature. Now if I do 'git checkout -' to go to
>>> the last branch, it will again go to maint (according to the current
>>> implementation). What do you think it should do? Is this the desired
>>> behavior? Or should it go to master branch since that was the branch
>>> checked out before copying.
>>>
>>> Also, in case this needs to be changed, can someone please point me to
>>> how it's being handled so that I can change the behavior.
>>
>> When somebody says "I want to rename my current branch to X", it is
>> clear that the person wants to end up being on a branch called X.
>>
>> To me, "I want to copy my current branch to Y" sounds more like "I
>> want to create another Y that looks just like the current branch,
>> but I want stay on my current branch".
>
> This would be more useful to me if the semantics were copy & checkout
> instead of just copy, since when I'd like to copy branches it's almost
> always because I'm on some topic branch and want to create & work on a
> new copy of that topic branch.
>
> It would also be consistent with "git branch -m" and easier to
> explain, i.e. "git branch -c just like -m except it doesn't delete the
> branch name/config you moved away from".
>
> Like with -m, you can still move around random other branches, e.g.:
>
>     # While on master
>     $ git branch -m some-other new-some-other
>
> This will just move some-other to new-some-other without checkout out
> new-some-other, it's only when the source name is the same as your
> currently checked out branch that you checkout a new branch,
>
> Now, of course the difference is that when you -m your current branch
> it doesn't really have a choice of whether to move your checkout as
> well (although I suppose it could leave you in a detached HEAD..) so
> it *could* be done differently with -c, but the current behavior makes
> more sense to me and matches the common case I'd use it for.
>
>> If you think copy makes @{-1} problematic, perhaps your copy is
>> doing more than it should (e.g. switching the current branch at the
>> same time, or something).
>
> I think what Sahil is getting at is asking where the @{-N} info is
> stored and why this isn't equivalent to:
>
>     $ git checkout -b one master
>     $ git checkout -b two master
>     $ git checkout master
>     $ git checkout one
>     $ git checkout two
>     $ git checkout - # Goes to "one", not "master"
>
> Which is in analogous flow without this feature that switches to the
> last branch, but with "git branch -c" if you were on 'one' and copied
> & checked out 'two' doing 'git checkout -' would bring you back to
> 'master', not 'one'.

Yes, indeed. I am not sure how @{-N} info is stored and why it's not
doing the expected thing. Expected thing here will be to go to the
original branch from where you copied the new branch if you run 'git
checkout -'.

Re: [PATCH/RFC v4 3/3] branch: add copy branch feature implementation

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Tue, Jun 6, 2017 at 2:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> When somebody says "I want to rename my current branch to X", it is
>> clear that the person wants to end up being on a branch called X.
>>
>> To me, "I want to copy my current branch to Y" sounds more like "I
>> want to create another Y that looks just like the current branch,
>> but I want stay on my current branch".
>
> This would be more useful to me if the semantics were copy & checkout
> instead of just copy, since when I'd like to copy branches it's almost
> always because I'm on some topic branch and want to create & work on a
> new copy of that topic branch.
>
> It would also be consistent with "git branch -m" and easier to
> explain, i.e. "git branch -c just like -m except it doesn't delete the
> branch name/config you moved away from".

The way I would use this feature, I'd imagine, would be to stash
away the current state of the branch before I try to do something
potentially stupid, and then recover the original state by reverting
back, i.e.

    $ git branch -c master backup
    ... do silly things to the master branch ...
    ... ok, enough silliness. go back to sane state ...
    $ git branch -M backup master

Having said that, I think it is merely the matter of personal taste
and there is no single "right" definition.  I do not have a strong
feeling either way.

To play devil's advocate, I'd anticipate a criticism, if we take
your interpretation (which I do not _mind_, even though I do not
have strong feeling _for_ it), saying "I just told you to _copy_ it.
If I wanted to check it out, I'd do that myself. Why are you doing
more than I asked, stupid Git." 

I have to say upfront that against such a criticism, I won't defend
your position myself.

[PATCH 1/3] config: create a function to format section headers

[Sahil Dua]

Factor out the logic which creates section headers in the config file,
e.g. the 'branch.foo' key will be turned into '[branch "foo"]'.

This introduces no function changes, but is needed for a later change
which adds support for copying branch sections in the config file.

Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 config.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 146cb3452adab..d5bb69e925dac 100644
--- a/config.c
+++ b/config.c
@@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
 	return 4;
 }
 
-static int store_write_section(int fd, const char *key)
+struct strbuf store_create_section(const char *key)
 {
 	const char *dot;
-	int i, success;
+	int i;
 	struct strbuf sb = STRBUF_INIT;
 
 	dot = memchr(key, '.', store.baselen);
@@ -2188,6 +2188,15 @@ static int store_write_section(int fd, const char *key)
 		strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
 	}
 
+	return sb;
+}
+
+static int store_write_section(int fd, const char *key)
+{
+	int success;
+
+	struct strbuf sb = store_create_section(key);
+
 	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
 	strbuf_release(&sb);
 

--
https://github.com/git/git/pull/363

[PATCH 2/3] branch: add test for -m renaming multiple config sections

[Sahil Dua]

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Add a test for how 'git branch -m' handles the renaming of multiple
config sections existing for one branch.

The config format we use is hybrid machine/human editable, and we do
our best to preserve the likes of comments and formatting when editing
the file with git-config.

This adds a test for the currently expected semantics in the face of
some rather obscure edge cases which are unlikely to occur in
practice.

Helped-by: Sahil Dua <sahildua2305@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 t/t3200-branch.sh | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 10f8f026ffb4b..28c02ffeadb4f 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -341,6 +341,43 @@ test_expect_success 'config information was renamed, too' '
 	test_must_fail git config branch.s/s.dummy
 '
 
+test_expect_success 'git branch -m correctly renames multiple config sections' '
+	test_when_finished "git checkout master" &&
+	git checkout -b source master &&
+
+	# Assert that a config file with multiple config sections has
+	# those sections preserved...
+	cat >expect <<-\EOF &&
+	branch.dest.key1=value1
+	some.gar.b=age
+	branch.dest.key2=value2
+	EOF
+	cat >config.branch <<\EOF &&
+;; Comment for source
+[branch "source"]
+	;; Comment for the source value
+	key1 = value1
+	;; Comment for some.gar
+[some "gar"]
+	;; Comment for the some.gar value
+	b = age
+	;; Comment for source, again
+[branch "source"]
+	;; Comment for the source value, again
+	key2 = value2
+EOF
+	cat config.branch >>.git/config &&
+	git branch -m source dest &&
+	git config -f .git/config -l | grep -F -e source -e dest -e some.gar >actual &&
+	test_cmp expect actual &&
+
+	# ...and that the comments for those sections are also
+	# preserved.
+	cat config.branch | sed "s/\"source\"/\"dest\"/" >expect &&
+	grep -A 9001 "Comment for source" .git/config >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'deleting a symref' '
 	git branch target &&
 	git symbolic-ref refs/heads/symref refs/heads/target &&

--
https://github.com/git/git/pull/363

[PATCH 3/3] branch: add a --copy (-c) option to go with --move (-m)

[Sahil Dua]

Add the ability to --copy a branch and its reflog and configuration,
this uses the same underlying machinery as the --move (-m) option
except the reflog and configuration is copied instead of being moved.

This is useful for e.g. copying a topic branch to a new version,
e.g. work to work-2 after submitting the work topic to the list, while
preserving all the tracking info and other configuration that goes
with the branch, and unlike --move keeping the other already-submitted
branch around for reference.

Like --move, when the source branch is the currently checked out
branch the HEAD is moved to the destination branch. In the case of
--move we don't really have a choice (other than remaining on a
detached HEAD), but it makes sense to do the same for --copy.

The most common usage of this feature is expected to be moving to a
new topic branch which is a copy of the current one, in that case
moving to the target branch is what the user wants, and doesn't
unexpectedly behave differently than --move would.

One outstanding caveat of this implementation is that:

    git checkout maint &&
    git checkout master &&
    git branch -c topic &&
    git checkout -

Will check out 'maint' instead of 'master'. This is because the @{-N}
feature (or its -1 shorthand "-") relies on HEAD reflogs created by
the checkout command, so in this case we'll checkout maint instead of
master, as the user might expect. What to do about that is left to a
future change.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-branch.txt |  14 ++-
 builtin/branch.c             |  67 ++++++++++----
 cache.h                      |   2 +
 config.c                     | 102 +++++++++++++++++----
 refs.c                       |  11 +++
 refs.h                       |   9 +-
 refs/files-backend.c         |  46 ++++++++--
 refs/refs-internal.h         |   4 +
 t/t3200-branch.sh            | 211 +++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 420 insertions(+), 46 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 81bd0a7b7741f..94fd89ddc5ab9 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -18,6 +18,7 @@ SYNOPSIS
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
 'git branch' --unset-upstream [<branchname>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
+'git branch' (-c | -C) [<oldbranch>] <newbranch>
 'git branch' (-d | -D) [-r] <branchname>...
 'git branch' --edit-description [<branchname>]
 
@@ -64,6 +65,10 @@ If <oldbranch> had a corresponding reflog, it is renamed to match
 renaming. If <newbranch> exists, -M must be used to force the rename
 to happen.
 
+The `-c` and `-C` options have the exact same semantics as `-m` and
+`-M`, except instead of the branch being renamed it along with its
+config and reflog will be copied to a new name.
+
 With a `-d` or `-D` option, `<branchname>` will be deleted.  You may
 specify more than one branch for deletion.  If the branch currently
 has a reflog then the reflog will also be deleted.
@@ -104,7 +109,7 @@ OPTIONS
 	In combination with `-d` (or `--delete`), allow deleting the
 	branch irrespective of its merged status. In combination with
 	`-m` (or `--move`), allow renaming the branch even if the new
-	branch name already exists.
+	branch name already exists, the same applies for `-c` (or `--copy`).
 
 -m::
 --move::
@@ -113,6 +118,13 @@ OPTIONS
 -M::
 	Shortcut for `--move --force`.
 
+-c::
+--copy::
+	Copy a branch and the corresponding reflog.
+
+-C::
+	Shortcut for `--copy --force`.
+
 --color[=<when>]::
 	Color branches to highlight current, local, and
 	remote-tracking branches.
diff --git a/builtin/branch.c b/builtin/branch.c
index 83fcda43dceec..89f64f4123a5e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-l] [-f] <branch-name> [<start-point>]"),
 	N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
 	N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
+	N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
 	N_("git branch [<options>] [-r | -a] [--points-at]"),
 	N_("git branch [<options>] [-r | -a] [--format]"),
 	NULL
@@ -449,15 +450,19 @@ static void reject_rebase_or_bisect_branch(const char *target)
 	free_worktrees(worktrees);
 }
 
-static void rename_branch(const char *oldname, const char *newname, int force)
+static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)
 {
 	struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
 	struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
 	int recovery = 0;
 	int clobber_head_ok;
 
-	if (!oldname)
-		die(_("cannot rename the current branch while not on any."));
+	if (!oldname) {
+		if (copy)
+			die(_("cannot copy the current branch while not on any."));
+		else
+			die(_("cannot rename the current branch while not on any."));
+	}
 
 	if (strbuf_check_branch_ref(&oldref, oldname)) {
 		/*
@@ -480,17 +485,33 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 
 	reject_rebase_or_bisect_branch(oldref.buf);
 
-	strbuf_addf(&logmsg, "Branch: renamed %s to %s",
-		 oldref.buf, newref.buf);
+	if (copy)
+		strbuf_addf(&logmsg, "Branch: copied %s to %s",
+			    oldref.buf, newref.buf);
+	else
+		strbuf_addf(&logmsg, "Branch: renamed %s to %s",
+			    oldref.buf, newref.buf);
 
-	if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
+	if (!copy && rename_ref(oldref.buf, newref.buf, logmsg.buf))
 		die(_("Branch rename failed"));
+	if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
+		die(_("Branch copy failed"));
 
-	if (recovery)
-		warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);
+	if (recovery) {
+		if (copy)
+			warning(_("Copied a misnamed branch '%s' away"),
+				oldref.buf + 11);
+		else
+			warning(_("Renamed a misnamed branch '%s' away"),
+				oldref.buf + 11);
+	}
 
-	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))
-		die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf)) {
+		if (copy)
+			die(_("Branch copied to %s, but HEAD is not updated!"), newname);
+		else
+			die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+	}
 
 	strbuf_release(&logmsg);
 
@@ -498,8 +519,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	strbuf_release(&oldref);
 	strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
 	strbuf_release(&newref);
-	if (git_config_rename_section(oldsection.buf, newsection.buf) < 0)
+	if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
 		die(_("Branch is renamed, but update of config-file failed"));
+	if (copy && strcmp(oldname, newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
+		die(_("Branch is copied, but update of config-file failed"));
 	strbuf_release(&oldsection);
 	strbuf_release(&newsection);
 }
@@ -537,7 +560,7 @@ static int edit_branch_description(const char *branch_name)
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-	int delete = 0, rename = 0, force = 0, list = 0;
+	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
@@ -574,6 +597,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
 		OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
 		OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
+		OPT_BIT('c', "copy", &copy, N_("copy a branch and its reflog"), 1),
+		OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
 		OPT_BOOL(0, "list", &list, N_("list branch names")),
 		OPT_BOOL('l', "create-reflog", &reflog, N_("create the branch's reflog")),
 		OPT_BOOL(0, "edit-description", &edit_description,
@@ -617,14 +642,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
+	if (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
 	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
 	    filter.no_commit)
 		list = 1;
 
-	if (!!delete + !!rename + !!new_upstream +
+	if (!!delete + !!rename + !!copy + !!new_upstream +
 	    list + unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
@@ -642,6 +667,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (force) {
 		delete *= 2;
 		rename *= 2;
+		copy *= 2;
 	}
 
 	if (delete) {
@@ -696,13 +722,22 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (edit_branch_description(branch_name))
 			return 1;
+	} else if (copy) {
+		if (!argc)
+			die(_("branch name required"));
+		else if (argc == 1)
+			copy_or_rename_branch(head, argv[0], 1, copy > 1);
+		else if (argc == 2)
+			copy_or_rename_branch(argv[0], argv[1], 1, copy > 1);
+		else
+			die(_("too many branches for a copy operation"));
 	} else if (rename) {
 		if (!argc)
 			die(_("branch name required"));
 		else if (argc == 1)
-			rename_branch(head, argv[0], rename > 1);
+			copy_or_rename_branch(head, argv[0], 0, rename > 1);
 		else if (argc == 2)
-			rename_branch(argv[0], argv[1], rename > 1);
+			copy_or_rename_branch(argv[0], argv[1], 0, rename > 1);
 		else
 			die(_("too many branches for a rename operation"));
 	} else if (new_upstream) {
diff --git a/cache.h b/cache.h
index 4d92aae0e8180..290e39d44d816 100644
--- a/cache.h
+++ b/cache.h
@@ -1941,6 +1941,8 @@ extern int git_config_set_multivar_in_file_gently(const char *, const char *, co
 extern void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
 extern int git_config_rename_section_in_file(const char *, const char *, const char *);
+extern int git_config_copy_section(const char *, const char *);
+extern int git_config_copy_section_in_file(const char *, const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
diff --git a/config.c b/config.c
index d5bb69e925dac..614e644389dc9 100644
--- a/config.c
+++ b/config.c
@@ -2638,8 +2638,8 @@ static int section_name_is_ok(const char *name)
 }
 
 /* if new_name == NULL, the section is removed instead */
-int git_config_rename_section_in_file(const char *config_filename,
-				      const char *old_name, const char *new_name)
+static int git_config_copy_or_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name, int copy)
 {
 	int ret = 0, remove = 0;
 	char *filename_buf = NULL;
@@ -2648,6 +2648,7 @@ int git_config_rename_section_in_file(const char *config_filename,
 	char buf[1024];
 	FILE *config_file = NULL;
 	struct stat st;
+	struct strbuf copystr = STRBUF_INIT;
 
 	if (new_name && !section_name_is_ok(new_name)) {
 		ret = error("invalid section name: %s", new_name);
@@ -2683,12 +2684,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 	while (fgets(buf, sizeof(buf), config_file)) {
 		int i;
 		int length;
+		int is_section = 0;
 		char *output = buf;
 		for (i = 0; buf[i] && isspace(buf[i]); i++)
 			; /* do nothing */
 		if (buf[i] == '[') {
 			/* it's a section */
-			int offset = section_name_match(&buf[i], old_name);
+			int offset;
+			is_section = 1;
+
+			/*
+			 * When encountering a new section under -c we
+			 * need to flush out any section we're already
+			 * coping and begin anew. There might be
+			 * multiple [branch "$name"] sections.
+			 */ 
+			if (copystr.len > 0) {
+				if (write_in_full(out_fd, copystr.buf, copystr.len) != copystr.len) {
+					ret = write_error(get_lock_file_path(lock));
+					goto out;
+				}
+				strbuf_reset(&copystr);
+			}
+
+			offset = section_name_match(&buf[i], old_name);
 			if (offset > 0) {
 				ret++;
 				if (new_name == NULL) {
@@ -2696,25 +2715,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 					continue;
 				}
 				store.baselen = strlen(new_name);
-				if (!store_write_section(out_fd, new_name)) {
-					ret = write_error(get_lock_file_path(lock));
-					goto out;
-				}
-				/*
-				 * We wrote out the new section, with
-				 * a newline, now skip the old
-				 * section's length
-				 */
-				output += offset + i;
-				if (strlen(output) > 0) {
+				if (!copy) {
+					if (!store_write_section(out_fd, new_name)) {
+						ret = write_error(get_lock_file_path(lock));
+						goto out;
+					}
+
 					/*
-					 * More content means there's
-					 * a declaration to put on the
-					 * next line; indent with a
-					 * tab
+					 * We wrote out the new section, with
+					 * a newline, now skip the old
+					 * section's length
 					 */
-					output -= 1;
-					output[0] = '\t';
+					output += offset + i;
+					if (strlen(output) > 0) {
+						/*
+						 * More content means there's
+						 * a declaration to put on the
+						 * next line; indent with a
+						 * tab
+						 */
+						output -= 1;
+						output[0] = '\t';
+					}
+				} else {
+					copystr = store_create_section(new_name);
 				}
 			}
 			remove = 0;
@@ -2722,11 +2746,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 		if (remove)
 			continue;
 		length = strlen(output);
+
+		if (!is_section && copystr.len > 0) {
+			strbuf_add(&copystr, output, length);
+		}
+
 		if (write_in_full(out_fd, output, length) != length) {
 			ret = write_error(get_lock_file_path(lock));
 			goto out;
 		}
 	}
+
+	/*
+	 * Copy a trailing section at the end of the config, won't be
+	 * flushed by the usual "flush because we have a new section
+	 * logic in the loop above.
+	 */
+	if (copystr.len > 0) {
+		if (write_in_full(out_fd, copystr.buf, copystr.len) != copystr.len) {
+			ret = write_error(get_lock_file_path(lock));
+			goto out;
+		}
+		strbuf_reset(&copystr);
+	}
+
 	fclose(config_file);
 	config_file = NULL;
 commit_and_out:
@@ -2742,11 +2785,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 	return ret;
 }
 
+int git_config_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
+{
+	return git_config_copy_or_rename_section_in_file(config_filename,
+					 old_name, new_name, 0);
+}
+
 int git_config_rename_section(const char *old_name, const char *new_name)
 {
 	return git_config_rename_section_in_file(NULL, old_name, new_name);
 }
 
+int git_config_copy_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
+{
+	return git_config_copy_or_rename_section_in_file(config_filename,
+					 old_name, new_name, 1);
+}
+
+int git_config_copy_section(const char *old_name, const char *new_name)
+{
+	return git_config_copy_section_in_file(NULL, old_name, new_name);
+}
+
 /*
  * Call this to report error for your variable that should not
  * get a boolean value (i.e. "[my] var" means "true").
diff --git a/refs.c b/refs.c
index f0685c92513e3..d724cda58f890 100644
--- a/refs.c
+++ b/refs.c
@@ -2032,3 +2032,14 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
 	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
 }
+
+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg)
+{
+	return refs->be->copy_ref(refs, oldref, newref, logmsg);
+}
+
+int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
+{
+	return refs_copy_existing_ref(get_main_ref_store(), oldref, newref, logmsg);
+}
diff --git a/refs.h b/refs.h
index 4be14c4b3cc65..00fce93f55574 100644
--- a/refs.h
+++ b/refs.h
@@ -440,7 +440,14 @@ char *shorten_unambiguous_ref(const char *refname, int strict);
 /** rename ref, return 0 on success **/
 int refs_rename_ref(struct ref_store *refs, const char *oldref,
 		    const char *newref, const char *logmsg);
-int rename_ref(const char *oldref, const char *newref, const char *logmsg);
+int rename_ref(const char *oldref, const char *newref,
+			const char *logmsg);
+
+/** copy ref, return 0 on success **/
+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg);
+int copy_existing_ref(const char *oldref, const char *newref,
+			const char *logmsg);
 
 int refs_create_symref(struct ref_store *refs, const char *refname,
 		       const char *target, const char *logmsg);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index d8b3f73147c8a..fcaf4f719136e 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1733,9 +1733,9 @@ static int commit_ref_update(struct files_ref_store *refs,
 			     const struct object_id *oid, const char *logmsg,
 			     struct strbuf *err);
 
-static int files_rename_ref(struct ref_store *ref_store,
+static int files_copy_or_rename_ref(struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
-			    const char *logmsg)
+			    const char *logmsg, int copy)
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
@@ -1767,8 +1767,12 @@ static int files_rename_ref(struct ref_store *ref_store,
 	}
 
 	if (flag & REF_ISSYMREF) {
-		ret = error("refname %s is a symbolic ref, renaming it is not supported",
-			    oldrefname);
+		if (copy)
+			ret = error("refname %s is a symbolic ref, copying it is not supported",
+				    oldrefname);
+		else
+			ret = error("refname %s is a symbolic ref, renaming it is not supported",
+				    oldrefname);
 		goto out;
 	}
 	if (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {
@@ -1776,13 +1780,19 @@ static int files_rename_ref(struct ref_store *ref_store,
 		goto out;
 	}
 
-	if (log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
+	if (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
 		ret = error("unable to move logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
 			    oldrefname, strerror(errno));
 		goto out;
 	}
 
-	if (refs_delete_ref(&refs->base, logmsg, oldrefname,
+	if (copy && log && copy_file(tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
+		ret = error("unable to copy logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
+			    oldrefname, strerror(errno));
+		goto out;
+	}
+
+	if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,
 			    orig_oid.hash, REF_NODEREF)) {
 		error("unable to delete old %s", oldrefname);
 		goto rollback;
@@ -1795,7 +1805,7 @@ static int files_rename_ref(struct ref_store *ref_store,
 	 * the safety anyway; we want to delete the reference whatever
 	 * its current value.
 	 */
-	if (!refs_read_ref_full(&refs->base, newrefname,
+	if (!copy && !refs_read_ref_full(&refs->base, newrefname,
 				RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
 				oid.hash, NULL) &&
 	    refs_delete_ref(&refs->base, NULL, newrefname,
@@ -1826,7 +1836,10 @@ static int files_rename_ref(struct ref_store *ref_store,
 	lock = lock_ref_sha1_basic(refs, newrefname, NULL, NULL, NULL,
 				   REF_NODEREF, NULL, &err);
 	if (!lock) {
-		error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
+		if (copy)
+			error("unable to copy '%s' to '%s': %s", oldrefname, newrefname, err.buf);
+		else
+			error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
 		strbuf_release(&err);
 		goto rollback;
 	}
@@ -1877,6 +1890,22 @@ static int files_rename_ref(struct ref_store *ref_store,
 	return ret;
 }
 
+static int files_rename_ref(struct ref_store *ref_store,
+			    const char *oldrefname, const char *newrefname,
+			    const char *logmsg)
+{
+	return files_copy_or_rename_ref(ref_store, oldrefname,
+				 newrefname, logmsg, 0);
+}
+
+static int files_copy_ref(struct ref_store *ref_store,
+			    const char *oldrefname, const char *newrefname,
+			    const char *logmsg)
+{
+	return files_copy_or_rename_ref(ref_store, oldrefname,
+				 newrefname, logmsg, 1);
+}
+
 static int close_ref(struct ref_lock *lock)
 {
 	if (close_lock_file(lock->lk))
@@ -3383,6 +3412,7 @@ struct ref_storage_be refs_be_files = {
 	files_create_symref,
 	files_delete_refs,
 	files_rename_ref,
+	files_copy_ref,
 
 	files_ref_iterator_begin,
 	files_read_raw_ref,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 192f9f85c97c0..3c51194edb190 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -543,6 +543,9 @@ typedef int delete_refs_fn(struct ref_store *ref_store, const char *msg,
 typedef int rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
+typedef int copy_ref_fn(struct ref_store *ref_store,
+			  const char *oldref, const char *newref,
+			  const char *logmsg);
 
 /*
  * Iterate over the references in `ref_store` whose names start with
@@ -641,6 +644,7 @@ struct ref_storage_be {
 	create_symref_fn *create_symref;
 	delete_refs_fn *delete_refs;
 	rename_ref_fn *rename_ref;
+	copy_ref_fn *copy_ref;
 
 	ref_iterator_begin_fn *iterator_begin;
 	read_raw_ref_fn *read_raw_ref;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 28c02ffeadb4f..23a773dc25e93 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -378,6 +378,217 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'git branch -c dumps usage' '
+	test_expect_code 128 git branch -c 2>err &&
+	test_i18ngrep "branch name required" err
+'
+
+test_expect_success 'git branch --copy dumps usage' '
+	test_expect_code 128 git branch --copy 2>err &&
+	test_i18ngrep "branch name required" err
+'
+
+test_expect_success 'git branch -c d e should work' '
+	git branch -l d &&
+	git reflog exists refs/heads/d &&
+	git config branch.d.dummy Hello &&
+	git branch -c d e &&
+	git reflog exists refs/heads/d &&
+	git reflog exists refs/heads/e &&
+	echo Hello >expect &&
+	git config branch.e.dummy >actual &&
+	test_cmp expect actual &&
+	echo Hello >expect &&
+	git config branch.d.dummy >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git branch --copy is a synonym for -c' '
+	git branch -l copy &&
+	git reflog exists refs/heads/copy &&
+	git config branch.copy.dummy Hello &&
+	git branch --copy copy copy-to &&
+	git reflog exists refs/heads/copy &&
+	git reflog exists refs/heads/copy-to &&
+	echo Hello >expect &&
+	git config branch.copy.dummy >actual &&
+	test_cmp expect actual &&
+	echo Hello >expect &&
+	git config branch.copy-to.dummy >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git branch -c ee ef should copy and checkout branch ef' '
+	git checkout -b ee &&
+	git reflog exists refs/heads/ee &&
+	git config branch.ee.dummy Hello &&
+	git branch -c ee ef &&
+	git reflog exists refs/heads/ee &&
+	git reflog exists refs/heads/ef &&
+	test $(git config branch.ee.dummy) = Hello &&
+	test $(git config branch.ef.dummy) = Hello &&
+	test $(git rev-parse --abbrev-ref HEAD) = ef
+'
+
+test_expect_success 'git branch -c f/f g/g should work' '
+	git branch -l f/f &&
+	git reflog exists refs/heads/f/f &&
+	git config branch.f/f.dummy Hello &&
+	git branch -c f/f g/g &&
+	git reflog exists refs/heads/f/f &&
+	git reflog exists refs/heads/g/g &&
+	test $(git config branch.f/f.dummy) = Hello &&
+	test $(git config branch.g/g.dummy) = Hello
+'
+
+test_expect_success 'git branch -c m2 m2 should work' '
+	git branch -l m2 &&
+	git reflog exists refs/heads/m2 &&
+	git config branch.m2.dummy Hello &&
+	git branch -c m2 m2 &&
+	git reflog exists refs/heads/m2 &&
+	test $(git config branch.m2.dummy) = Hello
+'
+
+test_expect_success 'git branch -c zz zz/zz should fail' '
+	git branch -l zz &&
+	git reflog exists refs/heads/zz &&
+	test_must_fail git branch -c zz zz/zz
+'
+
+test_expect_success 'git branch -c b/b b should fail' '
+	git branch -l b/b &&
+	test_must_fail git branch -c b/b b
+'
+
+test_expect_success 'git branch -C o/q o/p should work when o/p exists' '
+	git branch -l o/q &&
+	git reflog exists refs/heads/o/q &&
+	git reflog exists refs/heads/o/p &&
+	git branch -C o/q o/p
+'
+
+test_expect_success 'git branch -c -f o/q o/p should work when o/p exists' '
+	git reflog exists refs/heads/o/q &&
+	git reflog exists refs/heads/o/p &&
+	git branch -c -f o/q o/p
+'
+
+test_expect_success 'git branch -c qq rr/qq should fail when r exists' '
+	git branch qq &&
+	git branch rr &&
+	test_must_fail git branch -c qq rr/qq
+'
+
+test_expect_success 'git branch -C b1 b2 should fail when b2 is checked out' '
+	git branch b1 &&
+	git checkout -b b2 &&
+	test_must_fail git branch -C b1 b2
+'
+
+test_expect_success 'git branch -C c1 c2 should succeed when c1 is checked out' '
+	git checkout -b c1 &&
+	git branch c2 &&
+	git branch -C c1 c2 &&
+	test $(git rev-parse --abbrev-ref HEAD) = c2
+'
+
+test_expect_success 'git branch -C c1 c2 should add entries to .git/logs/HEAD' '
+	msg="Branch: copied refs/heads/c1 to refs/heads/c2" &&
+	grep "$msg$" .git/logs/HEAD
+'
+
+test_expect_success 'git branch -C master should work when master is checked out' '
+	git checkout master &&
+	git branch -C master
+'
+
+test_expect_success 'git branch -C master master should work when master is checked out' '
+	git checkout master &&
+	git branch -C master master
+'
+
+test_expect_success 'git branch -C master5 master5 should work when master is checked out' '
+	git checkout master &&
+	git branch master5 &&
+	git branch -C master5 master5
+'
+
+test_expect_success 'git branch -C ab cd should overwrite existing config for cd' '
+	git branch -l cd &&
+	git reflog exists refs/heads/cd &&
+	git config branch.cd.dummy CD &&
+	git branch -l ab &&
+	git reflog exists refs/heads/ab &&
+	git config branch.ab.dummy AB &&
+	git branch -C ab cd &&
+	git reflog exists refs/heads/ab &&
+	git reflog exists refs/heads/cd &&
+	test $(git config branch.ab.dummy) = AB &&
+	test $(git config branch.cd.dummy) = AB
+'
+
+test_expect_success 'git branch -c correctly copies multiple config sections' '
+	FOO=1 &&
+	export FOO &&
+	test_when_finished "git checkout master" &&
+	git checkout -b source2 master &&
+
+	# Assert that a config file with multiple config sections has
+	# those sections preserved...
+	cat >expect <<-\EOF &&
+	branch.source2.key1=value1
+	branch.dest2.key1=value1
+	more.gar.b=age
+	branch.source2.key2=value2
+	branch.dest2.key2=value2
+	EOF
+	cat >config.branch <<\EOF &&
+;; Comment for source2
+[branch "source2"]
+	;; Comment for the source2 value
+	key1 = value1
+;; Comment for more.gar
+[more "gar"]
+	;; Comment for the more.gar value
+	b = age
+	;; Comment for source, again
+[branch "source2"]
+	;; Comment for the source2 value, again
+	key2 = value2
+EOF
+	cat config.branch >>.git/config &&
+	git branch -c source2 dest2 &&
+	git config -f .git/config -l | grep -F -e source2 -e dest2 -e more.gar >actual &&
+	test_cmp expect actual &&
+
+	# ...and that the comments and formatting for those sections
+	# is also preserved.
+	cat >expect <<\EOF &&
+;; Comment for source2
+[branch "source2"]
+	;; Comment for the source2 value
+	key1 = value1
+;; Comment for more.gar
+[branch "dest2"]
+	;; Comment for the source2 value
+	key1 = value1
+;; Comment for more.gar
+[more "gar"]
+	;; Comment for the more.gar value
+	b = age
+	;; Comment for source, again
+[branch "source2"]
+	;; Comment for the source2 value, again
+	key2 = value2
+[branch "dest2"]
+	;; Comment for the source2 value, again
+	key2 = value2
+EOF
+	grep -A 9001 "Comment for source2" .git/config >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'deleting a symref' '
 	git branch target &&
 	git symbolic-ref refs/heads/symref refs/heads/target &&

--
https://github.com/git/git/pull/363

Re: [PATCH 3/3] branch: add a --copy (-c) option to go with --move (-m)

[Junio C Hamano]

Sahil Dua <sahildua2305@gmail.com> writes:

> Add the ability to --copy a branch and its reflog and configuration,
> this uses the same underlying machinery as the --move (-m) option
> except the reflog and configuration is copied instead of being moved.
>
> This is useful for e.g. copying a topic branch to a new version,
> e.g. work to work-2 after submitting the work topic to the list, while
> preserving all the tracking info and other configuration that goes
> with the branch, and unlike --move keeping the other already-submitted
> branch around for reference.
>
> Like --move, when the source branch is the currently checked out
> branch the HEAD is moved to the destination branch. In the case of
> --move we don't really have a choice (other than remaining on a
> detached HEAD), but it makes sense to do the same for --copy.

I strongly disagree with this "it makes sense to do the same".  It
would equally (if not more) make sense to keep the HEAD pointing at
the same.

Personally, I may use this feature if it didn't move HEAD, but I
wouldn't if HEAD gets moved.  But that may be just me.

Re: [PATCH 1/3] config: create a function to format section headers

[Junio C Hamano]

Sahil Dua <sahildua2305@gmail.com> writes:

> Factor out the logic which creates section headers in the config file,
> e.g. the 'branch.foo' key will be turned into '[branch "foo"]'.
>
> This introduces no function changes, but is needed for a later change
> which adds support for copying branch sections in the config file.
>
> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  config.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index 146cb3452adab..d5bb69e925dac 100644
> --- a/config.c
> +++ b/config.c
> @@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
>  	return 4;
>  }
>  
> -static int store_write_section(int fd, const char *key)
> +struct strbuf store_create_section(const char *key)
>  {
>  	const char *dot;
> -	int i, success;
> +	int i;
>  	struct strbuf sb = STRBUF_INIT;
>  
>  	dot = memchr(key, '.', store.baselen);
> @@ -2188,6 +2188,15 @@ static int store_write_section(int fd, const char *key)
>  		strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
>  	}
>  
> +	return sb;
> +}
> +
> +static int store_write_section(int fd, const char *key)
> +{
> +	int success;
> +
> +	struct strbuf sb = store_create_section(key);
> +
>  	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
>  	strbuf_release(&sb);
>  
>
> --
> https://github.com/git/git/pull/363

Makes sense.

Re: [PATCH 1/3] config: create a function to format section headers

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 13, 2017 at 6:17 PM, Sahil Dua <sahildua2305@gmail.com> wrote:
> Factor out the logic which creates section headers in the config file,
> e.g. the 'branch.foo' key will be turned into '[branch "foo"]'.

+CC Junio: This is to be applied, Sahil is using submitgit which
apparently doesn't CC you by default (I don't know if there's some
option for that he missed).

Also +CC Lars who hacked up -m initially, Alex who hacked on some of
the logic of git_config_rename_section(), and Johannes who also hacked
on some of the guts of that code.

Also, in the interest of full disclosure. Sahil's a co-worker of mine
who I've been mentoring on how to submit changes to git.git, after I
added a note to the internal company announcement saying I'd upgraded
git advertising that I'd be happy to help anyone who's interested
contribute to the project. That explains the off-list review & Sahil
submitting a patch of mine first seen as part of this series.

Re: [PATCH 2/3] branch: add test for -m renaming multiple config sections

[Junio C Hamano]

Sahil Dua <sahildua2305@gmail.com> writes:

> +	cat >expect <<-\EOF &&
> +	branch.dest.key1=value1
> +	some.gar.b=age
> +	branch.dest.key2=value2
> +	EOF
> +	cat >config.branch <<\EOF &&
> +;; Comment for source
> +[branch "source"]
> +	;; Comment for the source value
> +	key1 = value1
> +	;; Comment for some.gar
> +[some "gar"]
> +	;; Comment for the some.gar value
> +	b = age
> +	;; Comment for source, again
> +[branch "source"]
> +	;; Comment for the source value, again
> +	key2 = value2
> +EOF

Indenting using <<- would make it easier to read.  I.e.

	cat >config.branch <<-\EOF &&
	;; Comment for ...
	[branch "source"]
		;; Comment for ...
	...
	EOF

> +	cat config.branch >>.git/config &&
> +	git branch -m source dest &&
> +	git config -f .git/config -l | grep -F -e source -e dest -e some.gar >actual &&
> +	test_cmp expect actual &&
> +
> +	# ...and that the comments for those sections are also
> +	# preserved.
> +	cat config.branch | sed "s/\"source\"/\"dest\"/" >expect &&
> +	grep -A 9001 "Comment for source" .git/config >actual &&

Where does 9001 come from?  Is that just "an arbitrary large
number"?
	
Besides, "grep -A" is quite unportable.  Would

	sed -n -e "/Comment for source/,$p" .git/config >actual

work equally well?

> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'deleting a symref' '
>  	git branch target &&
>  	git symbolic-ref refs/heads/symref refs/heads/target &&
>
> --
> https://github.com/git/git/pull/363

Re: [PATCH 3/3] branch: add a --copy (-c) option to go with --move (-m)

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Sahil Dua <sahildua2305@gmail.com> writes:
>
>> Add the ability to --copy a branch and its reflog and configuration,
>> this uses the same underlying machinery as the --move (-m) option
>> except the reflog and configuration is copied instead of being moved.
>>
>> This is useful for e.g. copying a topic branch to a new version,
>> e.g. work to work-2 after submitting the work topic to the list, while
>> preserving all the tracking info and other configuration that goes
>> with the branch, and unlike --move keeping the other already-submitted
>> branch around for reference.
>>
>> Like --move, when the source branch is the currently checked out
>> branch the HEAD is moved to the destination branch. In the case of
>> --move we don't really have a choice (other than remaining on a
>> detached HEAD), but it makes sense to do the same for --copy.
>
> I strongly disagree with this "it makes sense to do the same".  It
> would equally (if not more) make sense to keep the HEAD pointing at
> the same.
>
> Personally, I may use this feature if it didn't move HEAD, but I
> wouldn't if HEAD gets moved.  But that may be just me.

Ah, that came out to be stronger than I intended.

While I do prefer "the HEAD is not moved by this command---if you
want to move to the newly created branch after copying, check it out
yourself" a lot better than what the patch does, I do not think I'd
care so strongly that I'd reject this patch series unless the
behaviour is changed.

But I do react strongly to an unsubstantiated claim "it makes sense
to do the same".  I can buy "We anticipate that in 50% of the case
users would find this branch switching annoying and in the other 50%
of the case, users would find it useful; since we need to pick one,
we just randomly decide to do the same as --move", though.

Re: [PATCH 2/3] branch: add test for -m renaming multiple config sections

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 13, 2017 at 7:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Sahil Dua <sahildua2305@gmail.com> writes:
>
>> +     cat >expect <<-\EOF &&
>> +     branch.dest.key1=value1
>> +     some.gar.b=age
>> +     branch.dest.key2=value2
>> +     EOF
>> +     cat >config.branch <<\EOF &&
>> +;; Comment for source
>> +[branch "source"]
>> +     ;; Comment for the source value
>> +     key1 = value1
>> +     ;; Comment for some.gar
>> +[some "gar"]
>> +     ;; Comment for the some.gar value
>> +     b = age
>> +     ;; Comment for source, again
>> +[branch "source"]
>> +     ;; Comment for the source value, again
>> +     key2 = value2
>> +EOF
>
> Indenting using <<- would make it easier to read.  I.e.
>
>         cat >config.branch <<-\EOF &&
>         ;; Comment for ...
>         [branch "source"]
>                 ;; Comment for ...
>         ...
>         EOF

I should have added a comment for that, I can't find any portable (but
suggestions welcome) way to do that and preserve the indentation, so
the test_cmp would still succeed if the moving/renaming function
munged all leading whitespace in the config with -\EOF as opposed to
\EOF.

>> +     cat config.branch >>.git/config &&
>> +     git branch -m source dest &&
>> +     git config -f .git/config -l | grep -F -e source -e dest -e some.gar >actual &&
>> +     test_cmp expect actual &&
>> +
>> +     # ...and that the comments for those sections are also
>> +     # preserved.
>> +     cat config.branch | sed "s/\"source\"/\"dest\"/" >expect &&
>> +     grep -A 9001 "Comment for source" .git/config >actual &&
>
> Where does 9001 come from?  Is that just "an arbitrary large
> number"?
>
> Besides, "grep -A" is quite unportable.  Would
>
>         sed -n -e "/Comment for source/,$p" .git/config >actual
>
> work equally well?

It's just a sufficiently large number, I thought -A was portable
enough after grepping the test suite, but on closer inspection it
turns out those were all git-grep invocations, oops. Yeah all I need
here is all lines after a line matching a given string, so that sed
command works, will fix it up to use that.

>> +     test_cmp expect actual
>> +'
>> +
>>  test_expect_success 'deleting a symref' '
>>       git branch target &&
>>       git symbolic-ref refs/heads/symref refs/heads/target &&
>>
>> --
>> https://github.com/git/git/pull/363

Re: [PATCH 2/3] branch: add test for -m renaming multiple config sections

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Tue, Jun 13, 2017 at 7:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Indenting using <<- would make it easier to read.  I.e.
>>
>>         cat >config.branch <<-\EOF &&
>>         ;; Comment for ...
>>         [branch "source"]
>>                 ;; Comment for ...
>>         ...
>>         EOF
>
> I should have added a comment for that, I can't find any portable (but
> suggestions welcome) way to do that and preserve the indentation, so
> the test_cmp would still succeed if the moving/renaming function
> munged all leading whitespace in the config with -\EOF as opposed to
> \EOF.

Ah, I see why it is done that way.  You could indent the lines in
the configuration file with SPs (<<- strips only HTs, no?)

> It's just a sufficiently large number, I thought -A was portable
> enough after grepping the test suite, but on closer inspection it
> turns out those were all git-grep invocations, oops. Yeah all I need
> here is all lines after a line matching a given string, so that sed
> command works, will fix it up to use that.

Oops, indeed ;-)  Thanks.

Re: [PATCH 2/3] branch: add test for -m renaming multiple config sections

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 13 2017, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Tue, Jun 13, 2017 at 7:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Indenting using <<- would make it easier to read.  I.e.
>>>
>>>         cat >config.branch <<-\EOF &&
>>>         ;; Comment for ...
>>>         [branch "source"]
>>>                 ;; Comment for ...
>>>         ...
>>>         EOF
>>
>> I should have added a comment for that, I can't find any portable (but
>> suggestions welcome) way to do that and preserve the indentation, so
>> the test_cmp would still succeed if the moving/renaming function
>> munged all leading whitespace in the config with -\EOF as opposed to
>> \EOF.
>
> Ah, I see why it is done that way.  You could indent the lines in
> the configuration file with SPs (<<- strips only HTs, no?)

Sure, we'll do that for v2.

>> It's just a sufficiently large number, I thought -A was portable
>> enough after grepping the test suite, but on closer inspection it
>> turns out those were all git-grep invocations, oops. Yeah all I need
>> here is all lines after a line matching a given string, so that sed
>> command works, will fix it up to use that.
>
> Oops, indeed ;-)  Thanks.

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Jonathan Nieder]

Hi,

Ævar Arnfjörð Bjarmason wrote:

> So the reason we have this for -m is:
>
>     commit 3f59481e33
>     Author: Jonathan Nieder <jrnieder@gmail.com>
>     Date:   Fri Nov 25 20:30:02 2011 -0600
>
>     branch: allow a no-op "branch -M <current-branch> HEAD"
>
>     Overwriting the current branch with a different commit is forbidden, as it
>     will make the status recorded in the index and the working tree out of
>     sync with respect to the HEAD. There however is no reason to forbid it if
>     the current branch is renamed to itself, which admittedly is something
>     only an insane user would do, but is handy for scripts.
>
> My understanding of that last part is that Jonathan/someone (see
> reported-by in that patch) had some script which was renaming
> branches, and it was easier for whatever reason to just make it no-op
> if the rename would have yielded the same result as doing nothing at
> all.
>
> Most likely your implementation will consist of just re-using the
> logic in rename_branch() (and renaming it to e.g.
> copy_or_rename_branch() ...) so you could just re-use the no-op
> behavior we use for -m, or if there's some reason not to no-op and
> error instead for -c we could just do that, but in any case this case
> of `git branch -c master master` or `git branch -c currentbranch`
> should be tested for.

I may be missing some context, but notice that the above mentioned
commit is about -M, not -m.

Thanks and hope that helps,
Jonathan

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 13 2017, Jonathan Nieder jotted:

> Hi,
>
> Ævar Arnfjörð Bjarmason wrote:
>
>> So the reason we have this for -m is:
>>
>>     commit 3f59481e33
>>     Author: Jonathan Nieder <jrnieder@gmail.com>
>>     Date:   Fri Nov 25 20:30:02 2011 -0600
>>
>>     branch: allow a no-op "branch -M <current-branch> HEAD"
>>
>>     Overwriting the current branch with a different commit is forbidden, as it
>>     will make the status recorded in the index and the working tree out of
>>     sync with respect to the HEAD. There however is no reason to forbid it if
>>     the current branch is renamed to itself, which admittedly is something
>>     only an insane user would do, but is handy for scripts.
>>
>> My understanding of that last part is that Jonathan/someone (see
>> reported-by in that patch) had some script which was renaming
>> branches, and it was easier for whatever reason to just make it no-op
>> if the rename would have yielded the same result as doing nothing at
>> all.
>>
>> Most likely your implementation will consist of just re-using the
>> logic in rename_branch() (and renaming it to e.g.
>> copy_or_rename_branch() ...) so you could just re-use the no-op
>> behavior we use for -m, or if there's some reason not to no-op and
>> error instead for -c we could just do that, but in any case this case
>> of `git branch -c master master` or `git branch -c currentbranch`
>> should be tested for.
>
> I may be missing some context, but notice that the above mentioned
> commit is about -M, not -m.

The context was just that that commit added a change in how -M
interacted when clobbering the current HEAD, and that -C should have a
test for that behavior, which the patch now submitted to the list has:

    +test_expect_success 'git branch -C master master should work when master is checked out' '
    +       git checkout master &&
    +       git branch -C master master
    +'

Re: [PATCH/RFC] branch: add tests for new copy branch feature

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:
> On Tue, Jun 13 2017, Jonathan Nieder jotted:
> > Ævar Arnfjörð Bjarmason wrote:

>>> My understanding of that last part is that Jonathan/someone (see
>>> reported-by in that patch) had some script which was renaming
>>> branches, and it was easier for whatever reason to just make it no-op
>>> if the rename would have yielded the same result as doing nothing at
>>> all.
>>>
>>> Most likely your implementation will consist of just re-using the
>>> logic in rename_branch() (and renaming it to e.g.
>>> copy_or_rename_branch() ...) so you could just re-use the no-op
>>> behavior we use for -m, or if there's some reason not to no-op and
>>> error instead for -c we could just do that, but in any case this case
>>> of `git branch -c master master` or `git branch -c currentbranch`
>>> should be tested for.
>>
>> I may be missing some context, but notice that the above mentioned
>> commit is about -M, not -m.
>
> The context was just that that commit added a change in how -M
> interacted when clobbering the current HEAD, and that -C should have a
> test for that behavior, which the patch now submitted to the list has:
>
>     +test_expect_success 'git branch -C master master should work when master is checked out' '
>     +       git checkout master &&
>     +       git branch -C master master
>     +'

Perfect, thanks.  Carry on. :)

Jonathan

Re: [PATCH 3/3] branch: add a --copy (-c) option to go with --move (-m)

[Sahil Dua]

On Tue, Jun 13, 2017 at 7:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Sahil Dua <sahildua2305@gmail.com> writes:
>>
>>> Add the ability to --copy a branch and its reflog and configuration,
>>> this uses the same underlying machinery as the --move (-m) option
>>> except the reflog and configuration is copied instead of being moved.
>>>
>>> This is useful for e.g. copying a topic branch to a new version,
>>> e.g. work to work-2 after submitting the work topic to the list, while
>>> preserving all the tracking info and other configuration that goes
>>> with the branch, and unlike --move keeping the other already-submitted
>>> branch around for reference.
>>>
>>> Like --move, when the source branch is the currently checked out
>>> branch the HEAD is moved to the destination branch. In the case of
>>> --move we don't really have a choice (other than remaining on a
>>> detached HEAD), but it makes sense to do the same for --copy.
>>
>> I strongly disagree with this "it makes sense to do the same".  It
>> would equally (if not more) make sense to keep the HEAD pointing at
>> the same.
>>
>> Personally, I may use this feature if it didn't move HEAD, but I
>> wouldn't if HEAD gets moved.  But that may be just me.
>
> Ah, that came out to be stronger than I intended.
>
> While I do prefer "the HEAD is not moved by this command---if you
> want to move to the newly created branch after copying, check it out
> yourself" a lot better than what the patch does, I do not think I'd
> care so strongly that I'd reject this patch series unless the
> behaviour is changed.
>
> But I do react strongly to an unsubstantiated claim "it makes sense
> to do the same".  I can buy "We anticipate that in 50% of the case
> users would find this branch switching annoying and in the other 50%
> of the case, users would find it useful; since we need to pick one,
> we just randomly decide to do the same as --move", though.

Okay, "it makes sense to do the same" was not the best way to say it.
Basically what it meant was that considering there are conflicting
opinions about how --copy should work with respect to checking out
branch automatically, it makes sense to do it the same way as --move
just to come to a conclusion. If we get user feedback that a lot of
users find it annoying, we can make it work the other way around
(without checkout branch automatically).

I will fix the commit message to explain this in v2. Is that okay?

[PATCH v2 1/3] config: create a function to format section headers

[Sahil Dua]

Factor out the logic which creates section headers in the config file,
e.g. the 'branch.foo' key will be turned into '[branch "foo"]'.

This introduces no function changes, but is needed for a later change
which adds support for copying branch sections in the config file.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 config.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index 34a139c..32fd3c8 100644
--- a/config.c
+++ b/config.c
@@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
 	return 4;
 }
 
-static int store_write_section(int fd, const char *key)
+static struct strbuf store_create_section(const char *key)
 {
 	const char *dot;
-	int i, success;
+	int i;
 	struct strbuf sb = STRBUF_INIT;
 
 	dot = memchr(key, '.', store.baselen);
@@ -2188,6 +2188,15 @@ static int store_write_section(int fd, const char *key)
 		strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
 	}
 
+	return sb;
+}
+
+static int store_write_section(int fd, const char *key)
+{
+	int success;
+
+	struct strbuf sb = store_create_section(key);
+
 	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
 	strbuf_release(&sb);
 
-- 
2.7.4 (Apple Git-66)

[PATCH v2 2/3] branch: add test for -m renaming multiple config sections

[Sahil Dua]

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Add a test for how 'git branch -m' handles the renaming of multiple
config sections existing for one branch.

The config format we use is hybrid machine/human editable, and we do
our best to preserve the likes of comments and formatting when editing
the file with git-config.

This adds a test for the currently expected semantics in the face of
some rather obscure edge cases which are unlikely to occur in
practice.

Helped-by: Sahil Dua <sahildua2305@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---

Changes
  * Use 'sed -n -e' instead of 'grep -A'.
  * Add mixed indentation for different config sections to
    test that formatting of renamed sections is preserved.

Note (from Avar): I ended up not taking Junio's commentary to use
<<-\EOF with spaces. I think it's important that this codepath be
tested for the exact formatting of the config being preserved, so the
test now does that with more exhaustive checks for the formatting at
the trivial expense of making the test not look like the usual
test idiom.

 t/t3200-branch.sh | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 10f8f02..5eb752c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -341,6 +341,47 @@ test_expect_success 'config information was renamed, too' '
 	test_must_fail git config branch.s/s.dummy
 '
 
+test_expect_success 'git branch -m correctly renames multiple config sections' '
+	test_when_finished "git checkout master" &&
+	git checkout -b source master &&
+
+	# Assert that a config file with multiple config sections has
+	# those sections preserved...
+	cat >expect <<-\EOF &&
+	branch.dest.key1=value1
+	some.gar.b=age
+	branch.dest.key2=value2
+	EOF
+	cat >config.branch <<\EOF &&
+;; Note the lack of -\EOF above & mixed indenting here. This is
+;; intentional, we are also testing that the formatting of copied
+;; sections is preserved.
+
+;; Comment for source. Tabs
+[branch "source"]
+	;; Comment for the source value
+	key1 = value1
+;; Comment for some.gar. Spaces
+[some "gar"]
+    ;; Comment for the some.gar value
+    b = age
+;; Comment for source, again. Mixed tabs/spaces.
+[branch "source"]
+    ;; Comment for the source value, again
+	key2 = value2
+EOF
+	cat config.branch >>.git/config &&
+	git branch -m source dest &&
+	git config -f .git/config -l | grep -F -e source -e dest -e some.gar >actual &&
+	test_cmp expect actual &&
+
+	# ...and that the comments for those sections are also
+	# preserved.
+	cat config.branch | sed "s/\"source\"/\"dest\"/" >expect &&
+	sed -n -e "/Note the lack/,\$p" .git/config >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'deleting a symref' '
 	git branch target &&
 	git symbolic-ref refs/heads/symref refs/heads/target &&
-- 
2.7.4 (Apple Git-66)

[PATCH v2 3/3] branch: add a --copy (-c) option to go with --move (-m)

[Sahil Dua]

Add the ability to --copy a branch and its reflog and configuration,
this uses the same underlying machinery as the --move (-m) option
except the reflog and configuration is copied instead of being moved.

This is useful for e.g. copying a topic branch to a new version,
e.g. work to work-2 after submitting the work topic to the list, while
preserving all the tracking info and other configuration that goes
with the branch, and unlike --move keeping the other already-submitted
branch around for reference.

Like --move, when the source branch is the currently checked out
branch the HEAD is moved to the destination branch. In the case of
--move we don't really have a choice (other than remaining on a
detached HEAD) and in order to keep the functionality consistent, we
are doing it in similar way for --copy too.

The most common usage of this feature is expected to be moving to a
new topic branch which is a copy of the current one, in that case
moving to the target branch is what the user wants, and doesn't
unexpectedly behave differently than --move would.

One outstanding caveat of this implementation is that:

    git checkout maint &&
    git checkout master &&
    git branch -c topic &&
    git checkout -

Will check out 'maint' instead of 'master'. This is because the @{-N}
feature (or its -1 shorthand "-") relies on HEAD reflogs created by
the checkout command, so in this case we'll checkout maint instead of
master, as the user might expect. What to do about that is left to a
future change.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
---
 Documentation/git-branch.txt |  14 ++-
 builtin/branch.c             |  67 ++++++++++----
 cache.h                      |   2 +
 config.c                     | 102 ++++++++++++++++----
 refs.c                       |  11 +++
 refs.h                       |   9 +-
 refs/files-backend.c         |  46 +++++++--
 refs/refs-internal.h         |   4 +
 t/t3200-branch.sh            | 215 +++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 424 insertions(+), 46 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 81bd0a7..94fd89d 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -18,6 +18,7 @@ SYNOPSIS
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
 'git branch' --unset-upstream [<branchname>]
 'git branch' (-m | -M) [<oldbranch>] <newbranch>
+'git branch' (-c | -C) [<oldbranch>] <newbranch>
 'git branch' (-d | -D) [-r] <branchname>...
 'git branch' --edit-description [<branchname>]
 
@@ -64,6 +65,10 @@ If <oldbranch> had a corresponding reflog, it is renamed to match
 renaming. If <newbranch> exists, -M must be used to force the rename
 to happen.
 
+The `-c` and `-C` options have the exact same semantics as `-m` and
+`-M`, except instead of the branch being renamed it along with its
+config and reflog will be copied to a new name.
+
 With a `-d` or `-D` option, `<branchname>` will be deleted.  You may
 specify more than one branch for deletion.  If the branch currently
 has a reflog then the reflog will also be deleted.
@@ -104,7 +109,7 @@ OPTIONS
 	In combination with `-d` (or `--delete`), allow deleting the
 	branch irrespective of its merged status. In combination with
 	`-m` (or `--move`), allow renaming the branch even if the new
-	branch name already exists.
+	branch name already exists, the same applies for `-c` (or `--copy`).
 
 -m::
 --move::
@@ -113,6 +118,13 @@ OPTIONS
 -M::
 	Shortcut for `--move --force`.
 
+-c::
+--copy::
+	Copy a branch and the corresponding reflog.
+
+-C::
+	Shortcut for `--copy --force`.
+
 --color[=<when>]::
 	Color branches to highlight current, local, and
 	remote-tracking branches.
diff --git a/builtin/branch.c b/builtin/branch.c
index 83fcda4..89f64f4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -27,6 +27,7 @@ static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-l] [-f] <branch-name> [<start-point>]"),
 	N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
 	N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
+	N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
 	N_("git branch [<options>] [-r | -a] [--points-at]"),
 	N_("git branch [<options>] [-r | -a] [--format]"),
 	NULL
@@ -449,15 +450,19 @@ static void reject_rebase_or_bisect_branch(const char *target)
 	free_worktrees(worktrees);
 }
 
-static void rename_branch(const char *oldname, const char *newname, int force)
+static void copy_or_rename_branch(const char *oldname, const char *newname, int copy, int force)
 {
 	struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
 	struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
 	int recovery = 0;
 	int clobber_head_ok;
 
-	if (!oldname)
-		die(_("cannot rename the current branch while not on any."));
+	if (!oldname) {
+		if (copy)
+			die(_("cannot copy the current branch while not on any."));
+		else
+			die(_("cannot rename the current branch while not on any."));
+	}
 
 	if (strbuf_check_branch_ref(&oldref, oldname)) {
 		/*
@@ -480,17 +485,33 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 
 	reject_rebase_or_bisect_branch(oldref.buf);
 
-	strbuf_addf(&logmsg, "Branch: renamed %s to %s",
-		 oldref.buf, newref.buf);
+	if (copy)
+		strbuf_addf(&logmsg, "Branch: copied %s to %s",
+			    oldref.buf, newref.buf);
+	else
+		strbuf_addf(&logmsg, "Branch: renamed %s to %s",
+			    oldref.buf, newref.buf);
 
-	if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
+	if (!copy && rename_ref(oldref.buf, newref.buf, logmsg.buf))
 		die(_("Branch rename failed"));
+	if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
+		die(_("Branch copy failed"));
 
-	if (recovery)
-		warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11);
+	if (recovery) {
+		if (copy)
+			warning(_("Copied a misnamed branch '%s' away"),
+				oldref.buf + 11);
+		else
+			warning(_("Renamed a misnamed branch '%s' away"),
+				oldref.buf + 11);
+	}
 
-	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))
-		die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+	if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf)) {
+		if (copy)
+			die(_("Branch copied to %s, but HEAD is not updated!"), newname);
+		else
+			die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
+	}
 
 	strbuf_release(&logmsg);
 
@@ -498,8 +519,10 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	strbuf_release(&oldref);
 	strbuf_addf(&newsection, "branch.%s", newref.buf + 11);
 	strbuf_release(&newref);
-	if (git_config_rename_section(oldsection.buf, newsection.buf) < 0)
+	if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
 		die(_("Branch is renamed, but update of config-file failed"));
+	if (copy && strcmp(oldname, newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
+		die(_("Branch is copied, but update of config-file failed"));
 	strbuf_release(&oldsection);
 	strbuf_release(&newsection);
 }
@@ -537,7 +560,7 @@ static int edit_branch_description(const char *branch_name)
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-	int delete = 0, rename = 0, force = 0, list = 0;
+	int delete = 0, rename = 0, copy = 0, force = 0, list = 0;
 	int reflog = 0, edit_description = 0;
 	int quiet = 0, unset_upstream = 0;
 	const char *new_upstream = NULL;
@@ -574,6 +597,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('D', NULL, &delete, N_("delete branch (even if not merged)"), 2),
 		OPT_BIT('m', "move", &rename, N_("move/rename a branch and its reflog"), 1),
 		OPT_BIT('M', NULL, &rename, N_("move/rename a branch, even if target exists"), 2),
+		OPT_BIT('c', "copy", &copy, N_("copy a branch and its reflog"), 1),
+		OPT_BIT('C', NULL, &copy, N_("copy a branch, even if target exists"), 2),
 		OPT_BOOL(0, "list", &list, N_("list branch names")),
 		OPT_BOOL('l', "create-reflog", &reflog, N_("create the branch's reflog")),
 		OPT_BOOL(0, "edit-description", &edit_description,
@@ -617,14 +642,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
 
-	if (!delete && !rename && !edit_description && !new_upstream && !unset_upstream && argc == 0)
+	if (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)
 		list = 1;
 
 	if (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||
 	    filter.no_commit)
 		list = 1;
 
-	if (!!delete + !!rename + !!new_upstream +
+	if (!!delete + !!rename + !!copy + !!new_upstream +
 	    list + unset_upstream > 1)
 		usage_with_options(builtin_branch_usage, options);
 
@@ -642,6 +667,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	if (force) {
 		delete *= 2;
 		rename *= 2;
+		copy *= 2;
 	}
 
 	if (delete) {
@@ -696,13 +722,22 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (edit_branch_description(branch_name))
 			return 1;
+	} else if (copy) {
+		if (!argc)
+			die(_("branch name required"));
+		else if (argc == 1)
+			copy_or_rename_branch(head, argv[0], 1, copy > 1);
+		else if (argc == 2)
+			copy_or_rename_branch(argv[0], argv[1], 1, copy > 1);
+		else
+			die(_("too many branches for a copy operation"));
 	} else if (rename) {
 		if (!argc)
 			die(_("branch name required"));
 		else if (argc == 1)
-			rename_branch(head, argv[0], rename > 1);
+			copy_or_rename_branch(head, argv[0], 0, rename > 1);
 		else if (argc == 2)
-			rename_branch(argv[0], argv[1], rename > 1);
+			copy_or_rename_branch(argv[0], argv[1], 0, rename > 1);
 		else
 			die(_("too many branches for a rename operation"));
 	} else if (new_upstream) {
diff --git a/cache.h b/cache.h
index 4d92aae..290e39d 100644
--- a/cache.h
+++ b/cache.h
@@ -1941,6 +1941,8 @@ extern int git_config_set_multivar_in_file_gently(const char *, const char *, co
 extern void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
 extern int git_config_rename_section_in_file(const char *, const char *, const char *);
+extern int git_config_copy_section(const char *, const char *);
+extern int git_config_copy_section_in_file(const char *, const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
diff --git a/config.c b/config.c
index 32fd3c8..cd98046 100644
--- a/config.c
+++ b/config.c
@@ -2638,8 +2638,8 @@ static int section_name_is_ok(const char *name)
 }
 
 /* if new_name == NULL, the section is removed instead */
-int git_config_rename_section_in_file(const char *config_filename,
-				      const char *old_name, const char *new_name)
+static int git_config_copy_or_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name, int copy)
 {
 	int ret = 0, remove = 0;
 	char *filename_buf = NULL;
@@ -2648,6 +2648,7 @@ int git_config_rename_section_in_file(const char *config_filename,
 	char buf[1024];
 	FILE *config_file = NULL;
 	struct stat st;
+	struct strbuf copystr = STRBUF_INIT;
 
 	if (new_name && !section_name_is_ok(new_name)) {
 		ret = error("invalid section name: %s", new_name);
@@ -2686,12 +2687,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 	while (fgets(buf, sizeof(buf), config_file)) {
 		int i;
 		int length;
+		int is_section = 0;
 		char *output = buf;
 		for (i = 0; buf[i] && isspace(buf[i]); i++)
 			; /* do nothing */
 		if (buf[i] == '[') {
 			/* it's a section */
-			int offset = section_name_match(&buf[i], old_name);
+			int offset;
+			is_section = 1;
+
+			/*
+			 * When encountering a new section under -c we
+			 * need to flush out any section we're already
+			 * coping and begin anew. There might be
+			 * multiple [branch "$name"] sections.
+			 */ 
+			if (copystr.len > 0) {
+				if (write_in_full(out_fd, copystr.buf, copystr.len) != copystr.len) {
+					ret = write_error(get_lock_file_path(lock));
+					goto out;
+				}
+				strbuf_reset(&copystr);
+			}
+
+			offset = section_name_match(&buf[i], old_name);
 			if (offset > 0) {
 				ret++;
 				if (new_name == NULL) {
@@ -2699,25 +2718,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 					continue;
 				}
 				store.baselen = strlen(new_name);
-				if (!store_write_section(out_fd, new_name)) {
-					ret = write_error(get_lock_file_path(lock));
-					goto out;
-				}
-				/*
-				 * We wrote out the new section, with
-				 * a newline, now skip the old
-				 * section's length
-				 */
-				output += offset + i;
-				if (strlen(output) > 0) {
+				if (!copy) {
+					if (!store_write_section(out_fd, new_name)) {
+						ret = write_error(get_lock_file_path(lock));
+						goto out;
+					}
+
 					/*
-					 * More content means there's
-					 * a declaration to put on the
-					 * next line; indent with a
-					 * tab
+					 * We wrote out the new section, with
+					 * a newline, now skip the old
+					 * section's length
 					 */
-					output -= 1;
-					output[0] = '\t';
+					output += offset + i;
+					if (strlen(output) > 0) {
+						/*
+						 * More content means there's
+						 * a declaration to put on the
+						 * next line; indent with a
+						 * tab
+						 */
+						output -= 1;
+						output[0] = '\t';
+					}
+				} else {
+					copystr = store_create_section(new_name);
 				}
 			}
 			remove = 0;
@@ -2725,11 +2749,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 		if (remove)
 			continue;
 		length = strlen(output);
+
+		if (!is_section && copystr.len > 0) {
+			strbuf_add(&copystr, output, length);
+		}
+
 		if (write_in_full(out_fd, output, length) != length) {
 			ret = write_error(get_lock_file_path(lock));
 			goto out;
 		}
 	}
+
+	/*
+	 * Copy a trailing section at the end of the config, won't be
+	 * flushed by the usual "flush because we have a new section
+	 * logic in the loop above.
+	 */
+	if (copystr.len > 0) {
+		if (write_in_full(out_fd, copystr.buf, copystr.len) != copystr.len) {
+			ret = write_error(get_lock_file_path(lock));
+			goto out;
+		}
+		strbuf_reset(&copystr);
+	}
+
 	fclose(config_file);
 	config_file = NULL;
 commit_and_out:
@@ -2745,11 +2788,30 @@ int git_config_rename_section_in_file(const char *config_filename,
 	return ret;
 }
 
+int git_config_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
+{
+	return git_config_copy_or_rename_section_in_file(config_filename,
+					 old_name, new_name, 0);
+}
+
 int git_config_rename_section(const char *old_name, const char *new_name)
 {
 	return git_config_rename_section_in_file(NULL, old_name, new_name);
 }
 
+int git_config_copy_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
+{
+	return git_config_copy_or_rename_section_in_file(config_filename,
+					 old_name, new_name, 1);
+}
+
+int git_config_copy_section(const char *old_name, const char *new_name)
+{
+	return git_config_copy_section_in_file(NULL, old_name, new_name);
+}
+
 /*
  * Call this to report error for your variable that should not
  * get a boolean value (i.e. "[my] var" means "true").
diff --git a/refs.c b/refs.c
index f0685c9..d724cda 100644
--- a/refs.c
+++ b/refs.c
@@ -2032,3 +2032,14 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 {
 	return refs_rename_ref(get_main_ref_store(), oldref, newref, logmsg);
 }
+
+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg)
+{
+	return refs->be->copy_ref(refs, oldref, newref, logmsg);
+}
+
+int copy_existing_ref(const char *oldref, const char *newref, const char *logmsg)
+{
+	return refs_copy_existing_ref(get_main_ref_store(), oldref, newref, logmsg);
+}
diff --git a/refs.h b/refs.h
index 4be14c4..00fce93 100644
--- a/refs.h
+++ b/refs.h
@@ -440,7 +440,14 @@ char *shorten_unambiguous_ref(const char *refname, int strict);
 /** rename ref, return 0 on success **/
 int refs_rename_ref(struct ref_store *refs, const char *oldref,
 		    const char *newref, const char *logmsg);
-int rename_ref(const char *oldref, const char *newref, const char *logmsg);
+int rename_ref(const char *oldref, const char *newref,
+			const char *logmsg);
+
+/** copy ref, return 0 on success **/
+int refs_copy_existing_ref(struct ref_store *refs, const char *oldref,
+		    const char *newref, const char *logmsg);
+int copy_existing_ref(const char *oldref, const char *newref,
+			const char *logmsg);
 
 int refs_create_symref(struct ref_store *refs, const char *refname,
 		       const char *target, const char *logmsg);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index d8b3f73..fcaf4f7 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1733,9 +1733,9 @@ static int commit_ref_update(struct files_ref_store *refs,
 			     const struct object_id *oid, const char *logmsg,
 			     struct strbuf *err);
 
-static int files_rename_ref(struct ref_store *ref_store,
+static int files_copy_or_rename_ref(struct ref_store *ref_store,
 			    const char *oldrefname, const char *newrefname,
-			    const char *logmsg)
+			    const char *logmsg, int copy)
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "rename_ref");
@@ -1767,8 +1767,12 @@ static int files_rename_ref(struct ref_store *ref_store,
 	}
 
 	if (flag & REF_ISSYMREF) {
-		ret = error("refname %s is a symbolic ref, renaming it is not supported",
-			    oldrefname);
+		if (copy)
+			ret = error("refname %s is a symbolic ref, copying it is not supported",
+				    oldrefname);
+		else
+			ret = error("refname %s is a symbolic ref, renaming it is not supported",
+				    oldrefname);
 		goto out;
 	}
 	if (!refs_rename_ref_available(&refs->base, oldrefname, newrefname)) {
@@ -1776,13 +1780,19 @@ static int files_rename_ref(struct ref_store *ref_store,
 		goto out;
 	}
 
-	if (log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
+	if (!copy && log && rename(sb_oldref.buf, tmp_renamed_log.buf)) {
 		ret = error("unable to move logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
 			    oldrefname, strerror(errno));
 		goto out;
 	}
 
-	if (refs_delete_ref(&refs->base, logmsg, oldrefname,
+	if (copy && log && copy_file(tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
+		ret = error("unable to copy logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
+			    oldrefname, strerror(errno));
+		goto out;
+	}
+
+	if (!copy && refs_delete_ref(&refs->base, logmsg, oldrefname,
 			    orig_oid.hash, REF_NODEREF)) {
 		error("unable to delete old %s", oldrefname);
 		goto rollback;
@@ -1795,7 +1805,7 @@ static int files_rename_ref(struct ref_store *ref_store,
 	 * the safety anyway; we want to delete the reference whatever
 	 * its current value.
 	 */
-	if (!refs_read_ref_full(&refs->base, newrefname,
+	if (!copy && !refs_read_ref_full(&refs->base, newrefname,
 				RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
 				oid.hash, NULL) &&
 	    refs_delete_ref(&refs->base, NULL, newrefname,
@@ -1826,7 +1836,10 @@ static int files_rename_ref(struct ref_store *ref_store,
 	lock = lock_ref_sha1_basic(refs, newrefname, NULL, NULL, NULL,
 				   REF_NODEREF, NULL, &err);
 	if (!lock) {
-		error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
+		if (copy)
+			error("unable to copy '%s' to '%s': %s", oldrefname, newrefname, err.buf);
+		else
+			error("unable to rename '%s' to '%s': %s", oldrefname, newrefname, err.buf);
 		strbuf_release(&err);
 		goto rollback;
 	}
@@ -1877,6 +1890,22 @@ static int files_rename_ref(struct ref_store *ref_store,
 	return ret;
 }
 
+static int files_rename_ref(struct ref_store *ref_store,
+			    const char *oldrefname, const char *newrefname,
+			    const char *logmsg)
+{
+	return files_copy_or_rename_ref(ref_store, oldrefname,
+				 newrefname, logmsg, 0);
+}
+
+static int files_copy_ref(struct ref_store *ref_store,
+			    const char *oldrefname, const char *newrefname,
+			    const char *logmsg)
+{
+	return files_copy_or_rename_ref(ref_store, oldrefname,
+				 newrefname, logmsg, 1);
+}
+
 static int close_ref(struct ref_lock *lock)
 {
 	if (close_lock_file(lock->lk))
@@ -3383,6 +3412,7 @@ struct ref_storage_be refs_be_files = {
 	files_create_symref,
 	files_delete_refs,
 	files_rename_ref,
+	files_copy_ref,
 
 	files_ref_iterator_begin,
 	files_read_raw_ref,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 192f9f8..3c51194 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -543,6 +543,9 @@ typedef int delete_refs_fn(struct ref_store *ref_store, const char *msg,
 typedef int rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
+typedef int copy_ref_fn(struct ref_store *ref_store,
+			  const char *oldref, const char *newref,
+			  const char *logmsg);
 
 /*
  * Iterate over the references in `ref_store` whose names start with
@@ -641,6 +644,7 @@ struct ref_storage_be {
 	create_symref_fn *create_symref;
 	delete_refs_fn *delete_refs;
 	rename_ref_fn *rename_ref;
+	copy_ref_fn *copy_ref;
 
 	ref_iterator_begin_fn *iterator_begin;
 	read_raw_ref_fn *read_raw_ref;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 5eb752c..5d03ad1 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -382,6 +382,221 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'git branch -c dumps usage' '
+	test_expect_code 128 git branch -c 2>err &&
+	test_i18ngrep "branch name required" err
+'
+
+test_expect_success 'git branch --copy dumps usage' '
+	test_expect_code 128 git branch --copy 2>err &&
+	test_i18ngrep "branch name required" err
+'
+
+test_expect_success 'git branch -c d e should work' '
+	git branch -l d &&
+	git reflog exists refs/heads/d &&
+	git config branch.d.dummy Hello &&
+	git branch -c d e &&
+	git reflog exists refs/heads/d &&
+	git reflog exists refs/heads/e &&
+	echo Hello >expect &&
+	git config branch.e.dummy >actual &&
+	test_cmp expect actual &&
+	echo Hello >expect &&
+	git config branch.d.dummy >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git branch --copy is a synonym for -c' '
+	git branch -l copy &&
+	git reflog exists refs/heads/copy &&
+	git config branch.copy.dummy Hello &&
+	git branch --copy copy copy-to &&
+	git reflog exists refs/heads/copy &&
+	git reflog exists refs/heads/copy-to &&
+	echo Hello >expect &&
+	git config branch.copy.dummy >actual &&
+	test_cmp expect actual &&
+	echo Hello >expect &&
+	git config branch.copy-to.dummy >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git branch -c ee ef should copy and checkout branch ef' '
+	git checkout -b ee &&
+	git reflog exists refs/heads/ee &&
+	git config branch.ee.dummy Hello &&
+	git branch -c ee ef &&
+	git reflog exists refs/heads/ee &&
+	git reflog exists refs/heads/ef &&
+	test $(git config branch.ee.dummy) = Hello &&
+	test $(git config branch.ef.dummy) = Hello &&
+	test $(git rev-parse --abbrev-ref HEAD) = ef
+'
+
+test_expect_success 'git branch -c f/f g/g should work' '
+	git branch -l f/f &&
+	git reflog exists refs/heads/f/f &&
+	git config branch.f/f.dummy Hello &&
+	git branch -c f/f g/g &&
+	git reflog exists refs/heads/f/f &&
+	git reflog exists refs/heads/g/g &&
+	test $(git config branch.f/f.dummy) = Hello &&
+	test $(git config branch.g/g.dummy) = Hello
+'
+
+test_expect_success 'git branch -c m2 m2 should work' '
+	git branch -l m2 &&
+	git reflog exists refs/heads/m2 &&
+	git config branch.m2.dummy Hello &&
+	git branch -c m2 m2 &&
+	git reflog exists refs/heads/m2 &&
+	test $(git config branch.m2.dummy) = Hello
+'
+
+test_expect_success 'git branch -c zz zz/zz should fail' '
+	git branch -l zz &&
+	git reflog exists refs/heads/zz &&
+	test_must_fail git branch -c zz zz/zz
+'
+
+test_expect_success 'git branch -c b/b b should fail' '
+	git branch -l b/b &&
+	test_must_fail git branch -c b/b b
+'
+
+test_expect_success 'git branch -C o/q o/p should work when o/p exists' '
+	git branch -l o/q &&
+	git reflog exists refs/heads/o/q &&
+	git reflog exists refs/heads/o/p &&
+	git branch -C o/q o/p
+'
+
+test_expect_success 'git branch -c -f o/q o/p should work when o/p exists' '
+	git reflog exists refs/heads/o/q &&
+	git reflog exists refs/heads/o/p &&
+	git branch -c -f o/q o/p
+'
+
+test_expect_success 'git branch -c qq rr/qq should fail when r exists' '
+	git branch qq &&
+	git branch rr &&
+	test_must_fail git branch -c qq rr/qq
+'
+
+test_expect_success 'git branch -C b1 b2 should fail when b2 is checked out' '
+	git branch b1 &&
+	git checkout -b b2 &&
+	test_must_fail git branch -C b1 b2
+'
+
+test_expect_success 'git branch -C c1 c2 should succeed when c1 is checked out' '
+	git checkout -b c1 &&
+	git branch c2 &&
+	git branch -C c1 c2 &&
+	test $(git rev-parse --abbrev-ref HEAD) = c2
+'
+
+test_expect_success 'git branch -C c1 c2 should add entries to .git/logs/HEAD' '
+	msg="Branch: copied refs/heads/c1 to refs/heads/c2" &&
+	grep "$msg$" .git/logs/HEAD
+'
+
+test_expect_success 'git branch -C master should work when master is checked out' '
+	git checkout master &&
+	git branch -C master
+'
+
+test_expect_success 'git branch -C master master should work when master is checked out' '
+	git checkout master &&
+	git branch -C master master
+'
+
+test_expect_success 'git branch -C master5 master5 should work when master is checked out' '
+	git checkout master &&
+	git branch master5 &&
+	git branch -C master5 master5
+'
+
+test_expect_success 'git branch -C ab cd should overwrite existing config for cd' '
+	git branch -l cd &&
+	git reflog exists refs/heads/cd &&
+	git config branch.cd.dummy CD &&
+	git branch -l ab &&
+	git reflog exists refs/heads/ab &&
+	git config branch.ab.dummy AB &&
+	git branch -C ab cd &&
+	git reflog exists refs/heads/ab &&
+	git reflog exists refs/heads/cd &&
+	test $(git config branch.ab.dummy) = AB &&
+	test $(git config branch.cd.dummy) = AB
+'
+
+test_expect_success 'git branch -c correctly copies multiple config sections' '
+	FOO=1 &&
+	export FOO &&
+	test_when_finished "git checkout master" &&
+	git checkout -b source2 master &&
+
+	# Assert that a config file with multiple config sections has
+	# those sections preserved...
+	cat >expect <<-\EOF &&
+	branch.source2.key1=value1
+	branch.dest2.key1=value1
+	more.gar.b=age
+	branch.source2.key2=value2
+	branch.dest2.key2=value2
+	EOF
+	cat >config.branch <<\EOF &&
+;; Note the lack of -\EOF above & mixed indenting here. This is
+;; intentional, we are also testing that the formatting of copied
+;; sections is preserved.
+
+;; Comment for source2. Tabs
+[branch "source2"]
+	;; Comment for the source2 value
+	key1 = value1
+;; Comment for more.gar. Spaces
+[more "gar"]
+    ;; Comment for the more.gar value
+    b = age
+;; Comment for source2, again. Mixed tabs/spaces.
+[branch "source2"]
+    ;; Comment for the source2 value, again
+	key2 = value2
+EOF
+	cat config.branch >>.git/config &&
+	git branch -c source2 dest2 &&
+	git config -f .git/config -l | grep -F -e source2 -e dest2 -e more.gar >actual &&
+	test_cmp expect actual &&
+
+	# ...and that the comments and formatting for those sections
+	# is also preserved.
+	cat >expect <<\EOF &&
+;; Comment for source2. Tabs
+[branch "source2"]
+	;; Comment for the source2 value
+	key1 = value1
+;; Comment for more.gar. Spaces
+[branch "dest2"]
+	;; Comment for the source2 value
+	key1 = value1
+;; Comment for more.gar. Spaces
+[more "gar"]
+    ;; Comment for the more.gar value
+    b = age
+;; Comment for source2, again. Mixed tabs/spaces.
+[branch "source2"]
+    ;; Comment for the source2 value, again
+	key2 = value2
+[branch "dest2"]
+    ;; Comment for the source2 value, again
+	key2 = value2
+EOF
+	sed -n -e "/Comment for source2/,\$p" .git/config >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'deleting a symref' '
 	git branch target &&
 	git symbolic-ref refs/heads/symref refs/heads/target &&
-- 
2.7.4 (Apple Git-66)

Re: [PATCH v2 1/3] config: create a function to format section headers

[Ramsay Jones]

On 18/06/17 22:16, Sahil Dua wrote:
> Factor out the logic which creates section headers in the config file,
> e.g. the 'branch.foo' key will be turned into '[branch "foo"]'.
> 
> This introduces no function changes, but is needed for a later change
> which adds support for copying branch sections in the config file.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>

Adding my SOB isn't really appropriate here - I only made a very
very minor contribution. At _most_ you might consider 'Helped-by:',
but even that would probably be overkill. ;-)

Thanks.

ATB,
Ramsay Jones


> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
> ---
>  config.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/config.c b/config.c
> index 34a139c..32fd3c8 100644
> --- a/config.c
> +++ b/config.c
> @@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
>  	return 4;
>  }
>  
> -static int store_write_section(int fd, const char *key)
> +static struct strbuf store_create_section(const char *key)
>  {
>  	const char *dot;
> -	int i, success;
> +	int i;
>  	struct strbuf sb = STRBUF_INIT;
>  
>  	dot = memchr(key, '.', store.baselen);
> @@ -2188,6 +2188,15 @@ static int store_write_section(int fd, const char *key)
>  		strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
>  	}
>  
> +	return sb;
> +}
> +
> +static int store_write_section(int fd, const char *key)
> +{
> +	int success;
> +
> +	struct strbuf sb = store_create_section(key);
> +
>  	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
>  	strbuf_release(&sb);
>  
> 

Re: [PATCH v2 1/3] config: create a function to format section headers

[Sahil Dua]

On Mon, Jun 19, 2017 at 2:08 PM, Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
>
>
> On 18/06/17 22:16, Sahil Dua wrote:
>> Factor out the logic which creates section headers in the config file,
>> e.g. the 'branch.foo' key will be turned into '[branch "foo"]'.
>>
>> This introduces no function changes, but is needed for a later change
>> which adds support for copying branch sections in the config file.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>
> Adding my SOB isn't really appropriate here - I only made a very
> very minor contribution. At _most_ you might consider 'Helped-by:',
> but even that would probably be overkill. ;-)

You sent a minor patch and I just applied it and squashed into my
patch. I'm okay with keeping your SOB, unless it's not appropriate to
keep it. (it's my first patch, so advise me. I don't have a strong
preference.)

>
> Thanks.
>
> ATB,
> Ramsay Jones
>
>
>> Signed-off-by: Sahil Dua <sahildua2305@gmail.com>
>> ---
>>  config.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/config.c b/config.c
>> index 34a139c..32fd3c8 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -2169,10 +2169,10 @@ static int write_error(const char *filename)
>>       return 4;
>>  }
>>
>> -static int store_write_section(int fd, const char *key)
>> +static struct strbuf store_create_section(const char *key)
>>  {
>>       const char *dot;
>> -     int i, success;
>> +     int i;
>>       struct strbuf sb = STRBUF_INIT;
>>
>>       dot = memchr(key, '.', store.baselen);
>> @@ -2188,6 +2188,15 @@ static int store_write_section(int fd, const char *key)
>>               strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
>>       }
>>
>> +     return sb;
>> +}
>> +
>> +static int store_write_section(int fd, const char *key)
>> +{
>> +     int success;
>> +
>> +     struct strbuf sb = store_create_section(key);
>> +
>>       success = write_in_full(fd, sb.buf, sb.len) == sb.len;
>>       strbuf_release(&sb);
>>
>>