You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2007/12/12 15:48:12 UTC

Status on issue-2897 branch

Hi All,
I changed the 'get-commit-revs-for-merge-ranges' initial implemenation 
to 'get-commit-and-merge-ranges' as the initial implementation is prone 
to edge case.

I reworked on my original work on 'extract and apply non-reflective 
changes' later realised one more issue with 
'get-commit-and-merge-ranges' API.

Today it stands out like this,
svn_error_t *
svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
                                   apr_array_header_t **merge_ranges_list,
                                   apr_array_header_t **commit_rangelist,
                                   const char* merge_target,
                                   const char* merge_source,
                                   svn_revnum_t min_commit_rev,
                                   svn_revnum_t max_commit_rev,
                                   svn_mergeinfo_inheritance_t inherit,
                                   apr_pool_t *pool);

For the dataset of
Merged r30, r32, r36 from /source to /target at r50
Merged r40, r42, r46 from /source to /target at r51
Current api would give
commit_rangelist  = [50, 50, 50, 51, 51, 51]
merge_ranges_list  = [30, 32, 36, 40, 42, 46]

Whereas it should give
commit_rangelist  = [50, 51]
merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]

I am fixing this right now.

With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by David Glasser <gl...@davidglasser.net>.
On Dec 12, 2007 12:22 PM, David Glasser <gl...@davidglasser.net> wrote:
> Note that I think I have a way to implement that request in the FS (no
> sqlite), by basically making every noderev with mergeinfo link to the
> previous mergerev where mergeinfo changed in a new tag.

For the sake of getting this out of my head:

In addition to the currently-implemented-on-branch new tags
("has-mergeinfo" and "mergeinfo-count"), there will be a
"mergeinfo-changed" flag and a "previous-mergeinfo-change" node-id
tag.

A call to svn_fs_change_node_prop(svn:mergeinfo) will cause the
mergeinfo-changed flag to be set.  (I'm not sure if it should be set
for 'propdeletes' or not.  I think it should be?)

Any time that a node is made mutable (ie, made into a transaction
node), the flag is cleared, and if it had been set, the
previous-mergeinfo-change field is set to the node-id of the noderev
it was cloned from.

I'm not 100% sure what we do when a node is made mutable under a
different path than its created path (ie, you do a copy above some
mergeinfo, and later change something under the mergeinfo)... That is,
I don't know if the linked list should "stop on copy" or not.  Maybe
it should.  In that case, the previous paragraph's thing only sets the
previous-mergeinfo-change field if the previous node was at the same
path; also, it should be cleared for a node which itself copied.

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by Kamesh Jayachandran <ka...@collab.net>.

David Glasser wrote:
> On Dec 12, 2007 7:48 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
>   
>> Hi All,
>> I changed the 'get-commit-revs-for-merge-ranges' initial implemenation
>> to 'get-commit-and-merge-ranges' as the initial implementation is prone
>> to edge case.
>>
>> I reworked on my original work on 'extract and apply non-reflective
>> changes' later realised one more issue with
>> 'get-commit-and-merge-ranges' API.
>>
>> Today it stands out like this,
>> svn_error_t *
>> svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
>>                                    apr_array_header_t **merge_ranges_list,
>>                                    apr_array_header_t **commit_rangelist,
>>                                    const char* merge_target,
>>                                    const char* merge_source,
>>                                    svn_revnum_t min_commit_rev,
>>                                    svn_revnum_t max_commit_rev,
>>                                    svn_mergeinfo_inheritance_t inherit,
>>                                    apr_pool_t *pool);
>>
>> For the dataset of
>> Merged r30, r32, r36 from /source to /target at r50
>> Merged r40, r42, r46 from /source to /target at r51
>> Current api would give
>> commit_rangelist  = [50, 50, 50, 51, 51, 51]
>> merge_ranges_list  = [30, 32, 36, 40, 42, 46]
>>
>> Whereas it should give
>> commit_rangelist  = [50, 51]
>> merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]
>>     
>
> I have finally gotten a chance to review the current state of the
> issue-2897 branch.
>
> It doesn't look like you actually use the returned merge_ranges_list
> anywhere; you just use the commit_rangelist.
>
> ie, all you're asking the repos for is "what revisions between R1 and
> R2 had mergeinfo changes for TO from SOURCE"?
>
> Are you going to need the merge_ranges_list later?
>   

