git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] git-svn: allow to specify svn branch for commands

From
EWEric Wong <normalperson@yhbt.net>
Date
Jun 5, 2007, 09:56 UTC
Message-ID
<20070605095608.GA8215@muzzle>
In-Reply-To
<4664E546.5060304@vilain.net>
Sam Vilain <sam@vilain.net> wrote:
Show 29 quoted lines
> Eric Wong wrote:
> > Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
> >> "git-svn dcommit" ends up making an arbitrary decision when pushing
> >> back merges.  Allow the user to specify which one is used, albeit in a
> >> rather hack-ish way.
> > 
> > Frightening...  Perhaps we should echo the final URL out
> > to the user and prompt them for confirmation.
> 
> Actually in between sending this to the list, I figured that it should
> be possible to detect when this is happening, at the expense of an extra
> "git-log" command.  Basically, take the first revision you found a
> commitlog entry in, and then do a log from the indicated head excluding
> that commit.  If you find more valid tips then the user is merging in
> gitspace.
> 
> Of course ideally you want to make sure that merge commits posted back
> are shipped with all of the necessary tokens for the various SVN-land
> tools out there.  eg, svk:merge, svnmerge, and whatever kooky system the
> SVN dev team come up with.  But that's a separate issue.
> 
> The original patch had a fairly dire bug, so here's a version that at
> least doesn't break the test suite.
> 
> Subject: [PATCH] git-svn: allow to specify svn branch for commands
> 
> "git-svn dcommit" ends up making an arbitrary decision when pushing
> back merges.  Allow the user to specify which one is used, albeit in a
> rather hack-ish way.

Actually, I believe 13c823fb520eaf1cded520213cf0ae4c3268208d already solves this problem. In fact, I'm not certain if you can find a case where this will help if 13c823fb520eaf1cded520213cf0ae4c3268208d is in place. A test case to prove that this is actually needed would be greatly appreciated :)

On the other hand (much to my chagrin), I haven't been working much on git-svn lately and my knowledge could be rusty.

