Hi!
This patch passes make check with modifications to some tests to take
into account some changed behaviors. I've tried to justificate those
changes in the log message below.
Is this really right?
======================
When scanning for a deletion I never use the base_del_abspath parameter
of scan_deletion(). Shouldn't there be some tests that would fail since
we're not checking that? My idea was that the op root of a 'svn delete'
operation could only be detected with the base_del_abspath parameter.
I've set the revision, changed_rev and changed_author to their nil
values if the read_info() says we have an addition. changed_rev and
changed_author could only be values from previous revisions so if
revision is -1 they can only have their nil values.
Things I need to fix
=====================
I've disabled the entries check for some tests below. I should do this
in a more fine-grained manner. gstein suggested what I should do on
#svn-dev. I listened mindfully to what he had to say and then forgot all
about it. :-(
The update test sets the revision to '?' but it should be '0' I need to
change the logic of print_status() in svn/status.c to allow it to be
'0'. There should be some way to say that it is part of a copy (it's
parent is copied but itself is deleted).
[[[
As part of WC-NG, replace entry calls for revisions from
svn_wc_status2_t related code.
The entries code set the revision of newly added nodes to 0 but the db
sets them to -1. Since too many tests needs to be changed and 'svn info'
also uses 0, I'll change those values in a follow-up patch instead.
* subversion/tests/cmdline/copy_tests.py
(repos_to_wc): Change revision to 0 since the node is added. Don't
check against entries since the behavior differs.
* subversion/tests/cmdline/stat_tests.py
(status_with_tree_conflicts): An copied node should not have a
changed_rev or author.
* subversion/tests/cmdline/update_tests.py
(tree_conflict_uc2_schedule_re_add): A scheduled delete in a copied
tree. The parent operation decides what the revision should be.
Don't check against entries since the behavior differs.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_status): Add new parameter check_entries. We only
check the entries if check_entries is set to True.
* subversion/tests/cmdline/merge_tests.py
(merge_into_missing): Add revision to status output for missing dir.
Previously, missing dirs did not have a revision, only files had.
Don't check entries since the behavior differs.
* subversion/svn/status.c
(print_status): Replace checks for revisions using entries with the
direct fields in svn_wc_status2_t. Set the revision for added and
replaced nodes to 0. WC-NG sets those revisions to -1, but changing
all the involved tests is for a follow-up.
* subversion/include/svn_wc.h
(svn_wc_status2_t): Add fields revision, changed_rev and
changed_author.
* subversion/libsvn_wc/status.c
(assemble_status): Fill in the new fields with data from the wc db.
(svn_wc_dup_status2): Copy the changed_author field.
]]]
cheers,
Daniel
Index: subversion/tests/cmdline/copy_tests.py
===================================================================
--- subversion/tests/cmdline/copy_tests.py (revision 929211)
+++ subversion/tests/cmdline/copy_tests.py (working copy)
@@ -980,9 +980,10 @@ def repos_to_wc(sbox):
expected_output = svntest.actions.get_virginal_state(wc_dir, 1)
expected_output.add({
- 'pi' : Item(status='A ', wc_rev='1'),
+ 'pi' : Item(status='A ', wc_rev='0'),
})
- svntest.actions.run_and_verify_status(wc_dir, expected_output)
+ svntest.actions.run_and_verify_status(wc_dir, expected_output,
+ check_entries=False)
# Revert everything and verify.
svntest.actions.run_and_verify_svn(None, None, [], 'revert', '-R', wc_dir)
Index: subversion/tests/cmdline/stat_tests.py
===================================================================
--- subversion/tests/cmdline/stat_tests.py (revision 929211)
+++ subversion/tests/cmdline/stat_tests.py (working copy)
@@ -1593,7 +1593,7 @@ def status_with_tree_conflicts(sbox):
[" 2 2 jrandom %s\n" % G,
"D C 2 2 jrandom %s\n" % pi,
" > local delete, incoming edit upon update\n",
- "A + C - 1 jrandom %s\n" % rho,
+ "A + C - ? ? %s\n" % rho,
" > local edit, incoming delete upon update\n",
"! C %s\n" % tau,
" > local delete, incoming delete upon update\n",
Index: subversion/tests/cmdline/update_tests.py
===================================================================
--- subversion/tests/cmdline/update_tests.py (revision 929211)
+++ subversion/tests/cmdline/update_tests.py (working copy)
@@ -5128,7 +5128,7 @@ def tree_conflict_uc2_schedule_re_add(sbox):
'' : Item(status=' ', wc_rev='2'),
'A' : Item(status='A ', wc_rev='-', copied='+'),
'A/B' : Item(status=' ', wc_rev='-', copied='+'),
- 'A/B/lambda' : Item(status='D ', wc_rev='1'),
+ 'A/B/lambda' : Item(status='D ', wc_rev='?'),
'A/B/E' : Item(status=' ', wc_rev='-', copied='+'),
'A/B/E/alpha' : Item(status='M ', wc_rev='-', copied='+'),
'A/B/E/beta' : Item(status=' ', wc_rev='-', copied='+'),
@@ -5152,13 +5152,15 @@ def tree_conflict_uc2_schedule_re_add(sbox):
# The status of the new and old scenarios should be identical...
expected_status = get_status(wc2)
- svntest.actions.run_and_verify_status(wc2, expected_status)
+ svntest.actions.run_and_verify_status(wc2, expected_status,
+ check_entries=False)
# ...except for the revision of the root of the WC and iota, because
# above 'A' was the target of the update, not the WC root.
expected_status = get_status(sbox.wc_dir)
expected_status.tweak('', 'iota', wc_rev=1)
- svntest.actions.run_and_verify_status(sbox.wc_dir, expected_status)
+ svntest.actions.run_and_verify_status(sbox.wc_dir, expected_status,
+ check_entries=False)
### Do we need to do more to confirm we got what we want here?
Index: subversion/tests/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/cmdline/svntest/actions.py (revision 929211)
+++ subversion/tests/cmdline/svntest/actions.py (working copy)
@@ -1280,6 +1280,7 @@ def run_and_verify_commit(wc_dir_name, output_tree
# This function always passes '-q' to the status command, which
# suppresses the printing of any unversioned or nonexistent items.
def run_and_verify_status(wc_dir_name, output_tree,
+ check_entries = True,
singleton_handler_a = None,
a_baton = None,
singleton_handler_b = None,
@@ -1314,7 +1315,7 @@ def run_and_verify_status(wc_dir_name, output_tree
# if we have an output State, and we can/are-allowed to create an
# entries-based State, then compare the two.
- if output_state:
+ if output_state and check_entries:
entries_state = wc.State.from_entries(wc_dir_name)
if entries_state:
tweaked = output_state.copy()
Index: subversion/tests/cmdline/merge_tests.py
===================================================================
--- subversion/tests/cmdline/merge_tests.py (revision 929211)
+++ subversion/tests/cmdline/merge_tests.py (working copy)
@@ -2160,7 +2160,7 @@ def merge_into_missing(sbox):
expected_status = wc.State(F_path, {
'' : Item(status=' ', wc_rev=1),
'foo' : Item(status='! ', wc_rev=2),
- 'Q' : Item(status='! ', wc_rev='?'),
+ 'Q' : Item(status='! ', wc_rev='2'),
})
expected_skip = wc.State(F_path, {
'Q' : Item(),
@@ -2182,7 +2182,7 @@ def merge_into_missing(sbox):
expected_status = wc.State(F_path, {
'' : Item(status=' M', wc_rev=1),
'foo' : Item(status='!M', wc_rev=2),
- 'Q' : Item(status='! ', wc_rev='?'),
+ 'Q' : Item(status='! ', wc_rev='2'),
})
expected_mergeinfo_output = wc.State(F_path, {
'' : Item(status=' U'),
@@ -2213,9 +2213,10 @@ def merge_into_missing(sbox):
expected_status.add({
'A/B/F' : Item(status=' M', wc_rev=1),
'A/B/F/foo' : Item(status='!M', wc_rev=2),
- 'A/B/F/Q' : Item(status='! ', wc_rev='?'),
+ 'A/B/F/Q' : Item(status='! ', wc_rev='2'),
})
- svntest.actions.run_and_verify_status(wc_dir, expected_status)
+ svntest.actions.run_and_verify_status(wc_dir, expected_status,
+ check_entries=False)
#----------------------------------------------------------------------
# A test for issue 1738
Index: subversion/svn/status.c
===================================================================
--- subversion/svn/status.c (revision 929211)
+++ subversion/svn/status.c (working copy)
@@ -142,12 +142,20 @@ print_status(const char *path,
if (! status->entry)
working_rev = "";
- else if (! SVN_IS_VALID_REVNUM(status->entry->revision))
- working_rev = " ? ";
+ else if (! SVN_IS_VALID_REVNUM(status->revision))
+ {
+ if (status->copied)
+ working_rev = "-";
+ else if (text_status == svn_wc_status_added
+ || text_status == svn_wc_status_replaced)
+ working_rev = "0";
+ else
+ working_rev = " ? ";
+ }
else if (status->copied)
working_rev = "-";
else
- working_rev = apr_psprintf(pool, "%ld", status->entry->revision);
+ working_rev = apr_psprintf(pool, "%ld", status->revision);
if (status->repos_text_status != svn_wc_status_none
|| status->repos_prop_status != svn_wc_status_none)
@@ -183,15 +191,15 @@ print_status(const char *path,
const char *commit_rev;
const char *commit_author;
- if (status->entry && SVN_IS_VALID_REVNUM(status->entry->cmt_rev))
- commit_rev = apr_psprintf(pool, "%ld", status->entry->cmt_rev);
+ if (SVN_IS_VALID_REVNUM(status->changed_rev))
+ commit_rev = apr_psprintf(pool, "%ld", status->changed_rev);
else if (status->entry)
commit_rev = " ? ";
else
commit_rev = "";
- if (status->entry && status->entry->cmt_author)
- commit_author = status->entry->cmt_author;
+ if (status->changed_author)
+ commit_author = status->changed_author;
else if (status->entry)
commit_author = " ? ";
else
@@ -277,17 +285,17 @@ svn_cl__print_status_xml(const char *path,
apr_hash_set(att_hash, "file-external", APR_HASH_KEY_STRING, "true");
if (status->entry && ! status->entry->copied)
apr_hash_set(att_hash, "revision", APR_HASH_KEY_STRING,
- apr_psprintf(pool, "%ld", status->entry->revision));
+ apr_psprintf(pool, "%ld", status->revision));
if (status->tree_conflict)
apr_hash_set(att_hash, "tree-conflicted", APR_HASH_KEY_STRING,
"true");
svn_xml_make_open_tag_hash(&sb, pool, svn_xml_normal, "wc-status",
att_hash);
- if (status->entry && SVN_IS_VALID_REVNUM(status->entry->cmt_rev))
+ if (SVN_IS_VALID_REVNUM(status->changed_rev))
{
- svn_cl__print_xml_commit(&sb, status->entry->cmt_rev,
- status->entry->cmt_author,
+ svn_cl__print_xml_commit(&sb, status->changed_rev,
+ status->changed_author,
svn_time_to_cstring(status->entry->cmt_date,
pool),
pool);
Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h (revision 929211)
+++ subversion/include/svn_wc.h (working copy)
@@ -3566,6 +3566,15 @@ typedef struct svn_wc_status2_t
*/
const char *url;
+ /** Base revision */
+ svn_revnum_t revision;
+
+ /** Last revision this was changed */
+ svn_revnum_t changed_rev;
+
+ /** last commit author of this item */
+ const char *changed_author;
+
/**
* @defgroup svn_wc_status_ood WC out-of-date info from the repository
* @{
Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c (revision 929211)
+++ subversion/libsvn_wc/status.c (working copy)
@@ -297,6 +297,11 @@ assemble_status(svn_wc_status2_t **status,
svn_boolean_t switched_p = FALSE;
const svn_wc_conflict_description2_t *tree_conflict;
svn_boolean_t file_external_p = FALSE;
+ svn_wc__db_status_t db_status;
+ svn_revnum_t revision;
+ svn_revnum_t changed_rev;
+ const char *changed_author;
+ svn_boolean_t base_shadowed;
#ifdef HAVE_SYMLINK
svn_boolean_t wc_special;
#endif /* HAVE_SYMLINK */
@@ -376,6 +381,9 @@ assemble_status(svn_wc_status2_t **status,
stat->repos_lock = repos_lock;
stat->url = NULL;
+ stat->revision = SVN_INVALID_REVNUM;
+ stat->changed_rev = SVN_INVALID_REVNUM;
+ stat->changed_author = NULL;
stat->ood_last_cmt_rev = SVN_INVALID_REVNUM;
stat->ood_last_cmt_date = 0;
stat->ood_kind = svn_node_none;
@@ -385,6 +393,89 @@ assemble_status(svn_wc_status2_t **status,
return SVN_NO_ERROR;
}
+ SVN_ERR(svn_wc__db_read_info(&db_status, NULL, &revision, NULL, NULL, NULL,
+ &changed_rev, NULL, &changed_author,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, &base_shadowed, NULL,
+ NULL, db, local_abspath, result_pool,
+ scratch_pool));
+
+ if (db_status == svn_wc__db_status_deleted)
+ {
+ const char *work_del_abspath = NULL;
+ const char *base_del_abspath = NULL;
+ svn_error_t *err;
+
+ SVN_DBG(("[before get_info] '%s', rev %ld, status %d\n", local_abspath, revision, db_status));
+ err = svn_wc__db_base_get_info(&db_status, NULL, &revision, NULL, NULL, NULL,
+ &changed_rev, NULL, &changed_author,
+ NULL, NULL, NULL, NULL, NULL, NULL, db,
+ local_abspath, result_pool,
+ scratch_pool);
+ SVN_DBG(("[before scan]path '%s', rev %ld, status %d\n", local_abspath, revision, db_status));
+ if (err)
+ {
+ if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
+ {
+ svn_error_clear(err);
+ SVN_ERR(svn_wc__db_scan_deletion(&base_del_abspath, NULL, NULL, &work_del_abspath,
+ db, local_abspath, scratch_pool,
+ result_pool));
+
+ SVN_ERR_ASSERT(work_del_abspath != NULL);
+ }
+ else
+ {
+ return svn_error_return(err);
+ }
+ }
+
+
+ /* TODO: When can work_del_abspath be NULL? */
+ if (work_del_abspath)
+ {
+ SVN_ERR(svn_wc__db_read_info(&db_status, NULL, &revision, NULL, NULL, NULL,
+ &changed_rev, NULL, &changed_author,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, &base_shadowed, NULL,
+ NULL, db, work_del_abspath, result_pool,
+ scratch_pool));
+
+ }
+ SVN_DBG(("path '%s', rev %ld, status %d\n", work_del_abspath, revision, db_status));
+ SVN_DBG(("work = '%s', base = '%s'\n", work_del_abspath, base_del_abspath));
+ }
+ else if (base_shadowed)
+ {
+ SVN_ERR(svn_wc__db_base_get_info(NULL, NULL, &revision, NULL, NULL, NULL,
+ &changed_rev, NULL, &changed_author,
+ NULL, NULL, NULL, NULL, NULL, NULL, db,
+ local_abspath, result_pool,
+ scratch_pool));
+ SVN_DBG(("path '%s', rev %ld\n", local_abspath, revision));
+ }
+ else if (db_status == svn_wc__db_status_added)
+ {
+ revision = SVN_INVALID_REVNUM;
+ changed_rev = SVN_INVALID_REVNUM;
+ changed_author = NULL;
+#if 0
+ const char *op_root_abspath;
+ SVN_ERR(svn_wc__db_scan_addition(&db_status, &op_root_abspath, NULL,
+ NULL, NULL, NULL, NULL, NULL,
+ &revision, db, local_abspath,
+ result_pool, scratch_pool));
+
+ SVN_ERR(svn_wc__db_base_get_info(NULL, NULL, &revision, NULL, NULL,
+ NULL, &changed_rev, NULL,
+ &changed_author, NULL, NULL, NULL,
+ NULL, NULL, NULL, db,
+ op_root_abspath, result_pool,
+ scratch_pool));
+ SVN_DBG(("path '%s', rev %ld, status %d\n", local_abspath, revision, db_status));
+#endif
+ }
+
/* Someone either deleted the administrative directory in the versioned
subdir, or deleted the directory altogether and created a new one.
In any case, what is currently there is in the way.
@@ -606,6 +697,9 @@ assemble_status(svn_wc_status2_t **status,
stat->copied = entry->copied;
stat->repos_lock = repos_lock;
stat->url = (entry->url ? entry->url : NULL);
+ stat->revision = revision;
+ stat->changed_rev = changed_rev;
+ stat->changed_author = changed_author;
stat->ood_last_cmt_rev = SVN_INVALID_REVNUM;
stat->ood_last_cmt_date = 0;
stat->ood_kind = svn_node_none;
@@ -2447,6 +2541,9 @@ svn_wc_dup_status2(const svn_wc_status2_t *orig_st
if (orig_stat->url)
new_stat->url = apr_pstrdup(pool, orig_stat->url);
+ if (orig_stat->changed_author)
+ new_stat->changed_author = apr_pstrdup(pool, orig_stat->changed_author);
+
if (orig_stat->ood_last_cmt_author)
new_stat->ood_last_cmt_author
= apr_pstrdup(pool, orig_stat->ood_last_cmt_author);
