Subject: Re: [PATCH] Add git-svn branch to allow branch creation in SVN repositories

From: Eric Wong <[email-masked]>
Date: Sat, 6 Sep 2008 19:33:16 -0700

Hi Florian,

Cool.  Some people have been asking for this feature.

I never implemented it myself because I've been unsure if it's
philosophically proper.  Ut's functionally redundant as svn(1) does the
same thing w/o needing a working copy.  But I'll accept your patch with
some minor modifications (comments inline).

Florian Ragwitz <[email-masked]> wrote:
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -149,6 +149,18 @@ and have no uncommitted changes.
>  	is very strongly discouraged.
>  --
>  
> +'branch'::
> +    Create a branch in the SVN repository.

Perhaps a "tag" command that is short for "branch -t" would
be helpful, too.

> +-t;;
> +--tag;;
> +    Create a tag by using the tags_subdir instead of the branches_subdir
> +    specified during git svn init.
> +
> diff --git a/git-svn.perl b/git-svn.perl
> index 7a1d26d..55a2052 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -537,6 +541,30 @@ sub cmd_dcommit {
>  	unlink $gs->{index};
>  }

Coding style things:

1. Don't pad declarations with extra whitespace.
2. No lines over 80 characters (assuming tabs are 8 chars wide)

> +sub cmd_branch {
> +	my $branch_name = shift or die "branch name required\n";
> +	my $head        = shift || 'HEAD';
> +
> +	my ($src, $rev, undef, $gs) = working_head_info($head);
> +
> +	my $remote      = Git::SVN::read_all_remotes()->{svn};

> +	my ($lft, $rgt) = @{ $remote->{ $_tag ? 'tags' : 'branches' }->{path} }{qw/left right/};
> +	my $dst         = join '/', $remote->{url}, $lft, $branch_name, ($rgt || ());

This doesn't appear to fail gracefully for SVN repositories
that don't follow the standard trunk, branches, tags layout.

Can you please ensure that it does and print an error message?

> +
> +	my $ctx = SVN::Client->new(
> +		auth    => Git::SVN::Ra::_auth_providers(),
> +		log_msg => sub { ${ $_[0] } = $_message || 'Create branch ' . $branch_name },
> +	);
> +
> +	eval {
> +		$ctx->ls($dst, 'HEAD', 0);
> +	} and die "branch ${branch_name} already exists\n";

Can you print the URLs being copied and add a --dry-run/-n
option (like dcommit).

> +	$ctx->copy($src, $rev, $dst);
> +
> +	$gs->fetch_all;
> +}
> +
>  sub cmd_find_rev {
>  	my $revision_or_hash = shift or die "SVN or git revision required ",
>  	                                    "as a command-line argument\n";

Can you also add a simple test case so we don't break it in the future?
Thanks.

-- 
Eric Wong