Yes. I need this 'merge_ranges_list' to find the non-reflective changes.

> Note that I think I have a way to implement that request in the FS (no
> sqlite), by basically making every noderev with mergeinfo link to the
> previous mergerev where mergeinfo changed in a new tag.
>
>   

That would be great!.

> Also, the query you do (in get_commit_and_merge_ranges) seems a little
> bogus as far as inheritance goes.  Basically it seems like you find
> the nearest ancestor that has some mergeinfo change somewhere inside
> the revision, and then use exactly that ancestor to do all the later
> queries.  However, if the ancestor that MERGE_TARGET inherits its
> mergeinfo from changes over the course of the revision range that
> you're searching (ie, maybe sometimes it's MERGE_TARGET, sometimes
> it's one ancestor, sometimes it's another) that query will be wrong.
> I don't see a trivial way to fix this.  (And I would not support
> releasing 1.5 with APIs that suffer from this sort of issue.)
>
>   

Thanks for catching that, will fix that. It broke since the recent 
change in API.


With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by David Glasser <gl...@davidglasser.net>.
On Dec 12, 2007 7:48 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
> Hi All,
> I changed the 'get-commit-revs-for-merge-ranges' initial implemenation
> to 'get-commit-and-merge-ranges' as the initial implementation is prone
> to edge case.
>
> I reworked on my original work on 'extract and apply non-reflective
> changes' later realised one more issue with
> 'get-commit-and-merge-ranges' API.
>
> Today it stands out like this,
> svn_error_t *
> svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
>                                    apr_array_header_t **merge_ranges_list,
>                                    apr_array_header_t **commit_rangelist,
>                                    const char* merge_target,
>                                    const char* merge_source,
>                                    svn_revnum_t min_commit_rev,
>                                    svn_revnum_t max_commit_rev,
>                                    svn_mergeinfo_inheritance_t inherit,
>                                    apr_pool_t *pool);
>
> For the dataset of
> Merged r30, r32, r36 from /source to /target at r50
> Merged r40, r42, r46 from /source to /target at r51
> Current api would give
> commit_rangelist  = [50, 50, 50, 51, 51, 51]
> merge_ranges_list  = [30, 32, 36, 40, 42, 46]
>
> Whereas it should give
> commit_rangelist  = [50, 51]
> merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]

I have finally gotten a chance to review the current state of the
issue-2897 branch.

It doesn't look like you actually use the returned merge_ranges_list
anywhere; you just use the commit_rangelist.

ie, all you're asking the repos for is "what revisions between R1 and
R2 had mergeinfo changes for TO from SOURCE"?

Are you going to need the merge_ranges_list later?

Note that I think I have a way to implement that request in the FS (no
sqlite), by basically making every noderev with mergeinfo link to the
previous mergerev where mergeinfo changed in a new tag.

Also, the query you do (in get_commit_and_merge_ranges) seems a little
bogus as far as inheritance goes.  Basically it seems like you find
the nearest ancestor that has some mergeinfo change somewhere inside
the revision, and then use exactly that ancestor to do all the later
queries.  However, if the ancestor that MERGE_TARGET inherits its
mergeinfo from changes over the course of the revision range that
you're searching (ie, maybe sometimes it's MERGE_TARGET, sometimes
it's one ancestor, sometimes it's another) that query will be wrong.
I don't see a trivial way to fix this.  (And I would not support
releasing 1.5 with APIs that suffer from this sort of issue.)

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by David Glasser <gl...@davidglasser.net>.
On Dec 13, 2007 12:54 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
>
>
> David Glasser wrote:
> > On Dec 12, 2007 11:23 PM, Kamesh Jayachandran <ka...@collab.net> wrote:
> >
> >>> (One big question I'm concerned about, looking at that API, is peg-rev
> >>> resolution.  What if "merge_target" refers to different, unrelated
> >>> lines of history at min_commit_rev and max_commit_rev?)
> >>>
> >>>
> >>>
> >> Caller subversion/libsvn_client/merge.c filter_reflected_revisions does
> >> a peg rev lookup and calls the API with relevant paths and revisions.
> >>
> >
> > Are you saying it *should* do that or that it does?  I don't see that now.
> >
> > --dave
> >
> >
> It does that, revision number and urls are normalized via
> normalize_merge_sources early in the call chain.

