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

From: Eric Wong <normalperson@yhbt.net>
Date: Sep 7, 2008, 02:33 UTC
Message-ID: <20080907023316.GA12392@untitled>
In-Reply-To: <1220614447-3751-1-git-send-email-rafl@debian.org>

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 <rafl@debian.org> wrote:

Show 8 quoted lines

> --- 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.

Show 12 quoted lines

> +-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)

Show 7 quoted lines

> +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?

Show 9 quoted lines

> +
> +	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).

Show 8 quoted lines

> +	$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