[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: Wed, 02 Feb 2011 17:59:15 +0530

Hyrum K Wright <hyrum_at_hyrumwright.org> writes:

> On Mon, Jan 31, 2011 at 10:52 PM, Noorul Islam K M <noorul_at_collab.net> wrote:
>
>> 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?
>
> Or more specifically, if the server doesn't support the required
> capability (as on the ignore-mergeinfo-log branch).
>
>>> 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?
>
> Well, reading back through issue 3690, it looks like it could use some
> design work. :)
>
> The reporter states the request, and then this motivation: "many users
> are not interested in reviewing changes to property changes and only
> care about content changes." To that I would ask "why?" What
> scenarios does this pop up in.
>
> I know my specific usecase is managing the release branches, and
> having to filter through all the svn:mergeinfo mods when attempting to
> find the last time CHANGES was textually modified. Maybe that would
> be accomplished by a 'ignore all prop mods' flag; maybe not. Maybe
> the approach on the ignore-mergeinfo-log branch is *too* discrete, and
> would end up being a performance killer; maybe not. The more I write,
> the the more I think this deserves some design consideration. (And
> hence my desire to do it on a branch rather than trunk: I don't want
> to hold up 1.7, and don't want to release a half-cooked idea.)
>
> One other option may be to have some way of using the ignored prop mod
> list, as on the branch, to specify ignoring *all* prop mods,
> irrespective of property name. From an API perspective, this could
> accomplish both goals. But I still think we need to think more about
> what problem we're trying to solve.
>

Is ignored_prop_mods list functionality completely implemented?

I assume that we can set any property to a target not necessarily what
is pre-defined by svn. For example.

$ svn ps foo bar file.txt

I think this is a valid command. In this case how will ignore_prop_mods
list be useful? Are you planning to handle this scenario? Am I
overlooking?

Thanks and Regards
Noorul
Received on 2011-02-02 13:30:49 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.