Re: [PATCH v3] teach fast-export an --anonymize option

From: Jeff King <peff@peff.net>
Date: Aug 31, 2014, 15:53 UTC
Message-ID: <20140831155350.GE17449@peff.net>
In-Reply-To: <CAPig+cSKfxAsr+RQp8N8GAOe9fa_6kGPi_-X99Vs-2bYbBoC8A@mail.gmail.com>

On Sun, Aug 31, 2014 at 06:34:08AM -0400, Eric Sunshine wrote:

Show 12 quoted lines

> >> I feel like this should be part of git-fast-export.txt, just to
> >> increase the user's confidence in the tool (and I don't expect most
> >> users to read this commit message).
> >
> > Hmph. Whenever I say "I think this patch is done", suddenly the comments
> > start pouring in. :)
> 
> Considering that the value of --anonymize is not yet known, is such an
> invasive change to fast-export.c warranted? Would it make sense
> instead to provide "anonymize" functionality as a contrib/ script or a
> distinct git-anonymize-foo command which accepts a fast-import stream
> as input and anonymizes it as output?

I considered that, but there's a non-trivial amount of work in the parsing of the stream (I had originally thought to just ship a perl script to operate on the stream). And while there's a fair bit of code added to fast-export.c, none of it is ever called unless --anonymize is set.

So while I am not 100% sure that the idea is a good one, I do not think it is hurting the current fast-export in any meaningful way. Two things we could do to minimize that are:

  1. Move the anonymization code into a separate C file to keep the
     fast-export source a little more pristine. I avoided doing this
     just because the interfaces to the functions are fairly tailored to
     what fast-export wants.

  2. Have a separate git-anonymize command which is basically running
     "git fast-export --anonymize" under the hood. This avoids polluting
     fast-export from the user's perspective (they do not need to care
     that it is running fast-export under the hood).

-Peff