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

Re: [PATCH] Re: Regarding issue 3690 - Work in progress

From: Noorul Islam K M <noorul_at_collab.net>
Date: Tue, 01 Feb 2011 10:22:22 +0530

Hyrum K Wright <hyrum_at_hyrumwright.org> writes:

> On Mon, Jan 31, 2011 at 10:53 AM, Noorul Islam K M <noorul_at_collab.net> wrote:
>
>> Daniel Becroft <djcbecroft_at_gmail.com> writes:
>>
>>> On 27/01/2011, at 17:04, Noorul Islam K M <noorul_at_collab.net> wrote:
>>>
>>>> Hi,
>>>>
>>>> I am planning to work on issue 3690. Before starting with this I have
>>>> few questions.
>>>>
>>>> 1. Hyrum updated the issue with his comment stating that already there
>>>>   is work going on in the branch ignore-mergeinfo which addresses
>>>>   subset of issue 3690, i.e ignoring changes to svn:mergeinfo. Is
>>>>   svn:mergeinfo an svn property set using svn pset command? I think Zvi
>>>>   Rackover is talking about a new option passing which a user can
>>>>   ignore revisions with just the following property changes alone.
>>>>
>>>>   author      eol-style   externals   keywords    mime-type
>>>>   date        executable  ignore      log         needs-lock
>>>
>>> Fyi, svn:author, svn:date and svn:log are revision properties - changes to these don't appear in the log.
>>>
>>> Cheers,
>>> Daniel
>>> Sent from my phone.
>>>
>>
>> I started working on this and I think I completed the changes for
>> svn_ra_local. Attached is the patch. This is a work in progress. I would
>> like to get some initial comments/suggestions on the patch and would
>> like to know whether I am proceeding on right direction.
>>
>> This patch adds new option '--ignore-properties' to 'log' sub
>> command. If this option is provided then command ignore revisions that
>> has only property changes from output.
>
> I haven't reviewed the patch, but would like to discuss the approach.
>
> Ignoring all or none of a set of prop mods may not be granular enough.
> Since consumers rarely change mergeinfo without changing content, I
> don't see that the new behavior would be exercised very often. What
> we really want to do is filter out mergeinfo changes on a per-node
> basis, not a per-revision basis as the new docs claim. (Perhaps the
> docs are inaccurate?)
>
> I don't see anywhere in the patch where you address RA layers other
> than ra_local. Also, what is the expected behavior when operating

I mentioned this in the mail. I will be handling this in the future. I
just wanted get feedback from list so that I might not end up direction
less.

> against older servers? If you send the ignore_properties flag to
> older servers, they will just ignore the flag and return all the
> revisions, and the client will dutifully report them. Do you have a
> plan for this scenario?

Thank you for pointing this out. So far I have not thought about
this. What about throwing an error, if there the server version is less
than what we expect?

>
> Finally, is there a reason why you chose to do this against trunk,
> rather than the ignore-mergeinfo-log branch, which already addresses
> many of the above concerns? I've been hacking on that branch a bit
> lately, and it isn't complete but patches and review are definitely
> welcome.
>

I did this against trunk because I could see lots of changes in
ignore-mergeinfo-log branch. For me to avoid confusion, I thought I will
start preparing patch against trunk. Also if I include this option also
to the ignore-mergeinfo-log branch, will that be possible to factor out
my changes alone? Or is it a good idea to have a separate branch for
this. I am not sure at this point.

Do you mean to say that ignore-mergeinfo-log has an option to filter out
revisions being send to the receiver if that revision contains only
property changes? Isn't that issue 3690 all about?

> Anyway, these are my thoughts, I'm interested in what others have to say.
>

Yes, I am also looking for more suggestions and feedback.

Thanks and Regards
Noorul
Received on 2011-02-01 05:55:19 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.