[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] fix for svndumpfilter not filtering out revisions that were initially empty

From: Igor Sereda <sereda_at_almworks.com>
Date: Thu, 8 Jul 2010 17:32:45 +0400

> I like the idea of this change, but I wonder if it can be made without
> introducing a new command-line option.  Your "expectations" as listed above
> certainly make sense.  That is, until you actually read the built-in
> documentation found in the program's usage message.  :-)

I see what you mean. Some thoughts:

1. I think "--drop-empty-revs" could have been something like
"--filter-revisions" instead, which would mean, keep revision if and
only if at least one node passes include/exclude filter. In that case,
empty revisions could be filtered by "include --filter-revisions /".

2. Initially I got a dump with empty revisions with svnsync, by
pulling a subdirectory of a repo. Probably svnsync could have an
option to drop empty revs. (But then what about rev numbers?)

3. While indeed help message for --drop-empty-revs matches the
behavior, the *comment* in the current code does not:

  /* Revision is written out in the following cases:
     1. No --drop-empty-revs has been supplied.
     2. --drop-empty-revs has been supplied,
      but revision has not all nodes dropped
     3. Revision had no nodes to begin with.
   */

Obviously, the guy who wrote the comment expected the same behavior as
I did, but the resulting code was a bit different. :)

Cheers,
Igor

On Thu, Jul 8, 2010 at 4:25 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> On 07/04/2010 12:08 PM, Igor Sereda wrote:
>> [[[
>> Fix for svndumpfilter.
>>
>>   * subversion/svndumpfilter/main.c
>>
>> Problem Reproduction Sequence:
>>
>>   1. Get a dump of a repository with empty revisions (for example, by
>> pulling a subdirectory of a remote repo with svnsync).
>>   2. Run svndumpfilter --drop-empty-revs to filter the dump.
>>
>> Expected:
>>
>>   Revisions that were initially empty are dropped. (As well as those
>> filtered out.)
>>
>> Observed:
>>
>>   Only revisions that were filtered out are dropped. Revisions that
>> were initially empty - remain.
>
> I like the idea of this change, but I wonder if it can be made without
> introducing a new command-line option.  Your "expectations" as listed above
> certainly make sense.  That is, until you actually read the built-in
> documentation found in the program's usage message.  :-)
>
> The --drop-empty-revs option has always been documented as doing exactly
> what it does today -- only dropping revs that were made empty by the
> filtering process:
>
>   $ svndumpfilter help include
>   include: Filter out nodes without given prefixes from dumpstream.
>   usage: svndumpfilter include PATH_PREFIX...
>
>   Valid options:
>     --drop-empty-revs        : Remove revisions emptied by filtering.
>   [...]
>
> To change that behavior now could arguably be considered a violation of our
> compatibility promises.
>
> What do others think?
>
>> The following patch fixes that.
>
> (In case it wasn't clear, my lack of comments about the patch don't
> constitute silent assent.  I've not yet reviewed the details of the change
> for accuracy.)
>
> --
> C. Michael Pilato <cmpilato_at_collab.net>
> CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
>
>
Received on 2010-07-08 15:33:29 CEST

This is an archived mail posted to the Subversion Dev mailing list.