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

Re: [PATCH] Was: Enhancement needed in svn status -u

From: Paul Burba <paulb_at_softlanding.com>
Date: 2006-10-02 21:44:43 CEST

"Daniel L. Rall" <dlr@collab.net> wrote on 09/29/2005 07:17:17 PM:

> On Wed, 28 Sep 2005, Paul Burba wrote:
>
> > "Daniel L. Rall" <dlr@finemaltcoding.com> wrote on 09/28/2005 12:58:20
AM:
> >
> > > On Mon, 26 Sep 2005, Paul Burba wrote:
> > >
> > > > Here is the latest patch for improved out of date info when using
svn
> > > > status -u.
> > > >
> > > > The background on all of this can be found here:
> > > > http://svn.haxx.se/dev/archive-2005-08/0257.shtml
> > > ...
>
> Committed with a few formatting adjustments in r16344.

Dan,

A blast from the past...

Recall that r16344 was the ultimate solution for providing out of date
info about a WC and that this originally arose due to problems with the
Subclipse Synchronize view that Mark Phippard described in
http://svn.haxx.se/subdev/archive-2005-08/0020.shtml. Recently Mark asked
me to look into a several problems/limitations that still adversely affect
the Synchronize view, these are:

1) Unless a directory has a pending child
   addition/deletion, or there is a prop change on the
   directory itself, the out of date (ood_*) fields
   in svn_wc_status2_t are not set, or are incorrectly
   set.

2) The out of date (ood_*) fields for the root of WC
   are *never* set.

3) OOD info never bubbles up beyond an out of date
   path's immediate parent (if even that). E.g. if
   WC/A/B/E/beta is out of date, A, and A/B don't
   have any ood info, and as described in #1, A/B/E
   might not have any either.

4) The OOD_last_cmt_rev field for deleted paths is
   always SVN_INVALID_REVNUM.

The attached patch attempts to address all of these issues:

1) This problem results from a flaw in my original patch which was the
basis for r16344 (the fault is definitely in my original code, not any
change you made). When we call tweak_statushash() for a dir_baton in
close_directory() we are passing the db->parent_baton, but the ood info
for dir_baton is in itself. So when tweak_statushash() sets the ood info
on the parent_baton it just resets all ood fields to the parent's existing
values. To fix this I added a second baton arg to tweak_statushash so the
ood info is available.

2) Following your and Lieven's example from r21685, I set the
eb->anchor_status->ood_* fields directly in close_directory().

3) This is closely related to #1: We only call tweak_statushash() in
close_directory() in the first place if a dir is added, deleted, or has a
prop change. I changed this to also make the call if
dir_baton->ood_last_commit_rev is set to anything but the default value of
SVN_INVALID_REVNUM.

4) This was a bit trickier to solve. I saw no straightforward way to find
out the revision an out of date path was deleted within libsvn_wc, so I
added a new function to libsvn_repos: svn_repos_deleted_rev(). I then use
this new function in reporter.c's delta_dirs() and update_entry() to get
the rev a path was deleted and then pass this info as the revision
argument to the svn_delta_editor_t's delete_entry() callback. I'm not
totally clear from this comment how the revision arg is supposed to be
used, currently we just pass SVN_INVALID_REVNUM. The doc string for
delete_entry() says:

  /** Remove the directory entry named @a path, a child of the directory
   * represented by @a parent_baton. If @a revision is set, it is used as
a
   * sanity check to ensure that you are removing the revision of @a path
   * that you really think you are.
   *
   * All allocations should be performed in @a pool.
   */

But I'm not sure what a "sanity check" implies exactly. I'd like to think
that what I'm doing (passing the rev the path was deleted) is compatible
with the doc string, but it seems *too* easy that an essentially unused
arg already exists and does just what I require :-)

Assuming my changes to libsvn_repos are ok, there is still the problem of
older servers not providing this information. My solution is to look to
deleted path's parent folder and use it's out of date info. There is some
chance that this info is actually correct, and even if it is wrong it
would still be valid in the sense that it is a revision higher than the
revision the path was deleted. Re the Subclipse Synchronize use case this
is at least somewhat helpful. And since the present alternative is no
info at all (i.e. SVN_INVALID_REVNUM) this slightly flawed alternative
seems at least a partial improvement.
 
> I have written code to expose this additional status information via the
> JavaHL bindings (org.tigris.subversion.javahl.Status), and am in the
process
> of figuring out how to add a unit test for that to
> BasicTests.testBasicStatus().

Did you create a test for this? I didn't see anything committed, but if
you have any significant work in progress could I have a copy? Problem #1
above highlights the need for a real test of this functionality.

If anyone has some time to look at this I'd appreciate it, thanks,

Paul B.

[[[
Further improvements to status information on working copy items which
are out of date compared to the repository.

Follow-up to r16344 (and its subsequent follow-ups: r16494, 16784, 16796,
16829, and 17844).

* subversion/include/svn_repos.h
  (svn_repos_deleted_rev): New function to find the revision a path was
  most recently deleted within a give revision range.

* subversion/libsvn_repos/reporter.c
  (update_entry, delta_dirs): Use the new function svn_repos_deleted_rev()
  to determine the revision deleted paths were deleted and pass this to
  the delete_entry callback rather than defaulting to SVN_INVALID_REVNUM.

* subversion/libsvn_repos/rev_hunt.c
  (svn_repos_deleted_rev): New function definition.

* subversion/libsvn_wc/status.c
  (tweak_statushash): Add second baton argument which contains the out
  of date info for a dir baton when tweaking that baton's parent. Add
  another argument to identify the revision a path was deleted when
  handling deletes. When deleting paths: Construct the correct url for
  the path and record deleted path's deleted revision in the path's
  svn_wc_status2_t structure. For pre-1.5 servers, which don't provide
  the deleted revision, use the parent's last committed rev as a best
  guess.
  (delete_entry, close_file): Supply new args to tweak_statushash()
  calls.
  (close_directory): Tweak status for directories even when the only
  change is that they have and an out of date descendents. Supply new
  args to tweak_statushash() call.
]]]

P.S. In testing this patch I used the following hack to make the debug
version of svn status show all the OOD info on the command line. It also
adds a temporary status test to setup various out of date scenarios. It
may prove useful to anyone who tries out this patch:

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

Received on Mon Oct 2 21:45:09 2006

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.