You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2010/02/23 16:21:09 UTC
svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions
subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
Author: philip
Date: Tue Feb 23 15:21:02 2010
New Revision: 915378
URL: http://svn.apache.org/viewvc?rev=915378&view=rev
Log:
Remove svn_wc__entry_modify2 from svn_wc__db_temp_op_delete.
* subversion/libsvn_wc/wc_db.c
(svn_wc__db_temp_op_delete): Use a new implementation.
(db_working_get_status, db_working_update_presence,
db_working_actual_remove, db_working_insert, is_root_of_copy): New.
* subversion/libsvn_wc/wc-queries.sql
(STMT_UPDATE_WORKING_PRESENCE, STMT_UPDATE_WORKING_KIND,
STMT_INSERT_WORKING_NODE_FROM_BASE_NODE): New.
* notes/wc-ng/transitions: Add note about obstructed.
Modified:
subversion/trunk/notes/wc-ng/transitions
subversion/trunk/subversion/libsvn_wc/wc-queries.sql
subversion/trunk/subversion/libsvn_wc/wc_db.c
Modified: subversion/trunk/notes/wc-ng/transitions
URL: http://svn.apache.org/viewvc/subversion/trunk/notes/wc-ng/transitions?rev=915378&r1=915377&r2=915378&view=diff
==============================================================================
--- subversion/trunk/notes/wc-ng/transitions (original)
+++ subversion/trunk/notes/wc-ng/transitions Tue Feb 23 15:21:02 2010
@@ -151,3 +151,6 @@
### commit time, the copy is performed, and nothing more. if you
### wanted to delete this node, then switch it to depth=empty
### and presence=not-present.
+
+Pre-centralisation directories can be obstructed, any such nodes can
+be treated as presence=normal.
Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=915378&r1=915377&r2=915378&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Tue Feb 23 15:21:02 2010
@@ -235,6 +235,14 @@
update working_node set depth = ?3
where wc_id = ?1 and local_relpath = ?2;
+-- STMT_UPDATE_WORKING_PRESENCE
+update working_node set presence = ?3
+where wc_id = ?1 and local_relpath =?2;
+
+-- STMT_UPDATE_WORKING_KIND
+update working_node set kind = ?3
+where wc_id = ?1 and local_relpath =?2;
+
-- STMT_LOOK_FOR_WORK
SELECT id FROM WORK_QUEUE LIMIT 1;
@@ -303,6 +311,15 @@
/* NOTE: ?6 is duplicated because we insert the same value in two columns. */
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15);
+-- STMT_INSERT_WORKING_NODE_FROM_BASE_NODE
+INSERT INTO WORKING_NODE (
+ wc_id, local_relpath, parent_relpath, presence, kind, checksum,
+ translated_size, changed_rev, changed_date, changed_author, depth,
+ symlink_target, last_mod_time )
+SELECT wc_id, local_relpath, parent_relpath, presence, kind, checksum,
+ translated_size, changed_rev, changed_date, changed_author, depth,
+ symlink_target, last_mod_time FROM BASE_NODE
+WHERE wc_id = ?1 AND local_relpath = ?2;
/* ------------------------------------------------------------------------- */
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=915378&r1=915377&r2=915378&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue Feb 23 15:21:02 2010
@@ -2130,6 +2130,63 @@
}
+/* A WORKING version of svn_wc__db_base_get_info, stripped down to
+ just the status column. */
+static svn_error_t *
+db_working_get_status(svn_wc__db_status_t *status,
+ svn_wc__db_t *db,
+ const char *local_abspath,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ svn_wc__db_pdh_t *pdh;
+ const char *local_relpath;
+ svn_sqlite__stmt_t *stmt;
+ svn_boolean_t have_row;
+ svn_error_t *err = SVN_NO_ERROR;
+
+ SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
+
+ SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
+ svn_sqlite__mode_readonly,
+ scratch_pool, scratch_pool));
+ VERIFY_USABLE_PDH(pdh);
+
+ SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
+ STMT_SELECT_WORKING_NODE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", pdh->wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__step(&have_row, stmt));
+
+ if (have_row)
+ {
+ svn_wc__db_kind_t node_kind = svn_sqlite__column_token(stmt, 1,
+ kind_map);
+
+ *status = svn_sqlite__column_token(stmt, 0, presence_map);
+
+ if (node_kind == svn_wc__db_kind_subdir
+ && *status == svn_wc__db_status_normal)
+ {
+ /* We're looking at the subdir record in the *parent* directory,
+ which implies per-dir .svn subdirs. We should be looking
+ at the subdir itself; therefore, it is missing or obstructed
+ in some way. Inform the caller. */
+ *status = svn_wc__db_status_obstructed;
+ }
+ }
+ else
+ {
+ err = svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
+ _("The node '%s' was not found."),
+ svn_dirent_local_style(local_abspath,
+ scratch_pool));
+ }
+
+
+ return svn_error_compose_create(err, svn_sqlite__reset(stmt));
+}
+
+
svn_error_t *
svn_wc__db_base_get_prop(const svn_string_t **propval,
svn_wc__db_t *db,
@@ -3136,66 +3193,317 @@
return SVN_NO_ERROR;
}
-svn_error_t *
-svn_wc__db_temp_op_delete(svn_wc__db_t *db,
- const char *local_abspath,
- apr_pool_t *scratch_pool)
+/* Update the working node for LOCAL_ABSPATH setting presence=STATUS */
+static svn_error_t *
+db_working_update_presence(svn_wc__db_status_t status,
+ svn_wc__db_t *db,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool)
{
- const svn_wc_entry_t *entry;
- svn_error_t *err;
+ svn_wc__db_pdh_t *pdh;
+ const char *local_relpath;
+ wcroot_t *wcroot;
+ svn_sqlite__stmt_t *stmt;
+ svn_sqlite__db_t *sdb;
- /* ### Initial implementation: Copy the entry based code here. This will be
- more logical when we look at BASE_NODE and WORKING_NODE */
+ SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
+ SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
+ svn_sqlite__mode_readwrite,
+ scratch_pool, scratch_pool));
+ VERIFY_USABLE_PDH(pdh);
+ wcroot = pdh->wcroot;
+ sdb = wcroot->sdb;
- err = svn_wc__get_entry(&entry, db, local_abspath, FALSE, svn_node_unknown,
- FALSE, scratch_pool, scratch_pool);
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb, STMT_UPDATE_WORKING_PRESENCE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__bind_token(stmt, 3, presence_map, status));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+
+ flush_entries(pdh);
- if (err && err->apr_err == SVN_ERR_NODE_UNEXPECTED_KIND)
+ /* ### Parent stub? I don't know; I'll punt for now as it passes
+ the regression tests as is and the problem will evaporate
+ when the db is centralised. */
+
+ return SVN_NO_ERROR;
+}
+
+/* Delete working and actual nodes for LOCAL_ABSPATH */
+static svn_error_t *
+db_working_actual_remove(svn_wc__db_t *db,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool)
+{
+ svn_wc__db_pdh_t *pdh;
+ const char *local_relpath;
+ wcroot_t *wcroot;
+ svn_sqlite__db_t *sdb;
+ svn_sqlite__stmt_t *stmt;
+
+ SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
+ SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
+ svn_sqlite__mode_readwrite,
+ scratch_pool, scratch_pool));
+
+ VERIFY_USABLE_PDH(pdh);
+ wcroot = pdh->wcroot;
+ sdb = wcroot->sdb;
+
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb, STMT_DELETE_WORKING_NODE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb, STMT_DELETE_ACTUAL_NODE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+
+ flush_entries(pdh);
+
+ if (!strcmp(local_relpath, ""))
{
- svn_error_clear(err);
- err = NULL;
+ /* ### Delete parent stub. Remove when db is centralised. */
+ SVN_ERR(navigate_to_parent(&pdh, db, pdh, svn_sqlite__mode_readwrite,
+ scratch_pool));
+ local_relpath = svn_dirent_basename(local_abspath, NULL);
+ VERIFY_USABLE_PDH(pdh);
+ wcroot = pdh->wcroot;
+ sdb = wcroot->sdb;
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb, STMT_DELETE_WORKING_NODE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+ flush_entries(pdh);
}
- else
- SVN_ERR(err);
- if (entry->schedule == svn_wc_schedule_add)
+ return SVN_NO_ERROR;
+}
+
+/* Insert a working node for LOCAL_ABSPATH with presence=STATUS. */
+static svn_error_t *
+db_working_insert(svn_wc__db_status_t status,
+ svn_wc__db_t *db,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool)
+{
+ svn_wc__db_pdh_t *pdh;
+ const char *local_relpath;
+ wcroot_t *wcroot;
+ svn_sqlite__stmt_t *stmt;
+ svn_sqlite__db_t *sdb;
+
+ SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
+ SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
+ svn_sqlite__mode_readwrite,
+ scratch_pool, scratch_pool));
+ VERIFY_USABLE_PDH(pdh);
+ wcroot = pdh->wcroot;
+ sdb = wcroot->sdb;
+
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
+ STMT_INSERT_WORKING_NODE_FROM_BASE_NODE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+
+ /* ### Do update as part of the insert so that there is only one query? */
+ SVN_ERR(db_working_update_presence(status, db, local_abspath, scratch_pool));
+
+ flush_entries(pdh);
+
+ if (!strcmp(local_relpath, ""))
{
- svn_boolean_t deleted;
+ /* ### Insert parent stub. Remove when db is centralised. */
+ SVN_ERR(navigate_to_parent(&pdh, db, pdh, svn_sqlite__mode_readwrite,
+ scratch_pool));
+ local_relpath = svn_dirent_basename(local_abspath, NULL);
+ VERIFY_USABLE_PDH(pdh);
+ wcroot = pdh->wcroot;
+ sdb = wcroot->sdb;
+ /* ### Should the parent stub have a full row like this? */
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
+ STMT_INSERT_WORKING_NODE_FROM_BASE_NODE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
+ STMT_UPDATE_WORKING_PRESENCE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__bind_token(stmt, 3, presence_map, status));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+ SVN_ERR(svn_sqlite__get_statement(&stmt, sdb, STMT_UPDATE_WORKING_KIND));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__bind_text(stmt, 3, "subdir"));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+ flush_entries(pdh);
+ }
+
+ return SVN_NO_ERROR;
+}
- SVN_ERR(svn_wc__node_is_deleted(&deleted, db, local_abspath, scratch_pool));
+/* Set *ROOT_OF_COPY to TRUE if LOCAL_ABSPATH is the root of a copy,
+ to FALSE otherwise. */
+static svn_error_t*
+is_root_of_copy(svn_boolean_t *root_of_copy,
+ svn_wc__db_t *db,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool)
+{
+ svn_wc__db_status_t status;
+ const char *op_root_abspath;
+ const char *original_repos_relpath, *original_repos_root;
+ const char *original_repos_uuid;
+ svn_revnum_t original_revision;
+
+ SVN_ERR(svn_wc__db_scan_addition(&status, &op_root_abspath,
+ NULL, NULL, NULL,
+ &original_repos_relpath,
+ &original_repos_root,
+ &original_repos_uuid,
+ &original_revision,
+ db, local_abspath,
+ scratch_pool, scratch_pool));
- if (!deleted)
- {
- /* This case is easy.. Just delete the entry */
- SVN_ERR(svn_wc__entry_remove(db, local_abspath, scratch_pool));
- SVN_ERR(svn_wc__db_temp_forget_directory(db, local_abspath, scratch_pool));
+ SVN_ERR_ASSERT(status == svn_wc__db_status_added
+ || status == svn_wc__db_status_copied);
- return SVN_NO_ERROR;
+ *root_of_copy = op_root_abspath && !strcmp(local_abspath, op_root_abspath);
+
+ if (*root_of_copy && status == svn_wc__db_status_copied)
+ {
+ /* ### merge sets the wrong copyfrom when adding a tree and so
+ the root detection above is unreliable. I'm "fixing" it
+ here because I just need to detect whether this is an
+ instance of the merge bug, and that's easier than fixing
+ scan_addition or merge. */
+ const char *parent_abspath = svn_dirent_dirname(local_abspath,
+ scratch_pool);
+ svn_wc__db_status_t parent_status;
+ const char *parent_original_repos_relpath, *parent_original_repos_root;
+ const char *parent_original_repos_uuid;
+ svn_revnum_t parent_original_revision;
+ svn_error_t *err;
+
+ err = svn_wc__db_scan_addition(&parent_status,
+ NULL, NULL, NULL, NULL,
+ &parent_original_repos_relpath,
+ &parent_original_repos_root,
+ &parent_original_repos_uuid,
+ &parent_original_revision,
+ db, parent_abspath,
+ scratch_pool, scratch_pool);
+ if (err)
+ {
+ if (err->apr_err != SVN_ERR_WC_PATH_NOT_FOUND)
+ return svn_error_return(err);
+ /* It really is a root */
+ svn_error_clear(err);
}
+ else if (parent_status == svn_wc__db_status_copied
+ && original_revision == parent_original_revision
+ && !strcmp(original_repos_uuid, parent_original_repos_uuid)
+ && !strcmp(original_repos_root, parent_original_repos_root)
+ && !strcmp(original_repos_relpath,
+ svn_dirent_join(parent_original_repos_relpath,
+ svn_dirent_basename(local_abspath,
+ scratch_pool),
+ scratch_pool)))
+ /* An instance of the merge bug */
+ *root_of_copy = FALSE;
}
+ return SVN_NO_ERROR;
+}
+
+/* Delete LOCAL_ABSPATH. Implements the delete transition from
+ notes/wc-ng/transitions. */
+svn_error_t *
+svn_wc__db_temp_op_delete(svn_wc__db_t *db,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool)
+{
+ svn_error_t *err;
+ svn_boolean_t base_none, working_none, new_working_none;
+ svn_wc__db_status_t base_status, working_status, new_working_status;
+
+ err = svn_wc__db_base_get_info(&base_status,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ db, local_abspath, scratch_pool, scratch_pool);
+ if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
{
- svn_wc_entry_t tmp_entry;
+ base_none = TRUE;
+ svn_error_clear(err);
+ }
+ else if (! err)
+ base_none = FALSE;
+ else
+ return svn_error_return(err);
- tmp_entry.schedule = svn_wc_schedule_delete;
+ if (!base_none && base_status == svn_wc__db_status_absent)
+ return SVN_NO_ERROR; /* ### return an error? */
- /* Don't update the directory entries for locally added directories
- and directories that are missing. (The first scenario abort()s
- and the second gives a WC missing error) */
- if (entry->kind != svn_node_dir ||
- (entry->schedule != svn_wc_schedule_add && *entry->name == '\0'))
- SVN_ERR(svn_wc__entry_modify2(db, local_abspath, entry->kind,
- FALSE, &tmp_entry,
- SVN_WC__ENTRY_MODIFY_SCHEDULE,
- scratch_pool));
+ err = db_working_get_status(&working_status,
+ db, local_abspath, scratch_pool, scratch_pool);
+ if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
+ {
+ working_none = TRUE;
+ svn_error_clear(err);
+ }
+ else if (! err)
+ working_none = FALSE;
+ else
+ return svn_error_return(err);
- /* And update the parent stub */
- if (entry->kind == svn_node_dir)
- SVN_ERR(svn_wc__entry_modify2(db, local_abspath, svn_node_dir,
- TRUE, &tmp_entry,
- SVN_WC__ENTRY_MODIFY_SCHEDULE,
- scratch_pool));
+ if (base_none && working_none)
+ return SVN_NO_ERROR; /* ### return an error? */
+
+ new_working_none = working_none;
+ new_working_status = working_status;
+ if (working_none)
+ {
+ if (base_status == svn_wc__db_status_normal
+ || base_status == svn_wc__db_status_obstructed
+ || base_status == svn_wc__db_status_incomplete
+ || base_status == svn_wc__db_status_excluded)
+ {
+ new_working_none = FALSE;
+ new_working_status = svn_wc__db_status_base_deleted;
+ }
+ }
+ else if ((working_status == svn_wc__db_status_normal
+ || working_status == svn_wc__db_status_obstructed)
+ && (base_none || base_status == svn_wc__db_status_not_present))
+ {
+ svn_boolean_t root_of_copy;
+ SVN_ERR(is_root_of_copy(&root_of_copy, db, local_abspath, scratch_pool));
+ if (root_of_copy)
+ new_working_none = TRUE;
+ else
+ new_working_status = svn_wc__db_status_not_present;
}
+ else if (working_status == svn_wc__db_status_normal)
+ {
+ svn_boolean_t root_of_copy;
+ SVN_ERR(is_root_of_copy(&root_of_copy, db, local_abspath, scratch_pool));
+ if (root_of_copy)
+ new_working_status = svn_wc__db_status_base_deleted;
+ else
+ new_working_status = svn_wc__db_status_not_present;
+ }
+ else if (working_status == svn_wc__db_status_incomplete)
+ {
+ svn_boolean_t root_of_copy;
+ SVN_ERR(is_root_of_copy(&root_of_copy, db, local_abspath, scratch_pool));
+ if (root_of_copy)
+ new_working_none = TRUE;
+ }
+
+ if (new_working_none && !working_none)
+ SVN_ERR(db_working_actual_remove(db, local_abspath, scratch_pool));
+ else if (!new_working_none && working_none)
+ SVN_ERR(db_working_insert(new_working_status,
+ db, local_abspath, scratch_pool));
+ else if (!new_working_none && !working_none
+ && new_working_status != working_status)
+ SVN_ERR(db_working_update_presence(new_working_status,
+ db, local_abspath, scratch_pool));
+ /* ### else nothing to do, return an error? */
return SVN_NO_ERROR;
}
Re: svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions
subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Feb 23, 2010 at 10:21, <ph...@apache.org> wrote:
> Author: philip
> Date: Tue Feb 23 15:21:02 2010
> New Revision: 915378
>
> URL: http://svn.apache.org/viewvc?rev=915378&view=rev
> Log:
> Remove svn_wc__entry_modify2 from svn_wc__db_temp_op_delete.
>
> * subversion/libsvn_wc/wc_db.c
> (svn_wc__db_temp_op_delete): Use a new implementation.
I like the direction this is headed.
I'm think that we may want to promote this function to
svn_wc__db_op_delete() and document that op_delete is NON-RECURSIVE.
And further state, that all children must be deleted FIRST (and assert
that in the func!).
I think. Not entirely sure. For example, deleting children of a copied
subtree leaves nodes with the 'not-present' presence. So as you delete
all the children, this is what you'd set them to. HOWEVER, when you
finally delete the root of that copy, then you must re-traverse the
subtree: remove all nodes that have no underlying BASE node, and
convert any that *do* over to presence='base-deleted'. And that would
have to happen in a big ol' transaction.
But one nice thing about defining op_delete as non-recursive means
that it is interruptable. At any point, the user could stop, leaving
the working copy in a sane state.
>...
Skipping a review. I've reviewed the (current) code, and have a few
mods which I'll commit in a bit.
See my other note, however, for removing the db_working_get_status()
function in favor of sticking to read_info().
Cheers,
-g
Re: svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:
>> I want both the base and the working presence, the base presence comes
>> from base_get_info but I couldn't see how to reliably distinguish
>> working presence from base presence when using _read_info. I get
>> SVN_ERR_WC_PATH_NOT_FOUND for no base and no working, and
>> base_shadowed TRUE for base and working, but it was not clear how to
>> distinguish base and no working from no base and working.
>
> Greg?
>
> I think the idea was that you always should be able to tell the difference
> from the status?
>
> Deleting incomplete, excluded or absent should be a user error.
Deleting incomplete should be a user error?
If I have BASE normal and no WORKING I can delete it, but then a
subsequent update can make BASE incomplete. We have to support that
as far as I can see. If we do support it then BASE incomplete and
WORKING base-deleted is a valid state, so why not allow the delete to
make the transition to that state?
--
Philip
Re: svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions
subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 11, 2010 at 00:15, Greg Stein <gs...@gmail.com> wrote:
>...
>>> I want both the base and the working presence, the base presence comes
>>> from base_get_info but I couldn't see how to reliably distinguish
>>> working presence from base presence when using _read_info. I get
>>> SVN_ERR_WC_PATH_NOT_FOUND for no base and no working, and
>>> base_shadowed TRUE for base and working, but it was not clear how to
>>> distinguish base and no working from no base and working.
>>
>> Greg?
>>
>> I think the idea was that you always should be able to tell the difference
>> from the status?
>
> read_info() on its own... no, you cannot always tell. status=normal
> could refer to a BASE node, or a WORKING node (and base_shadowed could
> be FALSE for these two cases).
This comment is wrong. status=normal means an unmodified BASE node. If
there is a WORKING node, then another status will always be returned.
I'm about to remove db_working_get_status(), since I'm in that code to
fix an incorrect scan_addition call.
Cheers,
-g
Re: svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions
subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
Getting back to this...
On Wed, Feb 24, 2010 at 07:15, Bert Huijben <be...@qqmail.nl> wrote:
>...
>> >> +/* A WORKING version of svn_wc__db_base_get_info, stripped down to
>> >> + just the status column. */
>> >> +static svn_error_t *
>> >> +db_working_get_status(svn_wc__db_status_t *status,
>> >> + svn_wc__db_t *db,
>> >> + const char *local_abspath,
>> >> + apr_pool_t *result_pool,
>> >> + apr_pool_t *scratch_pool) {
>> >
>> > Why reimplement this whole function if a
>> >
>> > return svn_error_return(
>> > svn_wc__db_read_status(status, ..... several NULLs..., db,
>> local_abspath, scratch_pool...) would suffice?
>> >
>> > _read_info is optimized for the case with many NULLs and has tests
>> > on it. (And we didn't see performance reasons yet to do this same
>> > split for several other helpers)
>>
>> I want both the base and the working presence, the base presence comes
>> from base_get_info but I couldn't see how to reliably distinguish
>> working presence from base presence when using _read_info. I get
>> SVN_ERR_WC_PATH_NOT_FOUND for no base and no working, and
>> base_shadowed TRUE for base and working, but it was not clear how to
>> distinguish base and no working from no base and working.
>
> Greg?
>
> I think the idea was that you always should be able to tell the difference
> from the status?
read_info() on its own... no, you cannot always tell. status=normal
could refer to a BASE node, or a WORKING node (and base_shadowed could
be FALSE for these two cases).
However. Where this code is called *does* have the information on
whether a BASE node is present (the 'base_none' variable). Thus, it
can certainly distinguish the cases.
If read_info() fails with SVN_ERR_WC_PATH_NOT_FOUND, then great.
temp_op_delete() should not have been called in that case.
If base_none is TRUE, and read_info succeeds, then you have a WORKING node.
If base_none is FALSE, and read_info returns base_shadowed=TRUE, then
you got a WORKING presence; otherwise, a copy of the BASE presence.
> Deleting incomplete, excluded or absent should be a user error.
As stated later in the thread, incomplete should succeed (by clearing
out the incomplete node). I agree that excluded and absent should be
an error.
>...
Cheers,
-g
RE: svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Philip Martin [mailto:philip.martin@wandisco.com]
> Sent: woensdag 24 februari 2010 12:08
> To: Bert Huijben
> Cc: dev@subversion.apache.org; philip@apache.org
> Subject: Re: svn commit: r915378 - in /subversion/trunk: notes/wc-
> ng/transitions subversion/libsvn_wc/wc-queries.sql
> subversion/libsvn_wc/wc_db.c
>
> "Bert Huijben" <be...@vmoo.com> writes:
>
> >> +-- STMT_INSERT_WORKING_NODE_FROM_BASE_NODE
> >> +INSERT INTO WORKING_NODE (
> >> + wc_id, local_relpath, parent_relpath, presence, kind, checksum,
> >> + translated_size, changed_rev, changed_date, changed_author,
> depth,
> >> + symlink_target, last_mod_time )
> >> +SELECT wc_id, local_relpath, parent_relpath, presence, kind,
> checksum,
> >> + translated_size, changed_rev, changed_date, changed_author,
> depth,
> >> + symlink_target, last_mod_time FROM BASE_NODE WHERE wc_id = ?1
> >> AND
> >> +local_relpath = ?2;
> >
> > I'm not sure how generic this copy from base to working is. I think
> > it is only valid for a few of the presence states. (Maybe use a more
> > specific name?)
>
> I'd really like an SQL statement that allows me to set the presence as
> ?3 while pulling the other values from BASE, but I don't know if that
> is possible.
I think you should be able to use SELECT wc_id, local_relpath,
parent_relpath, ?3 AS presence, ...
for this case. (I know you can just select a constant for the insert, but I
didn't check if you can fetch the value from a bound value)
>
> >> +/* A WORKING version of svn_wc__db_base_get_info, stripped down to
> >> + just the status column. */
> >> +static svn_error_t *
> >> +db_working_get_status(svn_wc__db_status_t *status,
> >> + svn_wc__db_t *db,
> >> + const char *local_abspath,
> >> + apr_pool_t *result_pool,
> >> + apr_pool_t *scratch_pool) {
> >
> > Why reimplement this whole function if a
> >
> > return svn_error_return(
> > svn_wc__db_read_status(status, ..... several NULLs..., db,
> local_abspath, scratch_pool...) would suffice?
> >
> > _read_info is optimized for the case with many NULLs and has tests
> > on it. (And we didn't see performance reasons yet to do this same
> > split for several other helpers)
>
> I want both the base and the working presence, the base presence comes
> from base_get_info but I couldn't see how to reliably distinguish
> working presence from base presence when using _read_info. I get
> SVN_ERR_WC_PATH_NOT_FOUND for no base and no working, and
> base_shadowed TRUE for base and working, but it was not clear how to
> distinguish base and no working from no base and working.
Greg?
I think the idea was that you always should be able to tell the difference
from the status?
Deleting incomplete, excluded or absent should be a user error.
>
> > (The result_pool argument from your function is not necessary as
> > status is no structure that needs allocation)
>
> Noted.
>
> >> +/* Update the working node for LOCAL_ABSPATH setting
> presence=STATUS
> >> */
> >> +static svn_error_t * db_working_update_presence(svn_wc__db_status_t
> >> +status,
> >> + svn_wc__db_t *db,
> >> + const char *local_abspath,
> >> + apr_pool_t *scratch_pool)
> >
> > For which presence states does this function work?
>
> It uses presence_map.
>
> > I think the supported list should be in the docstring and asserted
> > on top of the function, like all other svn_wc__db functions do.
> >
> > (I reviewed the base-deleted case... I don't think it is safe for
> > any other presence?)
>
> It's used to set base-deleted or not-present. What do you mean by safe?
Passing absent, excluded, incomplete, would give a +- undefined database
state.
>
> >> +/* Insert a working node for LOCAL_ABSPATH with presence=STATUS. */
> >> +static svn_error_t * db_working_insert(svn_wc__db_status_t status,
> >> + svn_wc__db_t *db,
> >> + const char *local_abspath,
> >> + apr_pool_t *scratch_pool) {
> >
> > Which presences does this support?
> >
> > Only base-delete?
>
> I believe so, I could have hard-coded it.
>
> > It certainly can't handle cases where the node kind between BASE and
> > WORKING differ and many other cases.
> >
> >> +static svn_error_t*
> >> +is_root_of_copy(svn_boolean_t *root_of_copy,
> >> + svn_wc__db_t *db,
> >> + const char *local_abspath,
> >> + apr_pool_t *scratch_pool) {
> >> + svn_wc__db_status_t status;
> >> + const char *op_root_abspath;
> >> + const char *original_repos_relpath, *original_repos_root;
> >> + const char *original_repos_uuid;
> >> + svn_revnum_t original_revision;
> >> +
> >> + SVN_ERR(svn_wc__db_scan_addition(&status, &op_root_abspath,
> >> + NULL, NULL, NULL,
> >> + &original_repos_relpath,
> >> + &original_repos_root,
> >> + &original_repos_uuid,
> >> + &original_revision,
> >> + db, local_abspath,
> >> + scratch_pool, scratch_pool));
> >>
> >> - if (!deleted)
> >> - {
> >> - /* This case is easy.. Just delete the entry */
> >> - SVN_ERR(svn_wc__entry_remove(db, local_abspath,
> scratch_pool));
> >> - SVN_ERR(svn_wc__db_temp_forget_directory(db,
> local_abspath,
> >> scratch_pool));
> >> + SVN_ERR_ASSERT(status == svn_wc__db_status_added
> >> + || status == svn_wc__db_status_copied);
> >>
> >> - return SVN_NO_ERROR;
> >> + *root_of_copy = op_root_abspath && !strcmp(local_abspath,
> >> + op_root_abspath);
> >> +
> >> + if (*root_of_copy && status == svn_wc__db_status_copied)
> >> + {
> >> + /* ### merge sets the wrong copyfrom when adding a tree and
> so
> >> + the root detection above is unreliable. I'm "fixing"
> it
> >> + here because I just need to detect whether this is an
> >> + instance of the merge bug, and that's easier than
> fixing
> >> + scan_addition or merge. */
> >
> > Adds can be merged by design, so you don't just see this behavior
> > below merges!
>
> I saw an explict merge bug that caused op_root_abspath to have the
> wrong value. I didn't see that behaviour with add but perhaps the
> regresssion tests don't trigger it.
>
> I think I should rename this function. It's about making the "remove
> or set not-present" decision, I've been using "root of copy" but
> really it's "add or root of copy" versus "within copy".
In WC-1.0 we had a hard time describing simple copies. With WC-NG we can
describe more advanced copies and we certainly need more test here...
(I added a few simple ones to show current failures)
>
> > Calls like
> > svn cp ^/trunk A
> > svn cp ^/branches/B A/B
> >
> > give you two copy roots below each other (and you can have N of
> > them). You really need to loop/recurse upwards to find the place
> > where the copy is added to or replaces a base node.
> >
> > Each of these needs different handling on delete with WC-NG. (A/B
> > was not really handled that well via entries especially if it was a
> > replacement).
> >
> > This specific case also touches the issue that we can't describe a
> > local add below a copy yet. (I think you raised that issue yourself)
>
> I've also noticed that I left in the STMT_UPDATE_WORKING_KIND code
> which is not needed. It was temporary code I put in while trying to
> get the parent stub stuff to work.
I really like that your implementation can safely ignore those parent
stubs...
I was still thinking about how we should fix that to get this working the
right way... but for a recorded delete you can ignore those states and you
can't accidentally remove them if they are in the parent stub.
Bert
Re: svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@vmoo.com> writes:
>> +-- STMT_INSERT_WORKING_NODE_FROM_BASE_NODE
>> +INSERT INTO WORKING_NODE (
>> + wc_id, local_relpath, parent_relpath, presence, kind, checksum,
>> + translated_size, changed_rev, changed_date, changed_author, depth,
>> + symlink_target, last_mod_time )
>> +SELECT wc_id, local_relpath, parent_relpath, presence, kind, checksum,
>> + translated_size, changed_rev, changed_date, changed_author, depth,
>> + symlink_target, last_mod_time FROM BASE_NODE WHERE wc_id = ?1
>> AND
>> +local_relpath = ?2;
>
> I'm not sure how generic this copy from base to working is. I think
> it is only valid for a few of the presence states. (Maybe use a more
> specific name?)
I'd really like an SQL statement that allows me to set the presence as
?3 while pulling the other values from BASE, but I don't know if that
is possible.
>> +/* A WORKING version of svn_wc__db_base_get_info, stripped down to
>> + just the status column. */
>> +static svn_error_t *
>> +db_working_get_status(svn_wc__db_status_t *status,
>> + svn_wc__db_t *db,
>> + const char *local_abspath,
>> + apr_pool_t *result_pool,
>> + apr_pool_t *scratch_pool) {
>
> Why reimplement this whole function if a
>
> return svn_error_return(
> svn_wc__db_read_status(status, ..... several NULLs..., db, local_abspath, scratch_pool...) would suffice?
>
> _read_info is optimized for the case with many NULLs and has tests
> on it. (And we didn't see performance reasons yet to do this same
> split for several other helpers)
I want both the base and the working presence, the base presence comes
from base_get_info but I couldn't see how to reliably distinguish
working presence from base presence when using _read_info. I get
SVN_ERR_WC_PATH_NOT_FOUND for no base and no working, and
base_shadowed TRUE for base and working, but it was not clear how to
distinguish base and no working from no base and working.
> (The result_pool argument from your function is not necessary as
> status is no structure that needs allocation)
Noted.
>> +/* Update the working node for LOCAL_ABSPATH setting presence=STATUS
>> */
>> +static svn_error_t * db_working_update_presence(svn_wc__db_status_t
>> +status,
>> + svn_wc__db_t *db,
>> + const char *local_abspath,
>> + apr_pool_t *scratch_pool)
>
> For which presence states does this function work?
It uses presence_map.
> I think the supported list should be in the docstring and asserted
> on top of the function, like all other svn_wc__db functions do.
>
> (I reviewed the base-deleted case... I don't think it is safe for
> any other presence?)
It's used to set base-deleted or not-present. What do you mean by safe?
>> +/* Insert a working node for LOCAL_ABSPATH with presence=STATUS. */
>> +static svn_error_t * db_working_insert(svn_wc__db_status_t status,
>> + svn_wc__db_t *db,
>> + const char *local_abspath,
>> + apr_pool_t *scratch_pool) {
>
> Which presences does this support?
>
> Only base-delete?
I believe so, I could have hard-coded it.
> It certainly can't handle cases where the node kind between BASE and
> WORKING differ and many other cases.
>
>> +static svn_error_t*
>> +is_root_of_copy(svn_boolean_t *root_of_copy,
>> + svn_wc__db_t *db,
>> + const char *local_abspath,
>> + apr_pool_t *scratch_pool) {
>> + svn_wc__db_status_t status;
>> + const char *op_root_abspath;
>> + const char *original_repos_relpath, *original_repos_root;
>> + const char *original_repos_uuid;
>> + svn_revnum_t original_revision;
>> +
>> + SVN_ERR(svn_wc__db_scan_addition(&status, &op_root_abspath,
>> + NULL, NULL, NULL,
>> + &original_repos_relpath,
>> + &original_repos_root,
>> + &original_repos_uuid,
>> + &original_revision,
>> + db, local_abspath,
>> + scratch_pool, scratch_pool));
>>
>> - if (!deleted)
>> - {
>> - /* This case is easy.. Just delete the entry */
>> - SVN_ERR(svn_wc__entry_remove(db, local_abspath, scratch_pool));
>> - SVN_ERR(svn_wc__db_temp_forget_directory(db, local_abspath,
>> scratch_pool));
>> + SVN_ERR_ASSERT(status == svn_wc__db_status_added
>> + || status == svn_wc__db_status_copied);
>>
>> - return SVN_NO_ERROR;
>> + *root_of_copy = op_root_abspath && !strcmp(local_abspath,
>> + op_root_abspath);
>> +
>> + if (*root_of_copy && status == svn_wc__db_status_copied)
>> + {
>> + /* ### merge sets the wrong copyfrom when adding a tree and so
>> + the root detection above is unreliable. I'm "fixing" it
>> + here because I just need to detect whether this is an
>> + instance of the merge bug, and that's easier than fixing
>> + scan_addition or merge. */
>
> Adds can be merged by design, so you don't just see this behavior
> below merges!
I saw an explict merge bug that caused op_root_abspath to have the
wrong value. I didn't see that behaviour with add but perhaps the
regresssion tests don't trigger it.
I think I should rename this function. It's about making the "remove
or set not-present" decision, I've been using "root of copy" but
really it's "add or root of copy" versus "within copy".
> Calls like
> svn cp ^/trunk A
> svn cp ^/branches/B A/B
>
> give you two copy roots below each other (and you can have N of
> them). You really need to loop/recurse upwards to find the place
> where the copy is added to or replaces a base node.
>
> Each of these needs different handling on delete with WC-NG. (A/B
> was not really handled that well via entries especially if it was a
> replacement).
>
> This specific case also touches the issue that we can't describe a
> local add below a copy yet. (I think you raised that issue yourself)
I've also noticed that I left in the STMT_UPDATE_WORKING_KIND code
which is not needed. It was temporary code I put in while trying to
get the parent stub stuff to work.
--
Philip
RE: svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
Posted by Bert Huijben <be...@vmoo.com>.
> -----Original Message-----
> From: philip@apache.org [mailto:philip@apache.org]
> Sent: dinsdag 23 februari 2010 16:21
> To: commits@subversion.apache.org
> Subject: svn commit: r915378 - in /subversion/trunk: notes/wc-ng/transitions
> subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db.c
>
> Author: philip
> Date: Tue Feb 23 15:21:02 2010
> New Revision: 915378
>
> URL: http://svn.apache.org/viewvc?rev=915378&view=rev
> Log:
> Remove svn_wc__entry_modify2 from svn_wc__db_temp_op_delete.
>
> * subversion/libsvn_wc/wc_db.c
> (svn_wc__db_temp_op_delete): Use a new implementation.
> (db_working_get_status, db_working_update_presence,
> db_working_actual_remove, db_working_insert, is_root_of_copy): New.
>
> * subversion/libsvn_wc/wc-queries.sql
> (STMT_UPDATE_WORKING_PRESENCE, STMT_UPDATE_WORKING_KIND,
> STMT_INSERT_WORKING_NODE_FROM_BASE_NODE): New.
>
> * notes/wc-ng/transitions: Add note about obstructed.
>
> Modified:
> subversion/trunk/notes/wc-ng/transitions
> subversion/trunk/subversion/libsvn_wc/wc-queries.sql
> subversion/trunk/subversion/libsvn_wc/wc_db.c
>
> Modified: subversion/trunk/notes/wc-ng/transitions
> URL: http://svn.apache.org/viewvc/subversion/trunk/notes/wc-
> ng/transitions?rev=915378&r1=915377&r2=915378&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/notes/wc-ng/transitions (original)
> +++ subversion/trunk/notes/wc-ng/transitions Tue Feb 23 15:21:02 2010
> @@ -151,3 +151,6 @@
> ### commit time, the copy is performed, and nothing more. if you
> ### wanted to delete this node, then switch it to depth=empty
> ### and presence=not-present.
> +
> +Pre-centralisation directories can be obstructed, any such nodes can be
> +treated as presence=normal.
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-
> queries.sql?rev=915378&r1=915377&r2=915378&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Tue Feb 23
> +++ 15:21:02 2010
> @@ -235,6 +235,14 @@
> update working_node set depth = ?3
> where wc_id = ?1 and local_relpath = ?2;
>
> +-- STMT_UPDATE_WORKING_PRESENCE
> +update working_node set presence = ?3
> +where wc_id = ?1 and local_relpath =?2;
> +
> +-- STMT_UPDATE_WORKING_KIND
> +update working_node set kind = ?3
> +where wc_id = ?1 and local_relpath =?2;
> +
> -- STMT_LOOK_FOR_WORK
> SELECT id FROM WORK_QUEUE LIMIT 1;
>
> @@ -303,6 +311,15 @@
> /* NOTE: ?6 is duplicated because we insert the same value in two columns.
> */ VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15);
>
> +-- STMT_INSERT_WORKING_NODE_FROM_BASE_NODE
> +INSERT INTO WORKING_NODE (
> + wc_id, local_relpath, parent_relpath, presence, kind, checksum,
> + translated_size, changed_rev, changed_date, changed_author, depth,
> + symlink_target, last_mod_time )
> +SELECT wc_id, local_relpath, parent_relpath, presence, kind, checksum,
> + translated_size, changed_rev, changed_date, changed_author, depth,
> + symlink_target, last_mod_time FROM BASE_NODE WHERE wc_id = ?1
> AND
> +local_relpath = ?2;
I'm not sure how generic this copy from base to working is. I think it is only valid for a few of the presence states. (Maybe use a more specific name?)
>
> /* ------------------------------------------------------------------------- */
>
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_
> db.c?rev=915378&r1=915377&r2=915378&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue Feb 23 15:21:02
> +++ 2010
> @@ -2130,6 +2130,63 @@
> }
>
>
> +/* A WORKING version of svn_wc__db_base_get_info, stripped down to
> + just the status column. */
> +static svn_error_t *
> +db_working_get_status(svn_wc__db_status_t *status,
> + svn_wc__db_t *db,
> + const char *local_abspath,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool) {
Why reimplement this whole function if a
return svn_error_return(
svn_wc__db_read_status(status, ..... several NULLs..., db, local_abspath, scratch_pool...) would suffice?
_read_info is optimized for the case with many NULLs and has tests on it. (And we didn't see performance reasons yet to do this same split for several other helpers)
(The result_pool argument from your function is not necessary as status is no structure that needs allocation)
> + svn_wc__db_pdh_t *pdh;
> + const char *local_relpath;
> + svn_sqlite__stmt_t *stmt;
> + svn_boolean_t have_row;
> + svn_error_t *err = SVN_NO_ERROR;
> +
> + SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
> +
> + SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
> + svn_sqlite__mode_readonly,
> + scratch_pool, scratch_pool));
> + VERIFY_USABLE_PDH(pdh);
> +
> + SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->wcroot->sdb,
> + STMT_SELECT_WORKING_NODE));
> + SVN_ERR(svn_sqlite__bindf(stmt, "is", pdh->wcroot->wc_id,
> + local_relpath)); SVN_ERR(svn_sqlite__step(&have_row, stmt));
> +
> + if (have_row)
> + {
> + svn_wc__db_kind_t node_kind = svn_sqlite__column_token(stmt, 1,
> + kind_map);
> +
> + *status = svn_sqlite__column_token(stmt, 0, presence_map);
> +
> + if (node_kind == svn_wc__db_kind_subdir
> + && *status == svn_wc__db_status_normal)
> + {
> + /* We're looking at the subdir record in the *parent* directory,
> + which implies per-dir .svn subdirs. We should be looking
> + at the subdir itself; therefore, it is missing or obstructed
> + in some way. Inform the caller. */
> + *status = svn_wc__db_status_obstructed;
> + }
> + }
> + else
> + {
> + err = svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> + _("The node '%s' was not found."),
> + svn_dirent_local_style(local_abspath,
> + scratch_pool));
> + }
> +
> +
> + return svn_error_compose_create(err, svn_sqlite__reset(stmt)); }
> +
> +
> svn_error_t *
> svn_wc__db_base_get_prop(const svn_string_t **propval,
> svn_wc__db_t *db, @@ -3136,66 +3193,317 @@
> return SVN_NO_ERROR;
> }
>
> -svn_error_t *
> -svn_wc__db_temp_op_delete(svn_wc__db_t *db,
> - const char *local_abspath,
> - apr_pool_t *scratch_pool)
> +/* Update the working node for LOCAL_ABSPATH setting presence=STATUS
> */
> +static svn_error_t * db_working_update_presence(svn_wc__db_status_t
> +status,
> + svn_wc__db_t *db,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool)
For which presence states does this function work?
I think the supported list should be in the docstring and asserted on top of the function, like all other svn_wc__db functions do.
(I reviewed the base-deleted case... I don't think it is safe for any other presence?)
> {
> - const svn_wc_entry_t *entry;
> - svn_error_t *err;
> + svn_wc__db_pdh_t *pdh;
> + const char *local_relpath;
> + wcroot_t *wcroot;
> + svn_sqlite__stmt_t *stmt;
> + svn_sqlite__db_t *sdb;
>
> - /* ### Initial implementation: Copy the entry based code here. This will be
> - more logical when we look at BASE_NODE and WORKING_NODE */
> + SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
> + SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
> + svn_sqlite__mode_readwrite,
> + scratch_pool, scratch_pool));
> + VERIFY_USABLE_PDH(pdh); wcroot = pdh->wcroot; sdb = wcroot->sdb;
>
> - err = svn_wc__get_entry(&entry, db, local_abspath, FALSE,
> svn_node_unknown,
> - FALSE, scratch_pool, scratch_pool);
> + SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> + STMT_UPDATE_WORKING_PRESENCE));
> SVN_ERR(svn_sqlite__bindf(stmt, "is",
> + wcroot->wc_id, local_relpath)); SVN_ERR(svn_sqlite__bind_token(stmt,
> + 3, presence_map, status)); SVN_ERR(svn_sqlite__step_done(stmt));
> +
> + flush_entries(pdh);
>
> - if (err && err->apr_err == SVN_ERR_NODE_UNEXPECTED_KIND)
> + /* ### Parent stub? I don't know; I'll punt for now as it passes
> + the regression tests as is and the problem will evaporate
> + when the db is centralised. */
> +
> + return SVN_NO_ERROR;
> +}
> +
> +/* Delete working and actual nodes for LOCAL_ABSPATH */ static
> +svn_error_t * db_working_actual_remove(svn_wc__db_t *db,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool) {
> + svn_wc__db_pdh_t *pdh;
> + const char *local_relpath;
> + wcroot_t *wcroot;
> + svn_sqlite__db_t *sdb;
> + svn_sqlite__stmt_t *stmt;
> +
> + SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
> + SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
> + svn_sqlite__mode_readwrite,
> + scratch_pool, scratch_pool));
> +
> + VERIFY_USABLE_PDH(pdh);
> + wcroot = pdh->wcroot;
> + sdb = wcroot->sdb;
> +
> + SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> + STMT_DELETE_WORKING_NODE)); SVN_ERR(svn_sqlite__bindf(stmt, "is",
> + wcroot->wc_id, local_relpath)); SVN_ERR(svn_sqlite__step_done(stmt));
> + SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> + STMT_DELETE_ACTUAL_NODE)); SVN_ERR(svn_sqlite__bindf(stmt, "is",
> + wcroot->wc_id, local_relpath)); SVN_ERR(svn_sqlite__step_done(stmt));
> +
> + flush_entries(pdh);
> +
> + if (!strcmp(local_relpath, ""))
> {
> - svn_error_clear(err);
> - err = NULL;
> + /* ### Delete parent stub. Remove when db is centralised. */
> + SVN_ERR(navigate_to_parent(&pdh, db, pdh,
> svn_sqlite__mode_readwrite,
> + scratch_pool));
> + local_relpath = svn_dirent_basename(local_abspath, NULL);
> + VERIFY_USABLE_PDH(pdh);
> + wcroot = pdh->wcroot;
> + sdb = wcroot->sdb;
> + SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> STMT_DELETE_WORKING_NODE));
> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> + SVN_ERR(svn_sqlite__step_done(stmt));
> + flush_entries(pdh);
> }
> - else
> - SVN_ERR(err);
>
> - if (entry->schedule == svn_wc_schedule_add)
> + return SVN_NO_ERROR;
> +}
> +
> +/* Insert a working node for LOCAL_ABSPATH with presence=STATUS. */
> +static svn_error_t * db_working_insert(svn_wc__db_status_t status,
> + svn_wc__db_t *db,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool) {
Which presences does this support?
Only base-delete?
It certainly can't handle cases where the node kind between BASE and WORKING differ and many other cases.
> + svn_wc__db_pdh_t *pdh;
> + const char *local_relpath;
> + wcroot_t *wcroot;
> + svn_sqlite__stmt_t *stmt;
> + svn_sqlite__db_t *sdb;
> +
> + SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
> + SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
> + svn_sqlite__mode_readwrite,
> + scratch_pool, scratch_pool));
> + VERIFY_USABLE_PDH(pdh); wcroot = pdh->wcroot; sdb = wcroot->sdb;
> +
> + SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> +
> + STMT_INSERT_WORKING_NODE_FROM_BASE_NODE));
> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> + SVN_ERR(svn_sqlite__step_done(stmt));
> +
> + /* ### Do update as part of the insert so that there is only one
> + query? */ SVN_ERR(db_working_update_presence(status, db,
> + local_abspath, scratch_pool));
> +
> + flush_entries(pdh);
> +
> + if (!strcmp(local_relpath, ""))
> {
> - svn_boolean_t deleted;
> + /* ### Insert parent stub. Remove when db is centralised. */
> + SVN_ERR(navigate_to_parent(&pdh, db, pdh,
> svn_sqlite__mode_readwrite,
> + scratch_pool));
> + local_relpath = svn_dirent_basename(local_abspath, NULL);
> + VERIFY_USABLE_PDH(pdh);
> + wcroot = pdh->wcroot;
> + sdb = wcroot->sdb;
> + /* ### Should the parent stub have a full row like this? */
> + SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> + STMT_INSERT_WORKING_NODE_FROM_BASE_NODE));
> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> + SVN_ERR(svn_sqlite__step_done(stmt));
> + SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> + STMT_UPDATE_WORKING_PRESENCE));
> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> + SVN_ERR(svn_sqlite__bind_token(stmt, 3, presence_map, status));
> + SVN_ERR(svn_sqlite__step_done(stmt));
> + SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> STMT_UPDATE_WORKING_KIND));
> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> + SVN_ERR(svn_sqlite__bind_text(stmt, 3, "subdir"));
> + SVN_ERR(svn_sqlite__step_done(stmt));
> + flush_entries(pdh);
> + }
> +
> + return SVN_NO_ERROR;
> +}
>
> - SVN_ERR(svn_wc__node_is_deleted(&deleted, db, local_abspath,
> scratch_pool));
> +/* Set *ROOT_OF_COPY to TRUE if LOCAL_ABSPATH is the root of a copy,
> + to FALSE otherwise. */
> +static svn_error_t*
> +is_root_of_copy(svn_boolean_t *root_of_copy,
> + svn_wc__db_t *db,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool) {
> + svn_wc__db_status_t status;
> + const char *op_root_abspath;
> + const char *original_repos_relpath, *original_repos_root;
> + const char *original_repos_uuid;
> + svn_revnum_t original_revision;
> +
> + SVN_ERR(svn_wc__db_scan_addition(&status, &op_root_abspath,
> + NULL, NULL, NULL,
> + &original_repos_relpath,
> + &original_repos_root,
> + &original_repos_uuid,
> + &original_revision,
> + db, local_abspath,
> + scratch_pool, scratch_pool));
>
> - if (!deleted)
> - {
> - /* This case is easy.. Just delete the entry */
> - SVN_ERR(svn_wc__entry_remove(db, local_abspath, scratch_pool));
> - SVN_ERR(svn_wc__db_temp_forget_directory(db, local_abspath,
> scratch_pool));
> + SVN_ERR_ASSERT(status == svn_wc__db_status_added
> + || status == svn_wc__db_status_copied);
>
> - return SVN_NO_ERROR;
> + *root_of_copy = op_root_abspath && !strcmp(local_abspath,
> + op_root_abspath);
> +
> + if (*root_of_copy && status == svn_wc__db_status_copied)
> + {
> + /* ### merge sets the wrong copyfrom when adding a tree and so
> + the root detection above is unreliable. I'm "fixing" it
> + here because I just need to detect whether this is an
> + instance of the merge bug, and that's easier than fixing
> + scan_addition or merge. */
Adds can be merged by design, so you don't just see this behavior below merges!
Calls like
svn cp ^/trunk A
svn cp ^/branches/B A/B
give you two copy roots below each other (and you can have N of them). You really need to loop/recurse upwards to find the place where the copy is added to or replaces a base node.
Each of these needs different handling on delete with WC-NG. (A/B was not really handled that well via entries especially if it was a replacement).
This specific case also touches the issue that we can't describe a local add below a copy yet. (I think you raised that issue yourself)
> + const char *parent_abspath = svn_dirent_dirname(local_abspath,
> + scratch_pool);
> + svn_wc__db_status_t parent_status;
> + const char *parent_original_repos_relpath,
> *parent_original_repos_root;
> + const char *parent_original_repos_uuid;
> + svn_revnum_t parent_original_revision;
> + svn_error_t *err;
> +
> + err = svn_wc__db_scan_addition(&parent_status,
> + NULL, NULL, NULL, NULL,
> + &parent_original_repos_relpath,
> + &parent_original_repos_root,
> + &parent_original_repos_uuid,
> + &parent_original_revision,
> + db, parent_abspath,
> + scratch_pool, scratch_pool);
> + if (err)
> + {
> + if (err->apr_err != SVN_ERR_WC_PATH_NOT_FOUND)
> + return svn_error_return(err);
> + /* It really is a root */
> + svn_error_clear(err);
> }
> + else if (parent_status == svn_wc__db_status_copied
> + && original_revision == parent_original_revision
> + && !strcmp(original_repos_uuid, parent_original_repos_uuid)
> + && !strcmp(original_repos_root, parent_original_repos_root)
> + && !strcmp(original_repos_relpath,
> + svn_dirent_join(parent_original_repos_relpath,
> + svn_dirent_basename(local_abspath,
> + scratch_pool),
> + scratch_pool)))
> + /* An instance of the merge bug */
> + *root_of_copy = FALSE;
Currently we don't record copy origins from different repositories, so the uuid and repos_root don't need specific checking (yet). But of course it doesn’t hurt to do it anyway.
(I'm not sure if you should look at the different copies below each other as separate copies.. or use this workaround)
> }
>
> + return SVN_NO_ERROR;
> +}
> +
> +/* Delete LOCAL_ABSPATH. Implements the delete transition from
> + notes/wc-ng/transitions. */
> +svn_error_t *
> +svn_wc__db_temp_op_delete(svn_wc__db_t *db,
> + const char *local_abspath,
> + apr_pool_t *scratch_pool) {
> + svn_error_t *err;
> + svn_boolean_t base_none, working_none, new_working_none;
> + svn_wc__db_status_t base_status, working_status, new_working_status;
> +
> + err = svn_wc__db_base_get_info(&base_status,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + db, local_abspath, scratch_pool,
> + scratch_pool); if (err && err->apr_err ==
> SVN_ERR_WC_PATH_NOT_FOUND)
> {
> - svn_wc_entry_t tmp_entry;
> + base_none = TRUE;
> + svn_error_clear(err);
> + }
> + else if (! err)
> + base_none = FALSE;
> + else
> + return svn_error_return(err);
This code path misses the case where a base-not-present is recorded in the parent stub but not in the directory itself. (This is the special check in the old cod using svn_wc__node_deleted). It just happens that your new code doesn't have to compensate for this.
Nice... I missed that in my tests ;-)
>
> - tmp_entry.schedule = svn_wc_schedule_delete;
> + if (!base_none && base_status == svn_wc__db_status_absent)
> + return SVN_NO_ERROR; /* ### return an error? */
>
> - /* Don't update the directory entries for locally added directories
> - and directories that are missing. (The first scenario abort()s
> - and the second gives a WC missing error) */
> - if (entry->kind != svn_node_dir ||
> - (entry->schedule != svn_wc_schedule_add && *entry->name == '\0'))
> - SVN_ERR(svn_wc__entry_modify2(db, local_abspath, entry->kind,
> - FALSE, &tmp_entry,
> - SVN_WC__ENTRY_MODIFY_SCHEDULE,
> - scratch_pool));
> + err = db_working_get_status(&working_status,
> + db, local_abspath, scratch_pool,
> + scratch_pool); if (err && err->apr_err ==
> SVN_ERR_WC_PATH_NOT_FOUND)
> + {
> + working_none = TRUE;
> + svn_error_clear(err);
> + }
> + else if (! err)
> + working_none = FALSE;
> + else
> + return svn_error_return(err);
>
> - /* And update the parent stub */
> - if (entry->kind == svn_node_dir)
> - SVN_ERR(svn_wc__entry_modify2(db, local_abspath, svn_node_dir,
> - TRUE, &tmp_entry,
> - SVN_WC__ENTRY_MODIFY_SCHEDULE,
> - scratch_pool));
> + if (base_none && working_none)
> + return SVN_NO_ERROR; /* ### return an error? */
> +
> + new_working_none = working_none;
> + new_working_status = working_status;
> + if (working_none)
> + {
> + if (base_status == svn_wc__db_status_normal
> + || base_status == svn_wc__db_status_obstructed
> + || base_status == svn_wc__db_status_incomplete
> + || base_status == svn_wc__db_status_excluded)
> + {
> + new_working_none = FALSE;
> + new_working_status = svn_wc__db_status_base_deleted;
> + }
> + }
> + else if ((working_status == svn_wc__db_status_normal
> + || working_status == svn_wc__db_status_obstructed)
> + && (base_none || base_status ==
> svn_wc__db_status_not_present))
> + {
> + svn_boolean_t root_of_copy;
> + SVN_ERR(is_root_of_copy(&root_of_copy, db, local_abspath,
> scratch_pool));
> + if (root_of_copy)
> + new_working_none = TRUE;
> + else
> + new_working_status = svn_wc__db_status_not_present;
> }
> + else if (working_status == svn_wc__db_status_normal)
> + {
> + svn_boolean_t root_of_copy;
> + SVN_ERR(is_root_of_copy(&root_of_copy, db, local_abspath,
> scratch_pool));
> + if (root_of_copy)
> + new_working_status = svn_wc__db_status_base_deleted;
> + else
> + new_working_status = svn_wc__db_status_not_present;
> + }
> + else if (working_status == svn_wc__db_status_incomplete)
> + {
> + svn_boolean_t root_of_copy;
> + SVN_ERR(is_root_of_copy(&root_of_copy, db, local_abspath,
> scratch_pool));
> + if (root_of_copy)
> + new_working_none = TRUE;
> + }
> +
> + if (new_working_none && !working_none)
> + SVN_ERR(db_working_actual_remove(db, local_abspath, scratch_pool));
> + else if (!new_working_none && working_none)
> + SVN_ERR(db_working_insert(new_working_status,
> + db, local_abspath, scratch_pool)); else
> + if (!new_working_none && !working_none
> + && new_working_status != working_status)
> + SVN_ERR(db_working_update_presence(new_working_status,
> + db, local_abspath,
> + scratch_pool));
> + /* ### else nothing to do, return an error? */
>
> return SVN_NO_ERROR;
> }
>
Bert