> 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