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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jun 24, 2010, 20:30 UTC
Message-ID: <20100624203041.GA2262@burratino>
In-Reply-To: <AANLkTinnB_TRYa0nzJHHqbHuJk9a7tfk8mAhr95Sc98E@mail.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.

Show 15 quoted lines

> > +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”.

Show 16 quoted lines

>> 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.

Show 5 quoted lines

>> +#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"

Show 6 quoted lines

>> +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