Marcin Owsiany <mar...@owsiany.pl> writes:
> Date: Sun, 24 Jun 2012 22:40:05 +0100
> Subject: [PATCH] git-svn: don't create master if another head exists
>
> git-svn insists on creating the "master" head (unless it exists) on every
> "fetch". It is useful that it gets created initially, when no head exists
> - users expect this git convention of having a "master" branch on initial
> clone.
>
> However creating it when there already is another head does not provide any
> value - the ref is never updated, so it just gets stale after a while. Also,
> some users find it annoying that it gets recreated, especially when they would
> like the git branch names to follow SVN repository branch names. More
> background in http://thread.gmane.org/gmane.comp.version-control.git/115030
>
> Make git-svn skip the "master" creation if HEAD points at a valid head. This
> means "master" does get created on initial "clone" but does not get recreated
> once a user deletes it.
The above description makes sense to me, but the code updated with
this patch doesn't quite make sense to me.
This is your patch with a bit wider context.
> diff --git a/git-svn.perl b/git-svn.perl
> index 0b074c4..2379a71 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1599,35 +1599,35 @@ sub rebase_cmd {
> sub post_fetch_checkout {
> return if $_no_checkout;
> my $gs = $Git::SVN::_head or return;
> return if verify_ref('refs/heads/master^0');
Does "master" matter here?
I am wondering why this is not
return if verify_ref("HEAD^0");
Moreover, since the code will check verify_ref("HEAD^0") anyway in
the place you updated, is this early return still necessary?
> # look for "trunk" ref if it exists
> my $remote = Git::SVN::read_all_remotes()->{$gs->{repo_id}};
> my $fetch = $remote->{fetch};
> if ($fetch) {
> foreach my $p (keys %$fetch) {
> basename($fetch->{$p}) eq 'trunk' or next;
> $gs = Git::SVN->new($fetch->{$p}, $gs->{repo_id}, $p);
> last;
> }
> }
>
> - my $valid_head = verify_ref('HEAD^0');
> + return if verify_ref('HEAD^0');
This one matches the description. When post_fetch_checkout() is
called, if HEAD is already pointing at a valid commit, we do not
want to run checkout (or create a ref).
> command_noisy(qw(update-ref refs/heads/master), $gs->refname);
> - return if ($valid_head || !verify_ref('HEAD^0'));
> + return unless verify_ref('HEAD^0');
I do not understand these three lines. Why aren't they like this?
command_noisy(qw(update-ref HEAD), $gs->refname) || return;
That is, in a fresh repository whose HEAD points at an unborn
'master', nothing changes from the current behaviour. If a fresh
repository whose HEAD points at some other unborn branch, should the
code still want to update 'master'? Wouldn't we rather want to
update that branch?
If the caller does not handle errors, it could be even clearer to
write it like
command_noisy(qw(update-ref HEAD), $gs->refname) ||
die "Cannot update HEAD!!!";
> return if $ENV{GIT_DIR} !~ m#^(?:.*/)?\.git$#;
> my $index = $ENV{GIT_INDEX_FILE} || "$ENV{GIT_DIR}/index";
> return if -f $index;
>
> return if command_oneline(qw/rev-parse --is-inside-work-tree/) eq
> 'false';
> return if command_oneline(qw/rev-parse --is-inside-git-dir/) eq
> 'true';
> command_noisy(qw/read-tree -m -u -v HEAD HEAD/);
> print STDERR "Checked out HEAD:\n ",
> $gs->full_url, " r", $gs->last_rev, "\n";
> if (auto_create_empty_directories($gs)) {
> $gs->mkemptydirs($gs->last_rev);
> }
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
