You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@finemaltcoding.com> on 2005/08/10 20:39:55 UTC

Re: [Subclipse-dev] Enhancement needed in svn status -u

On Fri, 05 Aug 2005, Mark Phippard wrote:

> This is something we critically need in Subclipse from JavaHL, but as I 
> understand things the root of the problem is in the svn_client API.  Let 
> me step back and explain.

Mark, I may mis-understand here, or this may've been sent before all the
discussion on the dev@subversion list occurred, but I thought JavaHL was
simply failing to fully expose the APIs/info needed by Subclipse.
 
...
> If you look at the screenshot, it is ultimately just a graphical 
> representation of the svn st -u command, which is essentially what drives 
> this feature in Subclipse.  We have worked hard over the course of the 
> past year to get this feature working perfectly, but there have always 
> been nagging problems here and there depending on the approach du jour. We 
> have always known, or at least suspected, that the fundamental problem is 
> that the JavaHL equivalent of svn st -u does not give us all of the 
> information we require to implement this feature.  Specifically, when it 
> gives us the list of changed items from the server, it does not give us 
> any information about the revision of the item on the server.  We get back 
> information about the local copy of the item (or no information at all if 
> it is a new item).  We really need the information of the item on the 
> server, specifically the last changed revision and the URL, but ideally we 
> would get other information such as whether it is a file or folder, the 
> last author, date etc....
> 
> We have had to resort to calling the JavaHL equivalent of svn info on each 
> item we get back from the status call, one item at a time, to get things 
> working correctly.  Needless to say, the performance is not acceptable 
> with this approach.
> 
> I was discussing this on IRC with sussman and he seemed to think that the 
> status process receives most, if not all, of this information from the 
> server and currently just discards it.  He suggested that the client API 
> would need to be rev'd to return the additional information and that I 
> should take this to the dev@ list for discussion.  I am a Java programmer, 
> so I cannot do this, but I have asked my co-worker Paul Burba, who has 
> done the EBCDIC port, to take a look at the API's and see what he can 
> figure out. 

Mark, if this is only a JavaHL change, I'm willing to make the changes,
but would like you tell me what the expected Java API should look like.

If the svn_client API needs to be rev'd, it's probably more than I can
bite off at this time, but my offer to adjust JavaHL stands after the
svn_client API offers support for what Subclipse needs.

- Dan

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Thu, 11 Aug 2005, Paul Burba wrote:

