cvsexportcommit dies when applying an (empty) merge commit

cvsexportcommit dies when applying an (empty) merge commit

[Nick Woolley]

Hi,

I have a git repository with a merge point on the master branch.  This merge
commit is empty, and just contains a commit message:

  Merge commit 'otherbranch'

I'm trying to export this branch into CVS using git-cvsexportcommit (the latest
version from the master branch). It's actually done in a wrapper script [1] but
the command that gets invoked is essentially:

 git cvsexportcommit -p -v -u -w  'cvscheckout/HEAD/my-cvs-module' -c \
    <parent commit> <commit>

Where <commit> is the empty merge commit.  However this invocation dies and
aborts the process of exporting the branch half way.

The fatal error I get is:

 Applying to CVS commit <commit> from parent <parent commit>
 Checking if patch will apply
 Applying
 error: No changes
 cannot patch at /usr/lib/git-core/git-cvsexportcommit line 324.

The vicinity of line 324 is (with some lines wrapped):

 print "Applying\n";
 if ($opt_W) {
     system("git checkout -q $commit^0") && die "cannot patch";
 } else {
     `GIT_DIR= git-apply $context --summary --numstat --apply
<.cvsexportcommit.diff` || die "cannot patch";
 }

It seems that the file .cvsexportcommit.diff is empty, so git-apply is refusing
to apply it.

Presumably the application would be a no-op, so this git-apply step could be
skipped.  So I tried modifying the script to do that and it seems to work:

 print "Applying\n";
 if ($opt_W) {
     system("git checkout -q $commit^0") && die "cannot patch";
 } elsif (-s ".cvsexportcommit.diff") {
     `GIT_DIR= git-apply $context --summary --numstat --apply
<.cvsexportcommit.diff` || die "cannot patch";
 } else {
    print "No changes\n";
 }

The modified git-cvsexportcommit script completes without errors, but
unsurprisingly, seems to export nothing, so that when imported back into git,
there is no empty commit.  There appears to be no log message added in CVS, either.

This does seem more acceptable than dying, although it doesn't faithfully
reproduce the git history.  However I'm not sure if that would be possible in
this case.

Is the existing behaviour deliberately fatal, or is this worth supplying a patch
for?


Cheers,

N

1. http://github.com/wu-lee/git-cvs

Re: cvsexportcommit dies when applying an (empty) merge commit

[Michael J Gruber]

Nick Woolley venit, vidit, dixit 25.11.2009 12:59:
> Hi,
> 
> I have a git repository with a merge point on the master branch.  This merge
> commit is empty, and just contains a commit message:
> 
>   Merge commit 'otherbranch'
> 
> I'm trying to export this branch into CVS using git-cvsexportcommit (the latest
> version from the master branch). It's actually done in a wrapper script [1] but
> the command that gets invoked is essentially:
> 
>  git cvsexportcommit -p -v -u -w  'cvscheckout/HEAD/my-cvs-module' -c \
>     <parent commit> <commit>
> 
> Where <commit> is the empty merge commit.  However this invocation dies and
> aborts the process of exporting the branch half way.
> 
> The fatal error I get is:
> 
>  Applying to CVS commit <commit> from parent <parent commit>
>  Checking if patch will apply
>  Applying
>  error: No changes
>  cannot patch at /usr/lib/git-core/git-cvsexportcommit line 324.
> 
> The vicinity of line 324 is (with some lines wrapped):
> 
>  print "Applying\n";
>  if ($opt_W) {
>      system("git checkout -q $commit^0") && die "cannot patch";
>  } else {
>      `GIT_DIR= git-apply $context --summary --numstat --apply
> <.cvsexportcommit.diff` || die "cannot patch";
>  }
> 
> It seems that the file .cvsexportcommit.diff is empty, so git-apply is refusing
> to apply it.
> 
> Presumably the application would be a no-op, so this git-apply step could be
> skipped.  So I tried modifying the script to do that and it seems to work:
> 
>  print "Applying\n";
>  if ($opt_W) {
>      system("git checkout -q $commit^0") && die "cannot patch";
>  } elsif (-s ".cvsexportcommit.diff") {
>      `GIT_DIR= git-apply $context --summary --numstat --apply
> <.cvsexportcommit.diff` || die "cannot patch";
>  } else {
>     print "No changes\n";
>  }
> 
> The modified git-cvsexportcommit script completes without errors, but
> unsurprisingly, seems to export nothing, so that when imported back into git,
> there is no empty commit.  There appears to be no log message added in CVS, either.
> 
> This does seem more acceptable than dying, although it doesn't faithfully
> reproduce the git history.  However I'm not sure if that would be possible in
> this case.
> 
> Is the existing behaviour deliberately fatal, or is this worth supplying a patch
> for?

I think the behavior is correct in the sense that you're telling git
cvsexportcommit to make a commit to cvs, and it can't, because there is
no change to commit. A merge can't be represented.

It leaves you the choice between omitting the trivial merge from cvs
history (that's what one would do for a merge based cvs<->git workflow)
and generating a fake commit in cvs. I don't know if it has something
like --allow-empty - it's file base, after all.

Michael

Re: cvsexportcommit dies when applying an (empty) merge commit

[Nick Woolley]

Michael J Gruber wrote:
> I think the behavior is correct in the sense that you're telling git
> cvsexportcommit to make a commit to cvs, and it can't, because there is
> no change to commit. A merge can't be represented.

The main problem with this is that by default it aborts the whole process with
an error.  I'd prefer a non-fatal warning rather than having to catch the error
and decide if it's ignorable or not, although for now I guess I'll have to, or
make a fork of git-cvsexportcommit.

CVS doesn't has an --allow-empty equivalent I know of, and nor does
git-cvsexportcommit.

Cheers,

N

Re: cvsexportcommit dies when applying an (empty) merge commit

[Robin Rosenberg]

onsdag 25 november 2009 12:59:22 skrev  Nick Woolley:
> Hi,
>
> I have a git repository with a merge point on the master branch.  This
> merge commit is empty, and just contains a commit message:
>
>   Merge commit 'otherbranch'
>
> I'm trying to export this branch into CVS using git-cvsexportcommit (the
> latest version from the master branch). It's actually done in a wrapper
> script [1] but the command that gets invoked is essentially:
>
>  git cvsexportcommit -p -v -u -w  'cvscheckout/HEAD/my-cvs-module' -c \
>     <parent commit> <commit>
>
> Where <commit> is the empty merge commit.  However this invocation dies and
> aborts the process of exporting the branch half way.
>
> The fatal error I get is:
>
>  Applying to CVS commit <commit> from parent <parent commit>
>  Checking if patch will apply
>  Applying
>  error: No changes
>  cannot patch at /usr/lib/git-core/git-cvsexportcommit line 324.
>
[....]
> Is the existing behaviour deliberately fatal, or is this worth supplying a
> patch for?

I'm not the only contributor, but I'd say its a omission. cvsexportcommit 
doesn't export commits. It export deltas, that is the change relative to one
of the parents. It is reasonable that cvsexportcommit can "export" an
empty commit by doing nothing and exiting with 0. Printing some kind of 
warning seems reasonable too.

- robin