First, that should be documented as part of the API.

Secondly, that's just the merge source --- I'm concerned about the merge target.

--dave


-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by Kamesh Jayachandran <ka...@collab.net>.

David Glasser wrote:
> On Dec 12, 2007 11:23 PM, Kamesh Jayachandran <ka...@collab.net> wrote:
>   
>>> (One big question I'm concerned about, looking at that API, is peg-rev
>>> resolution.  What if "merge_target" refers to different, unrelated
>>> lines of history at min_commit_rev and max_commit_rev?)
>>>
>>>
>>>       
>> Caller subversion/libsvn_client/merge.c filter_reflected_revisions does
>> a peg rev lookup and calls the API with relevant paths and revisions.
>>     
>
> Are you saying it *should* do that or that it does?  I don't see that now.
>
> --dave
>
>   
It does that, revision number and urls are normalized via 
normalize_merge_sources early in the call chain.

With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by David Glasser <gl...@davidglasser.net>.
On Dec 12, 2007 11:23 PM, Kamesh Jayachandran <ka...@collab.net> wrote:
> > (One big question I'm concerned about, looking at that API, is peg-rev
> > resolution.  What if "merge_target" refers to different, unrelated
> > lines of history at min_commit_rev and max_commit_rev?)
> >
> >
> Caller subversion/libsvn_client/merge.c filter_reflected_revisions does
> a peg rev lookup and calls the API with relevant paths and revisions.

Are you saying it *should* do that or that it does?  I don't see that now.

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by Kamesh Jayachandran <ka...@collab.net>.

David Glasser wrote:
> On Dec 12, 2007 7:48 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
>   
>> Hi All,
>> I changed the 'get-commit-revs-for-merge-ranges' initial implemenation
>> to 'get-commit-and-merge-ranges' as the initial implementation is prone
>> to edge case.
>>
>> I reworked on my original work on 'extract and apply non-reflective
>> changes' later realised one more issue with
>> 'get-commit-and-merge-ranges' API.
>>
>> Today it stands out like this,
>> svn_error_t *
>> svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
>>                                    apr_array_header_t **merge_ranges_list,
>>                                    apr_array_header_t **commit_rangelist,
>>                                    const char* merge_target,
>>                                    const char* merge_source,
>>                                    svn_revnum_t min_commit_rev,
>>                                    svn_revnum_t max_commit_rev,
>>                                    svn_mergeinfo_inheritance_t inherit,
>>                                    apr_pool_t *pool);
>>
>> For the dataset of
>> Merged r30, r32, r36 from /source to /target at r50
>> Merged r40, r42, r46 from /source to /target at r51
>> Current api would give
>> commit_rangelist  = [50, 50, 50, 51, 51, 51]
>> merge_ranges_list  = [30, 32, 36, 40, 42, 46]
>>
>> Whereas it should give
>> commit_rangelist  = [50, 51]
>> merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]
>>
>> I am fixing this right now.
>>     
>
> Hi Kamesh.  Can you clarify in text somewhere exactly what the
> information that this API needs to harvest is?  I have some ideas
> about how to better utilize (index and query) Sqlite that I haven't
> put into practice yet; I'd be happy to help with this.
>
> So what are the questions that this API needs to ask?  And what is
> your proposed schema?
>
>   

The schema change to mergeinfo_changed as already discussed, it will be 
same as 'mergeinfo' with just
recording fresh merges on a commit alone recorded on a mergeinfo_changed 
table.

The purpose of the API is to give a 1-1 corresponding lists
commit_rangelist of type apr_array_header_t *(comprising 
svn_merge_range_t* elements)
merge_ranges_list of type apr_array_header_t *(comprising 
apr_array_header_t* elements, each such element's list contains 
svn_merge_range_t *)
> (One big question I'm concerned about, looking at that API, is peg-rev
> resolution.  What if "merge_target" refers to different, unrelated
> lines of history at min_commit_rev and max_commit_rev?)
>
>   
Caller subversion/libsvn_client/merge.c filter_reflected_revisions does 
a peg rev lookup and calls the API with relevant paths and revisions.

Thanks for the review
With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by Kamesh Jayachandran <ka...@collab.net>.

