You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pa...@softlanding.com> on 2006/09/28 18:15:24 UTC
Re: svn commit: r21685 - in trunk/subversion: libsvn_wc tests/cmdline
dlr@tigris.org wrote on 09/27/2006 06:03:14 PM:
> Author: dlr
> Date: Wed Sep 27 15:03:14 2006
> New Revision: 21685
>
> Log:
> Fix issue #2533 (really this time), making 'svn status -u' show
> remotely changed properties on the root of a working copy (now without
> pesky double status output for paths at the root of the WC with
> '-v'!).
>
> This commit reverts portions of r21638.
>
> * subversion/libsvn_wc/status.c
> (close_directory): Store remote prop changes for the root of our
> working copy as flag's on the edit baton's anchor_status field,
> rather than as status changes of the current directory (which
> proved problematic for the algorithms used for status output).
>
> (tweak_statushash): Undo skip of the test for issue #2122 on the
> root directory (added by r21638).
>
> * subversion/tests/cmdline/stat_tests.py
> (test_list): Remove the XFail wrapper from the
> status_update_with_incoming_props() test.
>
> Patch by: lgo
> me
> Found by: pburba
>
>
> Modified:
> trunk/subversion/libsvn_wc/status.c
> trunk/subversion/tests/cmdline/stat_tests.py
>
> Modified: trunk/subversion/libsvn_wc/status.c
> URL: http://svn.collab.
> net/viewvc/svn/trunk/subversion/libsvn_wc/status.c?
> pathrev=21685&r1=21684&r2=21685
>
==============================================================================
> --- trunk/subversion/libsvn_wc/status.c (original)
> +++ trunk/subversion/libsvn_wc/status.c Wed Sep 27 15:03:14 2006
> @@ -1039,13 +1039,8 @@
> correctly in the first, that path would either be mentioned
> as an 'add' or not mentioned at all, depending on how we
> eventually fix the bugs in non-recursivity. See issue
> - #2122 for details.
> -
> - This behavior isn't appropriate for the root directory of our
> - WC, which may have properties which were changed remotely
> - (see issue #2533). */
> - if ((is_dir_baton && ((struct dir_baton *) baton)->parent_baton)
&&
> - repos_text_status != svn_wc_status_added)
> + #2122 for details. */
> + if (repos_text_status != svn_wc_status_added)
> return SVN_NO_ERROR;
>
> /* Use the public API to get a statstruct, and put it into the
hash. */
> @@ -1583,18 +1578,27 @@
> repos_prop_status = db->prop_changed ? svn_wc_status_modified
: 0;
> }
>
> - /* Add this directory's properties to its parent directory's
> - status hash, or if it's the root of the WC, to its own status
> - hash. Note that tweak_statushash() won't do anything if
> - repos_text_status is not svn_wc_status_added.
> -
> - NOTE: When we add directory locking, we need to find a
> - directory lock here. */
> - SVN_ERR(tweak_statushash((pb ? pb : db), TRUE,
> - eb->adm_access,
> - db->path, TRUE,
> - repos_text_status,
> - repos_prop_status, NULL));
> + /* Maybe add this directory to its parent's status hash. Note
> + that tweak_statushash won't do anything if repos_text_status
> + is not svn_wc_status_added. */
> + if (pb)
> + {
> + /* ### When we add directory locking, we need to find a
> + ### directory lock here. */
> + SVN_ERR(tweak_statushash(pb, TRUE,
> + eb->adm_access,
> + db->path, TRUE,
> + repos_text_status,
> + repos_prop_status, NULL));
> + }
> + else
> + {
> + /* We're editing the root dir of the WC. As its repos
> + status info isn't otherwise set, set it directly to
> + trigger invocation of the status callback below. */
> + eb->anchor_status->repos_prop_status = repos_prop_status;
> + eb->anchor_status->repos_text_status = repos_text_status;
Hello Gents,
Please don't kill the messenger, but I found another problem with the
latest fix for issue #2533.
When eb->anchor_status->repos_prop_status and
eb->anchor_status->repos_text_status are set above, the values of
repos_prop_status repos_text_status may be 0 (from lines 1573, 1577, or
1578). svn_wc_status_kind explicitly starts its enumeration at 1, so this
value is invalid. The most obvious ill-effect this has is with svn st -u
-v --xml, which trips the abort() on line 77 in generate_status_desc() in
svn/status.c.
The attached patch fixes the problem by using svn_wc_status_none rather
than 0 on lines 1573, 1577, and 1578. I also added a regression test for
this by expanding status_update_with_incoming_props.
This patch contains one change that is somewhat unrelated to r21685: In
the compare_unordered_output stat test, the way the output of
svntest.actions.run_and_verify_svn() is tested for correctness has
changed. Instead of passing the expected output, I examine the output
separately, without sensitivity to the order in which status reports the
paths. For an explanation of why this is necessary see:
http://svn.haxx.se/dev/archive-2006-08/0669.shtml.
I can remove this from the patch and commit it separately if you all want,
but the tests won't pass without it for me on XP and it's a fairly small
change so I thought I'd include it. Ultimately a more robust solution to
this problem is to add another argument to run_and_verify_svn() to tell it
if we care about order, but I don't really have time right now to update
it's 983 callers. It's on my TODO list though.
Anyway, I can commit this later today, but wanted to run it by you both
since you've been spending some time in libsvn_wc/status.c lately.
Paul B.
[[[
Fix r21685: svn st -u --xml aborts when root of WC has prop changes.
* subversion/libsvn_wc/status.c
(close_directory): Don't use 0 when setting svn_wc_status_kind enums
as this is an invalid value, use svn_wc_status_none instead.
* subversion/tests/cmdline/stat_tests.py
Import wc from svntest.
(status_update_with_incoming_props): Expand test to check for svn
st -u --xml abort. Use new compare_unordered_output() helper to
desensitize the test to the order of paths reported by status.
* subversion/tests/cmdline/svntest/main.py
(compare_unordered_output): New utility to compare lines of output
without sensitivity to their order.
]]]
Re: svn commit: r21685 - in trunk/subversion: libsvn_wc tests/cmdline
Posted by Daniel Rall <dl...@collab.net>.
On Thu, 28 Sep 2006, Paul Burba wrote:
...
> Please don't kill the messenger, but I found another problem with the
> latest fix for issue #2533.
...
pburba and lgo and I discussed this over IRC. Things looked good, and
a change was committed in r21696.
All changes related to issue #2533 are up for backport on the
'1.4.x-issue-2533' branch.