Show 25 quoted lines
> ---
> 
>  Documentation/git-svn.txt |   11 +++++++++++
>  git-svn.perl              |   17 ++++++++++-------
>  2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index c0d7d95..3e64522 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -69,6 +69,17 @@ COMMANDS
>  	argument if that is what you want.  This is useful if
>  	you wish to track multiple projects that share a common
>  	repository.
> +-B<svn_branch>;;
> +--branch=<svn_branch>;;
> +	Normally, git-svn is capable of figuring out which branch you
> +	are working on.  However, if you are doing merges between svn
> +	branches using git then the decision about which branch to
> +	dcommit to will end up being made based on which of the
> +	branches you are merging has the newest upstream commit.  This
> +	option enables a global filter that tells git-svn what to look
> +	for in the git-svn-id: line - specify a repository UUID or a
> +	branch name here.  So, it may be used with "git-svn log",
> +	"git-svn dcommit", etc.
I think you missed my comment here on it being a regexp :)
Show 42 quoted lines
>  'fetch'::
>  	Fetch unfetched revisions from the Subversion remote we are
> diff --git a/git-svn.perl b/git-svn.perl
> index e350061..906aa4b 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -54,7 +54,7 @@ $sha1 = qr/[a-f\d]{40}/;
>  $sha1_short = qr/[a-f\d]{4,40}/;
>  my ($_stdin, $_help, $_edit,
>  	$_message, $_file,
> -	$_template, $_shared,
> +	$_template, $_shared, $_branch,
>  	$_version, $_fetch_all, $_no_rebase,
>  	$_merge, $_strategy, $_dry_run, $_local,
>  	$_prefix, $_no_checkout, $_verbose);
> @@ -69,6 +69,7 @@ my %fc_opts = ( 'follow-parent|follow!' => \$Git::SVN::_follow_parent,
>  		'useSvmProps' => \$Git::SVN::_use_svm_props,
>  		'useSvnsyncProps' => \$Git::SVN::_use_svnsync_props,
>  		'log-window-size=i' => \$Git::SVN::Ra::_log_window_size,
> +		'branch|B=s' => \$_branch,
>  		'no-checkout' => \$_no_checkout,
>  		'quiet|q' => \$_q,
>  		'repack-flags|repack-args|repack-opts=s' =>
> @@ -367,7 +368,7 @@ sub cmd_dcommit {
>  	my $head = shift;
>  	$head ||= 'HEAD';
>  	my @refs;
> -	my ($url, $rev, $uuid, $gs) = working_head_info($head, \@refs);
> +	my ($url, $rev, $uuid, $gs) = working_head_info($head, \@refs, $_branch);
>  	unless ($gs) {
>  		die "Unable to determine upstream SVN information from ",
>  		    "$head history\n";
> @@ -441,7 +442,7 @@ sub cmd_find_rev {
>  		my $head = shift;
>  		$head ||= 'HEAD';
>  		my @refs;
> -		my (undef, undef, undef, $gs) = working_head_info($head, \@refs);
> +		my (undef, undef, undef, $gs) = working_head_info($head, \@refs, $_branch);
>  		unless ($gs) {
>  			die "Unable to determine upstream SVN information from ",
>  			    "$head history\n";
> @@ -457,7 +458,7 @@ sub cmd_find_rev {
  
I don't think you need this hunk.  You can already pass the remote
branch name here (same with "git svn log").
Show 18 quoted lines
>  sub cmd_rebase {
>  	command_noisy(qw/update-index --refresh/);
> -	my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
> +	my ($url, $rev, $uuid, $gs) = working_head_info('HEAD', undef, $_branch);
>  	unless ($gs) {
>  		die "Unable to determine upstream SVN information from ",
>  		    "working tree history\n";
> @@ -474,7 +475,7 @@ sub cmd_rebase {
>  }
>  
>  sub cmd_show_ignore {
> -	my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
> +	my ($url, $rev, $uuid, $gs) = working_head_info('HEAD', undef, $_branch);
>  	$gs ||= Git::SVN->new;
>  	my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum);
>  	$gs->traverse_ignore(\*STDOUT, $gs->{path}, $r);
> @@ -801,12 +802,14 @@ sub cmt_metadata {
>  }

You need to put --branch/-B into %cmd directly, %fc_opts won't cover this ('fc' => fetch/commit). On the other hand, we should probably make this command take a $head from the command-line like "git svn log" and "git svn find-rev"...

Show 13 quoted lines
>  sub working_head_info {
> -	my ($head, $refs) = @_;
> +	my ($head, $refs, $grep) = @_;
>  	my ($fh, $ctx) = command_output_pipe('rev-list', $head);
>  	while (my $hash = <$fh>) {
>  		chomp($hash);
>  		my ($url, $rev, $uuid) = cmt_metadata($hash);
>  		if (defined $url && defined $rev) {
> +			next unless (!$grep or
> +				$url =~ m{$grep} or $uuid =~ m{$grep});
>  			if (my $gs = Git::SVN->find_by_url($url)) {
>  				my $c = $gs->rev_db_get($rev);
>  				if ($c && $c eq $hash) {

This last 'if' statement here checking the rev_db (introduced in 13c823fb520eaf1cded520213cf0ae4c3268208d still allows what this patch is intended to do?

Show 9 quoted lines
> @@ -3394,7 +3397,7 @@ sub git_svn_log_cmd {
>  		last;
>  	}
>  
> -	my ($url, $rev, $uuid, $gs) = ::working_head_info($head);
> +	my ($url, $rev, $uuid, $gs) = ::working_head_info($head, undef, $_branch);
>  	$gs ||= Git::SVN->_new;
>  	my @cmd = (qw/log --abbrev-commit --pretty=raw --default/,
>  	           $gs->refname);

You don't need this last hunk. git svn log already takes remote branches as an argument, and I don't think the -B switch is passed here where you put it in %fc_opts.

-- 
Eric Wong
Previous: Sam VilainNext: Stephen Touset
Message 4 of 6 in “git-svn: allow to specify svn branch for commands”
  1. git-svn: allow to specify svn branch for commandsSam Vilain, Jun 5, 2007
  2. Eric WongJun 5, 2007
  3. Sam VilainJun 5, 2007
  4. Eric WongJun 5, 2007
  5. Stephen TousetJun 5, 2007
  6. Eric WongJun 5, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.