David Glasser wrote:
> On Dec 12, 2007 11:40 AM, David Glasser <gl...@davidglasser.net> wrote:
>   
>> On Dec 12, 2007 8:51 AM, David Glasser <gl...@davidglasser.net> wrote:
>>     
>>> On Dec 12, 2007 7:48 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
>>>       
>>>> Hi All,
>>>> I changed the 'get-commit-revs-for-merge-ranges' initial implemenation
>>>> to 'get-commit-and-merge-ranges' as the initial implementation is prone
>>>> to edge case.
>>>>
>>>> I reworked on my original work on 'extract and apply non-reflective
>>>> changes' later realised one more issue with
>>>> 'get-commit-and-merge-ranges' API.
>>>>
>>>> Today it stands out like this,
>>>> svn_error_t *
>>>> svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
>>>>                                    apr_array_header_t **merge_ranges_list,
>>>>                                    apr_array_header_t **commit_rangelist,
>>>>                                    const char* merge_target,
>>>>                                    const char* merge_source,
>>>>                                    svn_revnum_t min_commit_rev,
>>>>                                    svn_revnum_t max_commit_rev,
>>>>                                    svn_mergeinfo_inheritance_t inherit,
>>>>                                    apr_pool_t *pool);
>>>>
>>>> For the dataset of
>>>> Merged r30, r32, r36 from /source to /target at r50
>>>> Merged r40, r42, r46 from /source to /target at r51
>>>> Current api would give
>>>> commit_rangelist  = [50, 50, 50, 51, 51, 51]
>>>> merge_ranges_list  = [30, 32, 36, 40, 42, 46]
>>>>
>>>> Whereas it should give
>>>> commit_rangelist  = [50, 51]
>>>> merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]
>>>>
>>>> I am fixing this right now.
>>>>         
>>> Hi Kamesh.  Can you clarify in text somewhere exactly what the
>>> information that this API needs to harvest is?  I have some ideas
>>> about how to better utilize (index and query) Sqlite that I haven't
>>> put into practice yet; I'd be happy to help with this.
>>>
>>> So what are the questions that this API needs to ask?  And what is
>>> your proposed schema?
>>>
>>> (One big question I'm concerned about, looking at that API, is peg-rev
>>> resolution.  What if "merge_target" refers to different, unrelated
>>> lines of history at min_commit_rev and max_commit_rev?)
>>>       
>> Wouldn't the return value be more natural as a hash?  I guess you'd
>> have to sprintf the keys (commit revs) to strings.  And maybe you do
>> rely on the sort order; I dunno.
>>     
>
> Note that you don't have to sprintf; you just have to make sure that
> the revnums are in pool-allocated memory, and pass
> sizeof(svn_revnum_t) as the size arg in apr_hash_get/set.  See the use
> of pb->rev_map in libsvn_repos/load.c, eg.
>
>   

Hashtable may not workout as again I need to sort when I consume with 
other range API.

With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by David Glasser <gl...@davidglasser.net>.
On Dec 12, 2007 11:40 AM, David Glasser <gl...@davidglasser.net> wrote:
>
> On Dec 12, 2007 8:51 AM, David Glasser <gl...@davidglasser.net> wrote:
> >
> > On Dec 12, 2007 7:48 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
> > > Hi All,
> > > I changed the 'get-commit-revs-for-merge-ranges' initial implemenation
> > > to 'get-commit-and-merge-ranges' as the initial implementation is prone
> > > to edge case.
> > >
> > > I reworked on my original work on 'extract and apply non-reflective
> > > changes' later realised one more issue with
> > > 'get-commit-and-merge-ranges' API.
> > >
> > > Today it stands out like this,
> > > svn_error_t *
> > > svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
> > >                                    apr_array_header_t **merge_ranges_list,
> > >                                    apr_array_header_t **commit_rangelist,
> > >                                    const char* merge_target,
> > >                                    const char* merge_source,
> > >                                    svn_revnum_t min_commit_rev,
> > >                                    svn_revnum_t max_commit_rev,
> > >                                    svn_mergeinfo_inheritance_t inherit,
> > >                                    apr_pool_t *pool);
> > >
> > > For the dataset of
> > > Merged r30, r32, r36 from /source to /target at r50
> > > Merged r40, r42, r46 from /source to /target at r51
> > > Current api would give
> > > commit_rangelist  = [50, 50, 50, 51, 51, 51]
> > > merge_ranges_list  = [30, 32, 36, 40, 42, 46]
> > >
> > > Whereas it should give
> > > commit_rangelist  = [50, 51]
> > > merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]
> > >
> > > I am fixing this right now.
> >
> > Hi Kamesh.  Can you clarify in text somewhere exactly what the
> > information that this API needs to harvest is?  I have some ideas
> > about how to better utilize (index and query) Sqlite that I haven't
> > put into practice yet; I'd be happy to help with this.
> >
> > So what are the questions that this API needs to ask?  And what is
> > your proposed schema?
> >
> > (One big question I'm concerned about, looking at that API, is peg-rev
> > resolution.  What if "merge_target" refers to different, unrelated
> > lines of history at min_commit_rev and max_commit_rev?)
>
> Wouldn't the return value be more natural as a hash?  I guess you'd
> have to sprintf the keys (commit revs) to strings.  And maybe you do
> rely on the sort order; I dunno.

Note that you don't have to sprintf; you just have to make sure that
the revnums are in pool-allocated memory, and pass
sizeof(svn_revnum_t) as the size arg in apr_hash_get/set.  See the use
of pb->rev_map in libsvn_repos/load.c, eg.

--dave


-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by Kamesh Jayachandran <ka...@collab.net>.

David Glasser wrote:
> On Dec 12, 2007 8:51 AM, David Glasser <gl...@davidglasser.net> wrote:
>   
>> On Dec 12, 2007 7:48 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
>>     
>>> Hi All,
>>> I changed the 'get-commit-revs-for-merge-ranges' initial implemenation
>>> to 'get-commit-and-merge-ranges' as the initial implementation is prone
>>> to edge case.
>>>
>>> I reworked on my original work on 'extract and apply non-reflective
>>> changes' later realised one more issue with
>>> 'get-commit-and-merge-ranges' API.
>>>
>>> Today it stands out like this,
>>> svn_error_t *
>>> svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
>>>                                    apr_array_header_t **merge_ranges_list,
>>>                                    apr_array_header_t **commit_rangelist,
>>>                                    const char* merge_target,
>>>                                    const char* merge_source,
>>>                                    svn_revnum_t min_commit_rev,
>>>                                    svn_revnum_t max_commit_rev,
>>>                                    svn_mergeinfo_inheritance_t inherit,
>>>                                    apr_pool_t *pool);
>>>
>>> For the dataset of
>>> Merged r30, r32, r36 from /source to /target at r50
>>> Merged r40, r42, r46 from /source to /target at r51
>>> Current api would give
>>> commit_rangelist  = [50, 50, 50, 51, 51, 51]
>>> merge_ranges_list  = [30, 32, 36, 40, 42, 46]
>>>
>>> Whereas it should give
>>> commit_rangelist  = [50, 51]
>>> merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]
>>>
>>> I am fixing this right now.
>>>       
>> Hi Kamesh.  Can you clarify in text somewhere exactly what the
>> information that this API needs to harvest is?  I have some ideas
>> about how to better utilize (index and query) Sqlite that I haven't
>> put into practice yet; I'd be happy to help with this.
>>
>> So what are the questions that this API needs to ask?  And what is
>> your proposed schema?
>>
>> (One big question I'm concerned about, looking at that API, is peg-rev
>> resolution.  What if "merge_target" refers to different, unrelated
>> lines of history at min_commit_rev and max_commit_rev?)
>>     
>
> Wouldn't the return value be more natural as a hash?  I guess you'd
> have to sprintf the keys (commit revs) to strings.  And maybe you do
> rely on the sort order; I dunno.
>
> Also, is commit_rangelist a list of revnums or of ranges?  Is
> merge_ranges_lists a list of lists of revnums, or a list of
> rangelists?
>
> --dave
>
>   
commit_rangelist is of type apr_array_header_t *(comprising 
svn_merge_range_t* elements)
merge_ranges_list is of type apr_array_header_t *(comprising 
apr_array_header_t* elements, each such element's list contains 
svn_merge_range_t *) (currently fixing this).

