I cannot comment on the patch. But in general my feeling was that we
do not have to change the command line output if we do not want to,
but the notifications should be sent for API users so that we can know
these items were changed. So I trust Bert to handle that since he has
the same need as me (JavaHL patch not withstanding)
Mark
On Wed, Apr 29, 2009 at 10:50 AM, Paul Burba <ptburba_at_gmail.com> wrote:
> Yesterday in IRC:
>
> Apr 28 18:06:11 <Bert> pburba: Do you know what it would take to fix
> notifications for merge info only changes on files while merging? (I'm
> not sure if we would want it as output of svn, but I would really like
> to have the knowledge of which file changed as a notification in
> AnkhSVN (and I heard Mark would like to have the info too)
>
> Apr 28 18:07:54 <pburba> Bert: I dont understand the question, you
> want to suppress the notifications of property changes during a merge
> if the property is svn:mergeinfo?
> Apr 28 18:08:11 <pburba> files only?
>
> Apr 28 18:13:34 <Bert> No.. I want to know exactly which files are
> changed.. And I get reports of some files that receive svn:mergeinfo
> changes but aren't notified as changed
> Apr 28 18:14:30 <Bert> We use the notifications as hint to a status
> cache.. and only call svn status on files that have changes
> Apr 28 18:15:29 <Bert> (actually: We call it on the parent directory
> with --depth files, because that is virtually for free performance
> wise with WC-1.0.. but that is an implementation detail)
>
> Apr 28 18:16:47 <pburba> Bert: Ok, I understand you now. You want
> merge to notify that it is recording mergeinfo on a path describing
> the merge when that path was not directly affected by the diff (and
> thus not mentioned in the notifications). This type of thing can
> happen due to elision, shallow merges, shallow, targets, switched
> subtrees etc.
> Apr 28 18:17:39 <pburba> Bert: The thing is, the merge code doesn't
> record the mergeinfo describing the merge until after the merge (and
> the notifications) are complete. So it would have to be something
> tacked on to the merge output at the end.
>
> Apr 28 18:18:19 <Bert> Or just a notification with another action-id,
> so it wouldn't be notified by svn.. but is available to the bindings
> Apr 28 18:18:58 <Bert> But I wouldn't mind when or how it happens.. I
> just would like to have the paths of the files that changed :)
>
> Apr 28 18:25:05 <pburba> Bert: Ah, ok. Well the place that mergeinfo
> is recorded describing the merge is in
> libsvn_client/merge.c:do_directory_merge(), see the comment ''/*
> Record mergeinfo where appropriate.*/". (and yes that whole chunk of
> code needs to be factored out)
>
> Apr 28 19:33:55 <lisppaste4> Bert pasted "Initial version of mergeinfo
> change notifications" at http://paste.lisp.org/display/79375
> Apr 28 19:35:34 <Bert> pburba: If you can find some time, could you
> take a quick look at that patch to see if that would give me the
> info?.. (It's 1:30 AM here.. time to get some sleep)
>
> Apr 28 23:05:13 <pburba> Bert: I will check that out tomorrow
>
> Bert,
>
> Your patch looks good, making the notification callback from
> svn_client__record_wc_mergeinfo looks fine for the most part.
>
> My only *very* minor concerns are:
>
> 1) process_children_with_new_mergeinfo() will make a
> svn_wc_notify_merge_record_info notification but this isn't a
> situation where we are recording mergeinfo *describing the merge*.
> Rather it is the case where the diff itself added mergeinfo to a path
> that didn't previously have any mergeinfo and we need to make sure the
> path's mergeinfo gets any inherited mergeinfo made explicit. Of
> course these paths will later get mergeinfo describing the merge and
> the svn_wc_notify_merge_record_info notification will take place
> then...so I think this is largely a moot concern.
>
> 2) It will be possible to generate svn_wc_notify_merge_record_info
> notifications for paths that have no mergeinfo to start, get mergeinfo
> during the merge, then have their mergeinfo elided away, the end
> result being no change to the path. A false positive of sorts, but
> since you are only using this as a "hint" to the status cache there is
> no harm done.
>
> I made two minor comment tweaks to the attached patch:
>
> 1) Made clear in the doc string for
> svn_wc_notify_action_t:svn_wc_notify_merge_record_info that this
> action is for merges only and then only when recording mergeinfo
> *describing* the merge (to differentiate from property changes that
> are made to svn:mergeinfo as part of the diff).
>
> 2) Noted in doc string for svn_client__record_wc_mergeinfo that it now
> does a notification.
>
> Paul
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1980943
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1981052
Received on 2009-04-29 17:05:16 CEST