> "Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/11/2005 02:20:09 
> AM:
...
> > I don't think you need to rev the client API. You can add fields to the
> > end of svn_wc_status2_t (that's even documented!).
> > 
> > This chage seems pretty straight forward to me. Just add the fields you
> > need, and catch the entryprops in change_file_prop and change_dir_prop 
> in
> > libsvn_wc/status.c.  The URL field is a little tricky because of 
> switched
> > entries.  For entries that exist in the WC, it is readily available. For
> > added paths, there must be a nearest ancestor in the WC with an URL that
> > you can extend.
> 
> In looking at this, I came to more or less the same conclusion, though 
> undoubtedly it took me a *bit* longer to figure it out.
> 
> I'm still unsure about the need to rev svn_wc_status2_t or not.  Looking 
> at r13791 (Rev the svn_wc_status_t structure to svn_wc_status2_t, so as 
> not to break ABI) as a roadmap for adding to svn_wc_status_t, the 
> implication is that I need to rev svn_wc_status2_t, svn_wc_dup_status2, 
> svn_wc_status2, etc...but r13805 added the comment you alluded to above: 
> 
>  * @note Fields may be added to the end of this structure in future
>  * versions.  Therefore, users shouldn't allocate structures of this
>  * type, to preserve binary compatibility.
> 
> Does this warning alone give the green light to add members to 
> svn_wc_status2_t without revving it?  The following thread on ABI breakage 
> reveals no widespread consensus on this topic:
> 
>   http://svn.haxx.se/dev/archive-2005-03/0929.shtml
...

We've discussed something similar more recently:

http://svn.haxx.se/dev/archive-2005-04/0972.shtml
http://svn.haxx.se/dev/archive-2005-04/0992.shtml
http://svn.haxx.se/dev/archive-2005-04/0995.shtml

My interpretation of this discussion in relationship to the issue at hand,
in conjunction with the documentation on that structure, indicates that you
(conceptually) have the green light, Paul.

With regard to the referenced mail thread, I chose not to add a field to the
end of the struct based on API considerations mentioned by Greg Hudson (which
I privately shared), rather than ABI compatibility concerns.

- Dan

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 17 Aug 2005, Paul Burba wrote:

> "Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/17/2005 04:57:06
> PM:
>
> > What will these be when the WC entry is not ood? They could be either
> the
> > same as the entryprops in the svn_wc_entry_t, or NULL/invalid.  I've no
> > strong opinion.
>
> I was thinking the former, though I confess a lack of any compelling
> argument as to why.
>
On second thought, I like the latte, since the entryprops in the WC are
available anyway and might be lying if we just assume the same revision
implies the same entryprops (think propedit). So, NULL/INVALID_REVNUM if
they're not set by the server.

Thanks,
//Peter

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by Paul Burba <Pa...@softlanding.com>.
"Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/17/2005 04:57:06 
PM:

> On Wed, 17 Aug 2005, Paul Burba wrote:
> 
> > "Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/11/2005 
03:48:53
> > PM:
> >
> > > On Thu, 11 Aug 2005, Paul Burba wrote:
> > >
> > Ok, here is what I'm thinking:
> >
> This approach seems great.
> 
> 
> > * modify subversion/include/svn_wc.h
> > (svn_wc_status2_t): Add new members:
> >   svn_revnum_t ood_last_cmt_rev
> >   apr_time_t *ood_last_cmt_date
> >   svn_node_kind_t ood_kind
> >   const char *ood_url
> >   const char *ood_last_cmt_author
> >
> What will these be when the WC entry is not ood? They could be either 
the
> same as the entryprops in the svn_wc_entry_t, or NULL/invalid.  I've no
> strong opinion.

I was thinking the former, though I confess a lack of any compelling 
argument as to why.

Paul B. 

_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs. 
_____________________________________________________________________________

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 17 Aug 2005, Paul Burba wrote:

> "Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/11/2005 03:48:53
> PM:
>
> > On Thu, 11 Aug 2005, Paul Burba wrote:
> >
> Ok, here is what I'm thinking:
>
This approach seems great.


> * modify subversion/include/svn_wc.h
> (svn_wc_status2_t): Add new members:
>   svn_revnum_t ood_last_cmt_rev
>   apr_time_t *ood_last_cmt_date
>   svn_node_kind_t ood_kind
>   const char *ood_url
>   const char *ood_last_cmt_author
>
What will these be when the WC entry is not ood? They could be either the
same as the entryprops in the svn_wc_entry_t, or NULL/invalid.  I've no
strong opinion.

Regards,
//Peter

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by Paul Burba <Pa...@softlanding.com>.
Sorry for the delay in getting back to this topic, I've been off *racing* 
("trying not to get killed" is a more apt description) mountain bikes the 
last few days. 
"Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/11/2005 03:48:53 
PM:

> On Thu, 11 Aug 2005, Paul Burba wrote:
> 
> > "Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/11/2005 
02:20:09
> > AM:
> >
> > > I don't think you need to rev the client API. You can add fields to 
the
> > > end of svn_wc_status2_t (that's even documented!).
> > >
> > > This chage seems pretty straight forward to me. Just add the fields 
you
> > > need, and catch the entryprops in change_file_prop and 
change_dir_prop
> > in
> > > libsvn_wc/status.c.  The URL field is a little tricky because of
> > switched
> > > entries.  For entries that exist in the WC, it is readily available. 
For
> > > added paths, there must be a nearest ancestor in the WC with an URL 
that
> > > you can extend.
> >
> > In looking at this, I came to more or less the same conclusion, though
> > undoubtedly it took me a *bit* longer to figure it out.
> >
> I certainly could have saved you some time by posting earlier. Sorry for
> that. Hope you learned some things during your tour:-)

Thanks for the guidance Peter.  Yes, I learned quite a bit sifting through 
the
code, so it was a blessing in disguise.  Even if you had pointed me right 
to a
solution I suspect I would have had to do this anyway.

> > I'm still unsure about the need to rev svn_wc_status2_t or not. 
Looking
> > at r13791 (Rev the svn_wc_status_t structure to svn_wc_status2_t, so 
as
> > not to break ABI) as a roadmap for adding to svn_wc_status_t, the
> > implication is that I need to rev svn_wc_status2_t, 
svn_wc_dup_status2,
> > svn_wc_status2, etc...but r13805 added the comment you alluded to 
above:
> >
> >  * @note Fields may be added to the end of this structure in future
> >  * versions.  Therefore, users shouldn't allocate structures of this
> >  * type, to preserve binary compatibility.
> >
> The reason svn_wc_status_t was revved was that there was no such clause 
in
> that documentation and there were public APIs consuimg such struct that
> could have been allocated by the caller. If people allocate
> svn_wc_status2_t structs themselves, then I wouldn't mind if someone 
sent
> them naked to Antarctica. You can safely add fields to the end of this
> struct.

Ok, here is what I'm thinking:

* modify subversion/include/svn_wc.h
(svn_wc_status2_t): Add new members:
  svn_revnum_t ood_last_cmt_rev
  apr_time_t *ood_last_cmt_date
  svn_node_kind_t ood_kind
  const char *ood_url
  const char *ood_last_cmt_author

ood = "out of date", but maybe "update" makes more sense as a prefix...

* modify subversion/libsvn_wc/status.c
(dir_baton, file_baton):
   ...Add new members:
        svn_revnum_t ood_last_cmt_rev
        apr_time_t *ood_last_cmt_date
        svn_node_kind_t ood_kind
        const char *ood_url
        const char *ood_last_cmt_author

(assemble_status): Initialize new svn_wc_status2_t members.

(change_dir_prop): Temporarily store ood info in the directory_baton.
Set db->ood_kind to svn_node_dir, get db->ood_url using
find_dir_url, and pick up db->ood_last_cmt_author, db->ood_last_cmt_date, 
and
db->ood_last_cmt_rev from the relevant properties as each is changed.

(change_file_prop): Essentially the same as change_dir_prop, except
fb->ood_kind is set to svn_node_file and fb->ood_url is obtained by 
joining
find_dir_url with uri escaped fb->path.

(close_directory): Copy the new members stored in the dir_baton to the
svn_wc_status2_t *dir_status structure.  This accomplishes the real goal
here: making more detailed out of date information available to callers of
svn_client_status2, particularly for new items in the repository.  (See 
the
start of this thread http://svn.haxx.se/dev/archive-2005-08/0257.shtml).

(close_file): Same as close_directory, except that the svn_wc_status2_t
struct is obtained via fb->dir_baton->statii hash.

I've played around a little with the preceding ideas and so far so good. 
But
before I go any further I want to get some opinions on this approach.
So fire away please.  If this approach sounds acceptable I will submit a
formal patch.

As always, all comments, questions, criticisms, and/or nit-picks are
greatly appreciated.

Thanks,

Paul B.

_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs. 
_____________________________________________________________________________

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Thu, 11 Aug 2005, Paul Burba wrote:

> "Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/11/2005 02:20:09
> AM:
>
> > I don't think you need to rev the client API. You can add fields to the
> > end of svn_wc_status2_t (that's even documented!).
> >
> > This chage seems pretty straight forward to me. Just add the fields you
> > need, and catch the entryprops in change_file_prop and change_dir_prop
> in
> > libsvn_wc/status.c.  The URL field is a little tricky because of
> switched
> > entries.  For entries that exist in the WC, it is readily available. For
> > added paths, there must be a nearest ancestor in the WC with an URL that
> > you can extend.
>
> In looking at this, I came to more or less the same conclusion, though
> undoubtedly it took me a *bit* longer to figure it out.
>
I certainly could have saved you some time by posting earlier. Sorry for
that. Hope you learned some things during your tour:-)

> I'm still unsure about the need to rev svn_wc_status2_t or not.  Looking
> at r13791 (Rev the svn_wc_status_t structure to svn_wc_status2_t, so as
> not to break ABI) as a roadmap for adding to svn_wc_status_t, the
> implication is that I need to rev svn_wc_status2_t, svn_wc_dup_status2,
> svn_wc_status2, etc...but r13805 added the comment you alluded to above:
>
>  * @note Fields may be added to the end of this structure in future
>  * versions.  Therefore, users shouldn't allocate structures of this
>  * type, to preserve binary compatibility.
>
The reason svn_wc_status_t was revved was that there was no such clause in
that documentation and there were public APIs consuimg such struct that
could have been allocated by the caller. If people allocate
svn_wc_status2_t structs themselves, then I wouldn't mind if someone sent
them naked to Antarctica. You can safely add fields to the end of this
struct.

Thanks,
//Peter

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by Paul Burba <Pa...@softlanding.com>.
"Peter N. Lundblad" <pe...@famlundblad.se> wrote on 08/11/2005 02:20:09 
AM:

> On Wed, 10 Aug 2005, Mark Phippard wrote:
> 
> > My initial thought/hope was that the client API had the info that was
> > needed and JavaHL just needed to be revved to pass it back.  I thought
> > that svn st -u -v was showing the revision of the item in the 
repository.
> > Sussman pointed out to me on IRC that this is not the case.  We are
> > currently working on a design proposal for revving the client API to
> > include information about the repository item in the information it
> > returns.  We are just trying to wrap our heads around it right now.
> >
> I don't think you need to rev the client API. You can add fields to the
> end of svn_wc_status2_t (that's even documented!).
> 
> This chage seems pretty straight forward to me. Just add the fields you
> need, and catch the entryprops in change_file_prop and change_dir_prop 
in
> libsvn_wc/status.c.  The URL field is a little tricky because of 
switched
> entries.  For entries that exist in the WC, it is readily available. For
> added paths, there must be a nearest ancestor in the WC with an URL that
> you can extend.

In looking at this, I came to more or less the same conclusion, though 
undoubtedly it took me a *bit* longer to figure it out.

I'm still unsure about the need to rev svn_wc_status2_t or not.  Looking 
at r13791 (Rev the svn_wc_status_t structure to svn_wc_status2_t, so as 
not to break ABI) as a roadmap for adding to svn_wc_status_t, the 
implication is that I need to rev svn_wc_status2_t, svn_wc_dup_status2, 
svn_wc_status2, etc...but r13805 added the comment you alluded to above: 

 * @note Fields may be added to the end of this structure in future
 * versions.  Therefore, users shouldn't allocate structures of this
 * type, to preserve binary compatibility.

Does this warning alone give the green light to add members to 
svn_wc_status2_t without revving it?  The following thread on ABI breakage 
reveals no widespread consensus on this topic:

  http://svn.haxx.se/dev/archive-2005-03/0929.shtml

I might be missing something terribly simple as this is my first time 
tackling a change of this nature and my understanding of maintaining ABI 
is a bit shaky; so any guidance is greatly appreciated.

Thanks,

Paul B. 
 
> Hope this gives some hints,
> //Peter - who recently messed with status -u for locking...

_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs. 
_____________________________________________________________________________

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 10 Aug 2005, Mark Phippard wrote:

> My initial thought/hope was that the client API had the info that was
> needed and JavaHL just needed to be revved to pass it back.  I thought
> that svn st -u -v was showing the revision of the item in the repository.
> Sussman pointed out to me on IRC that this is not the case.  We are
> currently working on a design proposal for revving the client API to
> include information about the repository item in the information it
> returns.  We are just trying to wrap our heads around it right now.
>
I don't think you need to rev the client API. You can add fields to the
end of svn_wc_status2_t (that's even documented!).

This chage seems pretty straight forward to me. Just add the fields you
need, and catch the entryprops in change_file_prop and change_dir_prop in
libsvn_wc/status.c.  The URL field is a little tricky because of switched
entries.  For entries that exist in the WC, it is readily available.  For
added paths, there must be a nearest ancestor in the WC with an URL that
you can extend.

Hope this gives some hints,
//Peter - who recently messed with status -u for locking...

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

Re: [Subclipse-dev] Enhancement needed in svn status -u

Posted by Mark Phippard <Ma...@softlanding.com>.
Daniel Rall <dl...@finemaltcoding.com> wrote on 08/10/2005 04:39:55 PM:

> On Fri, 05 Aug 2005, Mark Phippard wrote:
> 
> > This is something we critically need in Subclipse from JavaHL, but as 
I 
> > understand things the root of the problem is in the svn_client API. 
Let 
> > me step back and explain.
> 
> Mark, I may mis-understand here, or this may've been sent before all the
> discussion on the dev@subversion list occurred, but I thought JavaHL was
> simply failing to fully expose the APIs/info needed by Subclipse.

My initial thought/hope was that the client API had the info that was 
needed and JavaHL just needed to be revved to pass it back.  I thought 
that svn st -u -v was showing the revision of the item in the repository. 
Sussman pointed out to me on IRC that this is not the case.  We are 
currently working on a design proposal for revving the client API to 
include information about the repository item in the information it 
returns.  We are just trying to wrap our heads around it right now.

> Mark, if this is only a JavaHL change, I'm willing to make the changes,
> but would like you tell me what the expected Java API should look like.
> 
> If the svn_client API needs to be rev'd, it's probably more than I can
> bite off at this time, but my offer to adjust JavaHL stands after the
> svn_client API offers support for what Subclipse needs.

I think you offered at one time to take a look at creating a JavaHL API 
for svn info that would take an array of URL's.  If this was something 
that could be done (where it was not just hiding a series of roundtrips) 
then that could be useful as a stop gap until the status API is changed. 
Do you know if the client API for info of a URL could take an array of 
items?

Mark





_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. by IBM Email Security Management Services powered by MessageLabs. 
_____________________________________________________________________________

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