With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by David Glasser <gl...@davidglasser.net>.
On Dec 12, 2007 8:51 AM, David Glasser <gl...@davidglasser.net> wrote:
>
> On Dec 12, 2007 7:48 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
> > Hi All,
> > I changed the 'get-commit-revs-for-merge-ranges' initial implemenation
> > to 'get-commit-and-merge-ranges' as the initial implementation is prone
> > to edge case.
> >
> > I reworked on my original work on 'extract and apply non-reflective
> > changes' later realised one more issue with
> > 'get-commit-and-merge-ranges' API.
> >
> > Today it stands out like this,
> > svn_error_t *
> > svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
> >                                    apr_array_header_t **merge_ranges_list,
> >                                    apr_array_header_t **commit_rangelist,
> >                                    const char* merge_target,
> >                                    const char* merge_source,
> >                                    svn_revnum_t min_commit_rev,
> >                                    svn_revnum_t max_commit_rev,
> >                                    svn_mergeinfo_inheritance_t inherit,
> >                                    apr_pool_t *pool);
> >
> > For the dataset of
> > Merged r30, r32, r36 from /source to /target at r50
> > Merged r40, r42, r46 from /source to /target at r51
> > Current api would give
> > commit_rangelist  = [50, 50, 50, 51, 51, 51]
> > merge_ranges_list  = [30, 32, 36, 40, 42, 46]
> >
> > Whereas it should give
> > commit_rangelist  = [50, 51]
> > merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]
> >
> > I am fixing this right now.
>
> Hi Kamesh.  Can you clarify in text somewhere exactly what the
> information that this API needs to harvest is?  I have some ideas
> about how to better utilize (index and query) Sqlite that I haven't
> put into practice yet; I'd be happy to help with this.
>
> So what are the questions that this API needs to ask?  And what is
> your proposed schema?
>
> (One big question I'm concerned about, looking at that API, is peg-rev
> resolution.  What if "merge_target" refers to different, unrelated
> lines of history at min_commit_rev and max_commit_rev?)

Wouldn't the return value be more natural as a hash?  I guess you'd
have to sprintf the keys (commit revs) to strings.  And maybe you do
rely on the sort order; I dunno.

Also, is commit_rangelist a list of revnums or of ranges?  Is
merge_ranges_lists a list of lists of revnums, or a list of
rangelists?

--dave


-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Status on issue-2897 branch

Posted by David Glasser <gl...@davidglasser.net>.
On Dec 12, 2007 7:48 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
> Hi All,
> I changed the 'get-commit-revs-for-merge-ranges' initial implemenation
> to 'get-commit-and-merge-ranges' as the initial implementation is prone
> to edge case.
>
> I reworked on my original work on 'extract and apply non-reflective
> changes' later realised one more issue with
> 'get-commit-and-merge-ranges' API.
>
> Today it stands out like this,
> svn_error_t *
> svn_ra_get_commit_and_merge_ranges(svn_ra_session_t *session,
>                                    apr_array_header_t **merge_ranges_list,
>                                    apr_array_header_t **commit_rangelist,
>                                    const char* merge_target,
>                                    const char* merge_source,
>                                    svn_revnum_t min_commit_rev,
>                                    svn_revnum_t max_commit_rev,
>                                    svn_mergeinfo_inheritance_t inherit,
>                                    apr_pool_t *pool);
>
> For the dataset of
> Merged r30, r32, r36 from /source to /target at r50
> Merged r40, r42, r46 from /source to /target at r51
> Current api would give
> commit_rangelist  = [50, 50, 50, 51, 51, 51]
> merge_ranges_list  = [30, 32, 36, 40, 42, 46]
>
> Whereas it should give
> commit_rangelist  = [50, 51]
> merge_ranges_list  = [[30, 32, 36], [40, 42, 46]]
>
> I am fixing this right now.

Hi Kamesh.  Can you clarify in text somewhere exactly what the
information that this API needs to harvest is?  I have some ideas
about how to better utilize (index and query) Sqlite that I haven't
put into practice yet; I'd be happy to help with this.

So what are the questions that this API needs to ask?  And what is
your proposed schema?

(One big question I'm concerned about, looking at that API, is peg-rev
resolution.  What if "merge_target" refers to different, unrelated
lines of history at min_commit_rev and max_commit_rev?)

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org