[PATCH] Teach cvsexportcommit to add new files

[PATCH] Teach cvsexportcommit to add new files

[Yann Dirson]

Teach cvsexportcommit to add new files
"cvs add" support was already there, but the "unknown" status
returned when querying a file not yet known to cvs caused the
script to abort prematurely.

---
commit 7e1b8a9af653c9a4f8d2a6a4b34ccf56a2f48526
tree f94f057ec672b715ffda28e359bd5d672fdfb6b3
parent 58e3fb40f7ca1c28f9105c15166884f80bb22e55
author Yann Dirson <ydirson@altern.org> Fri, 06 Jan 2006 21:31:21 +0100
committer Yann Dirson <dwitch@gandelf.nowhere.earth> Fri, 06 Jan 2006 21:31:21 +0100

 git-cvsexportcommit.perl |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index 5a8c011..d5fbee7 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -103,8 +103,16 @@ undef @files; # don't need it anymore
 
 # check that the files are clean and up to date according to cvs
 my $dirty;
-foreach my $f (@afiles, @mfiles, @dfiles) {
-    # TODO:we need to handle removed in cvs and/or new (from git) 
+foreach my $f (@afiles) {
+    my $status = `cvs -q status "$f" | grep '^File: '`;
+
+    unless ($status =~ m/Status: Unknown$/) {
+	$dirty = 1;
+	warn "File $f is already known in your CVS checkout!\n";
+    }
+}
+foreach my $f (@mfiles, @dfiles) {
+    # TODO:we need to handle removed in cvs
     my $status = `cvs -q status "$f" | grep '^File: '`;
 
     unless ($status =~ m/Status: Up-to-date$/) {

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

Re: [PATCH] Teach cvsexportcommit to add new files

[Junio C Hamano]

Yann Dirson <ydirson@altern.org> writes:

> Teach cvsexportcommit to add new files
> "cvs add" support was already there, but the "unknown" status
> returned when querying a file not yet known to cvs caused the
> script to abort prematurely.

Thanks.  I'll take the patch as is, but I noticed that the
cvsexportcommit still has some room for polishing.

 * The command is to be run from CVS working copy (as desribed
   in the documentation) and seems to assume that the parent
   commit is checked out at least for the paths involved between
   the commit and the parent (not clearly documented).

 * I wonder if it is safe with filenames with embedded SP?

 * This undef @files seem premature; later you will say
   cleanupcvs(@files) won't you?

> $opt_v && print "The commit affects:\n ";
> $opt_v && print join ("\n ", @afiles,@mfiles,@dfiles) . "\n\n";
> undef @files; # don't need it anymore
> ...
> ### NOTE: if you are planning to die() past this point
> ###       you MUST call cleanupcvs(@files) before die()

 * I think this grep '^Binary' is unsafe; different versions of
   gnu diff say just "Files differ".

> print "'Patching' binary files\n";
> 
> my @bfiles = `git-diff-tree -p $parent $commit | grep '^Binary'`;

 * ls-tree can take $parent (a commit which is by definition a
   treeish) happily here, so rev-parse is not needed.

>     my $tree = `git-rev-parse $parent^{tree} `;
>     chomp $tree;
>     my $blob = `git-ls-tree $tree "$f" | cut -f 1 | cut -d ' ' -f 3`;
>     chomp $blob;

 * another way would be git-hash-object (without -w) the working
   tree file (which you already know matches the $parent
   version) and compare the hash with $blob without extracting
   it with cat-file.

>     `git-cat-file blob $blob > $tmpdir/blob`;
>     `cmp -q $f $tmpdir/blob`;

I am getting an impression that _if_ we care about applying
fuzzy patch like this, giving fuzz feature to git-apply would
help this program somewhat, but at the same time I do not think
of a good reason to even accept nonexact patch when synching two
SCM histories.

Re: [PATCH] Teach cvsexportcommit to add new files

[Martin Langhoff]

On 1/7/06, Junio C Hamano <junkio@cox.net> wrote:
> Yann Dirson <ydirson@altern.org> writes:
>
> > Teach cvsexportcommit to add new files
> > "cvs add" support was already there, but the "unknown" status
> > returned when querying a file not yet known to cvs caused the
> > script to abort prematurely.
>
> Thanks.  I'll take the patch as is, but I noticed that the
> cvsexportcommit still has some room for polishing.

Definitely. It's a bit of a hack job that I got started with, and as
it's /worked for me/ so far, haven't done much with it. I am actually
quite busy at the moment, so I don't think you'll see many patches
from me on this fron yet...

(BTW: I'm hoping to find a bit of time to code the git-cvsdaemon that
I've mentioned before, which would be a hell of an interesting
addition, methinks.)

>  * The command is to be run from CVS working copy (as desribed
>    in the documentation) and seems to assume that the parent
>    commit is checked out at least for the paths involved between
>    the commit and the parent (not clearly documented).

Indeed, and you can also use it for the same thing you'd use git-am --
for instance, to "merge into CVS" feature branches kept on GIT.

>  * I wonder if it is safe with filenames with embedded SP?

It's not -- shame on me. Backticks should be replaced with
safe_pipe_capture() as Eric's done on git-archimport.

>  * I think this grep '^Binary' is unsafe; different versions of
>    gnu diff say just "Files differ".

Unsure how to detect binary files safely. Is there a way to ask git
whether it considers the files to be binary?

(snipped several good suggestions here...)

> I am getting an impression that _if_ we care about applying
> fuzzy patch like this, giving fuzz feature to git-apply would
> help this program somewhat, but at the same time I do not think
> of a good reason to even accept nonexact patch when synching two
> SCM histories.

Well, with some fuzz you can use this to merge something into CVS even
if the trunk or branch you're merging into has moved forward a bit.
OTOH, I guess you can go back to git and do a git-rebase if needed.

cheers,


martin

cvsexportcommit / cvsimport issues

[Yann Dirson]

On Sat, Jan 07, 2006 at 08:29:08PM +1300, Martin Langhoff wrote:
> Definitely. It's a bit of a hack job that I got started with, and as
> it's /worked for me/ so far, haven't done much with it. I am actually
> quite busy at the moment, so I don't think you'll see many patches
> from me on this fron yet...

In any case, I'm likely to send occasional patches as I hit the
limitations.

> Indeed, and you can also use it for the same thing you'd use git-am --
> for instance, to "merge into CVS" feature branches kept on GIT.

That's also the way I use it.  We have an official CVS repo, and I'm
doing all of my work in git, until it's ready for publication in the
central CVS repo.



While we're talking about cvs<->git, I also hit a couple of problems
with git-cvsimport, which I did not have time to fully investigate:

- when there are very large files in the CVS repo (eg. a 85MB
tarball), cvsimport timeouts while fetching it (saying the server has
gone away), when accesing via pserver or ssh.  I tried to play with
the socket timeout, but also it helped to reproducibly import a 15MB
file, I could not raise it enough to get the 85MB file.

Hunting for this problem, I found myself asking the question, why it
was chosen in the first place to implement a cvs client, instead of
using the reference implementation.

I was finally able to workaround this by transfering a copy of the
cvsroot to the local machine, but that's not quite practical :)


- the "-k" flag is dangerous as-is, since it overrides any flag
(eg. -kb) set on the files in the cvs repo.  I got a plain
(non-compressed) tarball of sources corrupted because of this, and was
subsequently unable to use -k at all.  Fortunately, I had done the
original cvs imports using -ko, so it was not a problem for me, but we
may want to fix the issue eventually, and at least document this
caveat.


- when doing an incremental import using -P, where the 1st patchset
was just a module creation with a single .cvsignore to allow creation
of a pseudo-vendor-branch, and the 2nd patchset was an import on the
said branch, cvsimport complained that it could not find the branch in
my git repo, and I had to create it manually, whereas while importing
all patchsets in one batch, the branch was created without manual
intervention (but import subsequently failed because of the big file
as mentionned above).  Similarly, still when using -P, using the local
repository copy, importing the initial patchset complained 'cannot
find "origin"' - I prefered to do this initial import using the
pserver access, which worked fine, and did the later imports via the
local repo.

We could thus want to mention in the doc that -P can also be used to
help circumvent problems by using for each patchset the most adequate
import method :)


- as of cvsps 2.1, patchsets only contain a single tag, even when
there are several of them on the cvs repo.  We should surely document
this shortcoming so people do not get caught by surprise.


Best regards,
-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>