Re: [PATCH 9/9] Add a sample user for the svndump library

[Jonathan Nieder <jrnieder@gmail.com>]

Ramkumar Ramachandra wrote:
> Jonathan Nieder wrote:

>> NEEDSWORK: litters cwd with useless .bin files.
>> But I hope it is enough to show the idea.
>
> How do you propose we solve this?

Turn off persistence until it is ready.  At that point, we will need
to access the target git repo anyway, so we can keep extra metadata in
the .git directory.

> > +QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir
> > +QUIET_SUBDIR1 =
> 
> > +ifneq ($(findstring $(MAKEFLAGS),s),s)
> > +ifndef V
> > +       QUIET_CC      = @echo '   ' CC $@;
> > +       QUIET_LINK    = @echo '   ' LINK $@;
> > +       QUIET_SUBDIR0 = +@subdir=
> > +       QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
> > +                       $(MAKE) $(PRINT_DIR) -C $$subdir
> > +endif
> > +endif
> 
> I saw this in the Git Makefile too, but I didn't understand the logic
> behind it. Could you explain it to me?

See commit 74f2b2a.

Summary: this produces the

    CC foo.o

lines.  The idea is that long command lines distract from what is more
important, which is the compiler output.  The behavior can be turned
off with “make V=1” or “make -s”.

>> diff --git a/contrib/svn-fe/svn-fe.c b/contrib/svn-fe/svn-fe.c
>> new file mode 100644
>> index 0000000..d84dd4f
>> --- /dev/null
>> +++ b/contrib/svn-fe/svn-fe.c
>> @@ -0,0 +1,43 @@
>> +/*
>> + * Parse and rearrange a svnadmin dump.
>> + * Create the dump with:
>> + * svnadmin dump --incremental -r<startrev>:<endrev> <repository> >outfile
>> + *
>> + * Copyright (C) 2010 David Barr <david.barr@cordelta.com>.
>> + * All rights reserved.
>
> That's a huge license header applies just to the trivial five-line
> program, right? Is it necessary at all?

I dunno.  I included the license header instead of refering to LICENSE
because this file tends to be installed in /usr/share/doc/git/contrib
and LICENSE does not.

Maybe the file should get a simpler license?  e.g.:

 This file is in the public domain.
 You may freely use, modify, distribute, and relicense it.

>> +#include <stdlib.h>
>> +#include "../../vcs-svn/svndump.h"
>
> Inelegant. Why not include ../../vcs-svn in the path you're searching
> for headers?

Right, this should be changed to

 #include <stdlib.h>
 #include "vcs-svn/svndump.h"

>> +svnadmin dump --incremental REPO | svn-fe [url] | git fast-import
>
> If the user doesn't have a clue about SVN, they won't know what REPO
> is here: Without knowing anything about svnadmin, I'd naively try it
> with a remote repository. Maybe include a note about having to mirror
> a complete repository locally using svnsync (or otherwise) first?

Sounds reasonable.  Care to suggest wording?

>> +Converts a textual representation of a Subversion repository into
>> +input suitable for git-fast-import(1) and similar importers.
>
> To be more specific, "Subversion dumpfile (version: 2)" from FILE(1).

Do version 3 dumpfiles fail?

Jonathan