You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Igor Sereda <se...@almworks.com> on 2010/07/08 13:32:45 UTC
Re: [PATCH] fix for svndumpfilter not filtering out revisions that
were initially empty
> 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 <cm...@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 <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
>
Re: [PATCH] fix for svndumpfilter not filtering out revisions that
were initially empty
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 07/08/2010 09:32 AM, Igor Sereda wrote:
>> 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 /".
Heheh. "Could have been" is a luxury we can always entertain but rarely do
anything about.
A new option for the behavior you propose could be as simple as
--drop-all-empty-revs.
> 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. :)
Yeah, that may be true. But as you well know, our users will be banking on
the user-visible documentation and current behavior more so than our code
comments. :-)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand