You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2010/05/07 20:02:05 UTC
svn commit: r942161 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c
props.c wc-queries.sql wc_db.c wc_db.h
Author: gstein
Date: Fri May 7 18:02:04 2010
New Revision: 942161
URL: http://svn.apache.org/viewvc?rev=942161&view=rev
Log:
Fix up some subtle issues around dealing with properties in the database.
In particular, begin adjusting the wc_db functions to sometimes return
NULL for properties because they are not available given the state of the
node. Also fix the fetching of the pristine properties: a WORKING_NODE
could be deleting the node/props, so the pristines ARE in BASE_NODE.
Adjust some callers to deal with NULL return values. Further work here is
necessary, as this was not an exhaustive review.
* subversion/libsvn_wc/wc_db.h:
(svn_wc__db_read_props, svn_wc__db_read_pristine_props): note that NULL
may be returned.
* subversion/libsvn_wc/wc_db.c:
(svn_wc__db_read_pristine_props): if the node is "base-deleted", then
the pristines are in BASE_NODE (and we should ignore any that might
have snuck into WORKING_NODE). expand some of the comments here.
* subversion/libsvn_wc/wc-queries.sql:
(STMT_SELECT_WORKING_PROPS): we'll need the 'presence' column, too.
* subversion/libsvn_wc/props.c:
(load_pristine_props): allow for a NULL return from wc_db and adjust the
assertions made.
(load_actual_props): allow for a NULL return from load_pristine_props,
compensating with an empty set of props. also allow for a NULL from
wc_db and adjust the assertions.
(immediate_install_props): remove the BASE_PROPS param, and just fetch
the pristines in here.
(svn_wc__get_pristine_props): add some comments to clarify
(svn_wc__internal_propset): no need to fetch the pristines, as
immediate_install_props will do it now.
(svn_wc__has_props): compensate for a NULL return. add some comments.
(svn_wc__internal_propdiff): add question about NULL return from
load_pristine_props.
* subversion/libsvn_wc/adm_ops.c:
(mark_item_copied): compensate for a NULL set of pristine props
Modified:
subversion/trunk/subversion/libsvn_wc/adm_ops.c
subversion/trunk/subversion/libsvn_wc/props.c
subversion/trunk/subversion/libsvn_wc/wc-queries.sql
subversion/trunk/subversion/libsvn_wc/wc_db.c
subversion/trunk/subversion/libsvn_wc/wc_db.h
Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=942161&r1=942160&r2=942161&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Fri May 7 18:02:04 2010
@@ -1232,6 +1232,13 @@ mark_item_copied(svn_wc__db_t *db,
SVN_WC__ENTRY_MODIFY_COPIED, scratch_pool));
/* Reinstall the pristine properties on WORKING */
+ /* ### this is all pretty crappy code anyways. not much of a problem
+ ### to pile on here.
+ ### thus... the node's original state may suggest there are no
+ ### pristine properties. in this case, we should say this (copied)
+ ### node has an empty set of properties. */
+ if (props == NULL)
+ props = apr_hash_make(scratch_pool);
SVN_ERR(svn_wc__db_temp_working_set_props(db, local_abspath, props,
scratch_pool));
Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=942161&r1=942160&r2=942161&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Fri May 7 18:02:04 2010
@@ -179,20 +179,22 @@ load_pristine_props(apr_hash_t **props,
SVN_ERR(svn_wc__db_read_pristine_props(&db_base_props, db,
local_abspath,
scratch_pool, scratch_pool));
- SVN_ERR_ASSERT(db_base_props != NULL);
- if (*props != NULL)
+ if (*props != NULL && apr_hash_count(*props) > 0)
{
apr_array_header_t *diffs;
+ SVN_ERR_ASSERT(db_base_props != NULL);
+
SVN_ERR(svn_prop_diffs(&diffs, *props, db_base_props, scratch_pool));
SVN_ERR_ASSERT(diffs->nelts == 0);
}
else
{
- /* If the propfile is missing, then we should see empty props
+ /* If the propfile is missing, then we should see no/empty props
in the database. */
- SVN_ERR_ASSERT(apr_hash_count(db_base_props) == 0);
+ SVN_ERR_ASSERT(db_base_props == NULL
+ || apr_hash_count(db_base_props) == 0);
}
}
#endif
@@ -216,8 +218,15 @@ load_actual_props(apr_hash_t **props,
For this case, just use the pristine properties. This is very
different from an empty file, which means "all props deleted". */
if (*props == NULL)
- SVN_ERR(load_pristine_props(props, db, local_abspath,
- result_pool, scratch_pool));
+ {
+ SVN_ERR(load_pristine_props(props, db, local_abspath,
+ result_pool, scratch_pool));
+
+ /* If pristines are not defined for this node, then define this
+ node to have an empty set of properties. */
+ if (*props == NULL)
+ *props = apr_hash_make(result_pool);
+ }
#ifdef TEST_DB_PROPS
{
@@ -228,10 +237,16 @@ load_actual_props(apr_hash_t **props,
SVN_ERR(svn_wc__db_read_props(&db_props, db, local_abspath,
scratch_pool, scratch_pool));
- SVN_ERR_ASSERT(db_props != NULL);
- SVN_ERR(svn_prop_diffs(&diffs, *props, db_props, scratch_pool));
- SVN_ERR_ASSERT(diffs->nelts == 0);
+ if (db_props != NULL)
+ {
+ SVN_ERR(svn_prop_diffs(&diffs, *props, db_props, scratch_pool));
+ SVN_ERR_ASSERT(diffs->nelts == 0);
+ }
+ else
+ {
+ SVN_ERR_ASSERT(apr_hash_count(*props) == 0);
+ }
}
#endif
@@ -382,13 +397,18 @@ static svn_error_t *
immediate_install_props(svn_wc__db_t *db,
const char *local_abspath,
svn_wc__db_kind_t kind,
- apr_hash_t *base_props,
apr_hash_t *working_props,
apr_pool_t *scratch_pool)
{
+ apr_hash_t *base_props;
const char *propfile_abspath;
apr_array_header_t *prop_diffs;
+ /* ### no pristines should be okay. */
+ SVN_ERR_W(load_pristine_props(&base_props, db, local_abspath,
+ scratch_pool, scratch_pool),
+ _("Failed to load pristine properties"));
+
SVN_ERR(svn_wc__prop_path(&propfile_abspath, local_abspath, kind,
svn_wc__props_working, scratch_pool));
@@ -1921,6 +1941,8 @@ svn_wc__get_pristine_props(apr_hash_t **
scratch_pool, scratch_pool));
if (status == svn_wc__db_status_added)
{
+ /* Resolve the status. copied and moved_here arrive with properties,
+ while a simple add does not. */
SVN_ERR(svn_wc__db_scan_addition(&status, NULL,
NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
@@ -1952,6 +1974,7 @@ svn_wc__get_pristine_props(apr_hash_t **
/* status: normal, moved_here, copied, deleted */
+ /* After the above checks, these pristines should always be present. */
return svn_error_return(load_pristine_props(props, db, local_abspath,
result_pool, scratch_pool));
}
@@ -2209,7 +2232,7 @@ svn_wc__internal_propset(svn_wc__db_t *d
void *notify_baton,
apr_pool_t *scratch_pool)
{
- apr_hash_t *prophash, *base_prophash;
+ apr_hash_t *prophash;
enum svn_prop_kind prop_kind = svn_property_kind(NULL, name);
svn_wc_notify_action_t notify_action;
svn_wc__db_kind_t kind;
@@ -2283,9 +2306,6 @@ svn_wc__internal_propset(svn_wc__db_t *d
/* If not, we'll set the file to read-only at commit time. */
}
- SVN_ERR_W(load_pristine_props(&base_prophash, db, local_abspath,
- scratch_pool, scratch_pool),
- _("Failed to load pristine properties"));
SVN_ERR_W(load_actual_props(&prophash, db, local_abspath,
scratch_pool, scratch_pool),
_("Failed to load current properties"));
@@ -2362,8 +2382,8 @@ svn_wc__internal_propset(svn_wc__db_t *d
/* Drop it right onto the disk. We don't need loggy since we aren't
coordinating this change with anything else. */
- SVN_ERR(immediate_install_props(db, local_abspath, kind,
- base_prophash, prophash, scratch_pool));
+ SVN_ERR(immediate_install_props(db, local_abspath, kind, prophash,
+ scratch_pool));
if (notify_func)
{
@@ -2532,9 +2552,12 @@ svn_wc__has_props(svn_boolean_t *has_pro
{
apr_hash_t *props;
+ /* ### this function is only used by status.c. should nuke it all. */
+
+ /* ### no pristines should be okay */
SVN_ERR(load_pristine_props(&props, db, local_abspath,
scratch_pool, scratch_pool));
- if (apr_hash_count(props))
+ if (props != NULL && apr_hash_count(props))
{
*has_props = TRUE;
return SVN_NO_ERROR;
@@ -2632,6 +2655,8 @@ svn_wc__internal_propdiff(apr_array_head
SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
+ /* ### if pristines are not defined, then should this raise an error,
+ ### or use an empty set? */
SVN_ERR(load_pristine_props(&baseprops, db, local_abspath,
result_pool, scratch_pool));
Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=942161&r1=942160&r2=942161&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Fri May 7 18:02:04 2010
@@ -111,8 +111,8 @@ select properties from base_node
where wc_id = ?1 and local_relpath = ?2;
-- STMT_SELECT_WORKING_PROPS
-select properties from working_node
-where wc_id = ?1 and local_relpath = ?2;
+SELECT properties, presence FROM WORKING_NODE
+WHERE wc_id = ?1 AND local_relpath = ?2;
-- STMT_SELECT_ACTUAL_PROPS
select properties from actual_node
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=942161&r1=942160&r2=942161&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Fri May 7 18:02:04 2010
@@ -4357,34 +4357,45 @@ svn_wc__db_read_pristine_props(apr_hash_
{
svn_sqlite__stmt_t *stmt;
svn_boolean_t have_row;
- svn_error_t *err;
-
- *props = NULL;
SVN_ERR(get_statement_for_path(&stmt, db, local_abspath,
STMT_SELECT_WORKING_PROPS, scratch_pool));
SVN_ERR(svn_sqlite__step(&have_row, stmt));
- /* If there is a WORKING row, then we're done.
+ /* If there is a WORKING row, then examine its status:
- For adds/copies/moves, then properties are in this row.
- For deletes, there are no properties.
+ For adds/copies/moves, then pristine properties are in this row.
- Regardless, we never look at BASE. The properties (or not) are here. */
+ For deletes, the pristines may be located here (as a result of a
+ copy/move-here), or they are located in BASE.
+ ### right now, we don't have a strong definition yet. moving to the
+ ### proposed NODE_DATA system will create more determinism around
+ ### where props are located and their relation to layered operations. */
if (have_row)
{
- err = svn_sqlite__column_properties(props, stmt, 0, result_pool,
- scratch_pool);
- SVN_ERR(svn_error_compose_create(err, svn_sqlite__reset(stmt)));
- if (*props == NULL)
- {
- /* ### is this a DB constraint violation? the column "probably"
- ### should never be null. maybe we should leave it null for
- ### delete operations, so this is okay. */
- *props = apr_hash_make(result_pool);
+ svn_wc__db_status_t presence;
+
+ /* For "base-deleted", it is obvious the pristine props are located
+ in the BASE table. Fall through to fetch them.
+
+ ### for regular deletes, the properties should be in the WORKING
+ ### row. though operation layering and the suggested NODE_DATA may
+ ### really be needed to ensure the props are always available,
+ ### and what "pristine" really means. */
+ presence = svn_sqlite__column_token(stmt, 1, presence_map);
+ if (presence != svn_wc__db_status_base_deleted)
+ {
+ svn_error_t *err;
+
+ err = svn_sqlite__column_properties(props, stmt, 0, result_pool,
+ scratch_pool);
+ SVN_ERR(svn_error_compose_create(err, svn_sqlite__reset(stmt)));
+
+ /* ### *PROPS may be NULL. is this okay? */
+ return SVN_NO_ERROR;
}
- return SVN_NO_ERROR;
}
+
SVN_ERR(svn_sqlite__reset(stmt));
/* No WORKING node, so the props must be in the BASE node. */
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=942161&r1=942160&r2=942161&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Fri May 7 18:02:04 2010
@@ -1381,9 +1381,11 @@ svn_wc__db_read_prop(const svn_string_t
/* Set *PROPS to the properties of the node LOCAL_ABSPATH in the ACTUAL
tree (looking through to the WORKING or BASE tree as required).
+ ### *PROPS will be set to NULL in the following situations:
+ ### ... tbd
+
PROPS maps "const char *" names to "const svn_string_t *" values.
If the node has no properties, set *PROPS to an empty hash.
- *PROPS will never be set to NULL.
If the node is not present, return an error.
Allocate *PROPS and its keys and values in RESULT_POOL.
*/
@@ -1398,9 +1400,11 @@ svn_wc__db_read_props(apr_hash_t **props
/* Set *PROPS to the properties of the node LOCAL_ABSPATH in the WORKING
tree (looking through to the BASE tree as required).
+ ### *PROPS will set set to NULL in the following situations:
+ ### ... tbd. see props.c:svn_wc__get_pristine_props()
+
*PROPS maps "const char *" names to "const svn_string_t *" values.
If the node has no properties, set *PROPS to an empty hash.
- *PROPS will never be set to NULL.
If the node is not present, return an error.
Allocate *PROPS and its keys and values in RESULT_POOL.
*/