You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Hyrum K. Wright" <hy...@mail.utexas.edu> on 2010/06/01 11:48:04 UTC
Re: svn commit: r949964 - /subversion/trunk/subversion/libsvn_wc/wc_db.c
On Tue, Jun 1, 2010 at 4:33 AM, <ph...@apache.org> wrote:
> Author: philip
> Date: Tue Jun 1 08:33:08 2010
> New Revision: 949964
>
> URL: http://svn.apache.org/viewvc?rev=949964&view=rev
> Log:
> * subversion/libsvn_wc/wc_db.c
> (temp_cross_db_copy): Bind all the parameters when copying the
> ACTUAL_NODE, use a blob for properties.
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/wc_db.c
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=949964&r1=949963&r2=949964&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue Jun 1 08:33:08 2010
> @@ -2267,14 +2267,16 @@ temp_cross_db_copy(svn_wc__db_t *db,
> scratch_pool);
> const char *tree_conflict_data = svn_sqlite__column_text(stmt, 5,
>
> scratch_pool);
> - const char *properties = svn_sqlite__column_text(stmt, 6,
> scratch_pool);
> + apr_size_t props_size;
> + const char *properties = svn_sqlite__column_blob(stmt, 6,
> &props_size,
> + scratch_pool);
>
Should we be using svn_sqlite__column_properties() here (and extending
_bindf() so that it knows about properties)? I understand we're just doing
a straight copy, so we don't necessarily need to parse and unparse the
properties, but if this is the case, perhaps a comment in the code would be
useful.
> SVN_ERR(svn_sqlite__reset(stmt));
> SVN_ERR(svn_sqlite__get_statement(&stmt, dst_pdh->wcroot->sdb,
> STMT_INSERT_ACTUAL_NODE));
> - SVN_ERR(svn_sqlite__bindf(stmt, "isss",
> + SVN_ERR(svn_sqlite__bindf(stmt, "issbsssss",
> dst_pdh->wcroot->wc_id, dst_relpath,
> svn_relpath_dirname(dst_relpath,
> scratch_pool),
> - properties,
> + properties, props_size,
> conflict_old, conflict_new,
> conflict_working,
> changelist, tree_conflict_data));
> SVN_ERR(svn_sqlite__step(&have_row, stmt));
>
>
>
-Hyrum
Re: svn commit: r949964 - /subversion/trunk/subversion/libsvn_wc/wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Jun 3, 2010 at 12:53, Greg Stein <gs...@gmail.com> wrote:
> On Tue, Jun 1, 2010 at 08:40, Philip Martin <ph...@wandisco.com> wrote:
>> "Hyrum K. Wright" <hy...@mail.utexas.edu> writes:
>>
>>>> scratch_pool);
>>>> - const char *properties = svn_sqlite__column_text(stmt, 6,
>>>> scratch_pool);
>>>> + apr_size_t props_size;
>>>> + const char *properties = svn_sqlite__column_blob(stmt, 6,
>>>> &props_size,
>>>> + scratch_pool);
>>>>
>>>
>>> Should we be using svn_sqlite__column_properties() here (and extending
>>> _bindf() so that it knows about properties)? I understand we're just doing
>>> a straight copy, so we don't necessarily need to parse and unparse the
>>> properties, but if this is the case, perhaps a comment in the code would be
>>> useful.
>>
>
> It is still nice to have a comment that the properties are being
> passed as an unserialized blob. That code will live for another couple
> months (probably), and who knows how many times it will be read (or
> copied or referred to as an example) in the meantime.
(euh... that was in response to Philip's "Perhaps..." comment)
Re: svn commit: r949964 - /subversion/trunk/subversion/libsvn_wc/wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Jun 1, 2010 at 08:40, Philip Martin <ph...@wandisco.com> wrote:
> "Hyrum K. Wright" <hy...@mail.utexas.edu> writes:
>
>>> scratch_pool);
>>> - const char *properties = svn_sqlite__column_text(stmt, 6,
>>> scratch_pool);
>>> + apr_size_t props_size;
>>> + const char *properties = svn_sqlite__column_blob(stmt, 6,
>>> &props_size,
>>> + scratch_pool);
>>>
>>
>> Should we be using svn_sqlite__column_properties() here (and extending
>> _bindf() so that it knows about properties)? I understand we're just doing
>> a straight copy, so we don't necessarily need to parse and unparse the
>> properties, but if this is the case, perhaps a comment in the code would be
>> useful.
>
It is still nice to have a comment that the properties are being
passed as an unserialized blob. That code will live for another couple
months (probably), and who knows how many times it will be read (or
copied or referred to as an example) in the meantime.
Cheers,
-g
Re: svn commit: r949964 - /subversion/trunk/subversion/libsvn_wc/wc_db.c
Posted by Philip Martin <ph...@wandisco.com>.
"Hyrum K. Wright" <hy...@mail.utexas.edu> writes:
>> scratch_pool);
>> - const char *properties = svn_sqlite__column_text(stmt, 6,
>> scratch_pool);
>> + apr_size_t props_size;
>> + const char *properties = svn_sqlite__column_blob(stmt, 6,
>> &props_size,
>> + scratch_pool);
>>
>
> Should we be using svn_sqlite__column_properties() here (and extending
> _bindf() so that it knows about properties)? I understand we're just doing
> a straight copy, so we don't necessarily need to parse and unparse the
> properties, but if this is the case, perhaps a comment in the code would be
> useful.
Perhaps. It's all temporary and will disappear when we centralise.
--
Philip
Re: svn commit: r949964 - /subversion/trunk/subversion/libsvn_wc/wc_db.c
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@vmoo.com> writes:
> STMT_INSERT_ACTUAL_NODE does an insert or replace, which might be invalid in
> this case.
Do you mean because copy should not replace an ACTUAL node? I suppose
that's true.
One of the things I am still struggling with is what checking should
happen and where. For example, it's not valid to copy a source node
with BASE.presence=not-present as the root of a copy, but it is valid
as a child of a root. Should svn_wc__db_op_copy enforce that? Should
svn_wc_copy3 check instead, or should it rely on svn_wc__db_op_copy to
check, or should both check?
It's more effecient to do the check in svn_wc_copy3 as that is where
the recursion happens. If the wc is locked does that make it valid to
check in svn_wc_copy3? Where should we check for a lock is another
thing I am struggling with.
>
> On Tue, Jun 1, 2010 at 4:33 AM, <ph...@apache.org> wrote:
>
> Author: philip
> Date: Tue Jun 1 08:33:08 2010
> New Revision: 949964
>
> URL: http://svn.apache.org/viewvc?rev=949964
> <http://svn.apache.org/viewvc?rev=949964&view=rev> &view=rev
> Log:
> * subversion/libsvn_wc/wc_db.c
> (temp_cross_db_copy): Bind all the parameters when copying the
> ACTUAL_NODE, use a blob for properties.
--
Philip
RE: svn commit: r949964 - /subversion/trunk/subversion/libsvn_wc/wc_db.c
Posted by Bert Huijben <be...@vmoo.com>.
STMT_INSERT_ACTUAL_NODE does an insert or replace, which might be invalid in
this case. (It's documentation says that it is only used from entries.c). I
think we should create a similar statement (maybe with the missing rows)
that is always safe to use
Bert
From: hyrum@hyrumwright.org [mailto:hyrum@hyrumwright.org] On Behalf Of
Hyrum K. Wright
Sent: dinsdag 1 juni 2010 13:48
To: dev@subversion.apache.org
Cc: commits@subversion.apache.org
Subject: Re: svn commit: r949964 -
/subversion/trunk/subversion/libsvn_wc/wc_db.c
On Tue, Jun 1, 2010 at 4:33 AM, <ph...@apache.org> wrote:
Author: philip
Date: Tue Jun 1 08:33:08 2010
New Revision: 949964
URL: http://svn.apache.org/viewvc?rev=949964
<http://svn.apache.org/viewvc?rev=949964&view=rev> &view=rev
Log:
* subversion/libsvn_wc/wc_db.c
(temp_cross_db_copy): Bind all the parameters when copying the
ACTUAL_NODE, use a blob for properties.
Modified:
subversion/trunk/subversion/libsvn_wc/wc_db.c
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?r
ev=949964
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?
rev=949964&r1=949963&r2=949964&view=diff> &r1=949963&r2=949964&view=diff
============================================================================
==
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue Jun 1 08:33:08 2010
@@ -2267,14 +2267,16 @@ temp_cross_db_copy(svn_wc__db_t *db,
scratch_pool);
const char *tree_conflict_data = svn_sqlite__column_text(stmt, 5,
scratch_pool);
- const char *properties = svn_sqlite__column_text(stmt, 6,
scratch_pool);
+ apr_size_t props_size;
+ const char *properties = svn_sqlite__column_blob(stmt, 6,
&props_size,
+ scratch_pool);
Should we be using svn_sqlite__column_properties() here (and extending
_bindf() so that it knows about properties)? I understand we're just doing
a straight copy, so we don't necessarily need to parse and unparse the
properties, but if this is the case, perhaps a comment in the code would be
useful.
SVN_ERR(svn_sqlite__reset(stmt));
SVN_ERR(svn_sqlite__get_statement(&stmt, dst_pdh->wcroot->sdb,
STMT_INSERT_ACTUAL_NODE));
- SVN_ERR(svn_sqlite__bindf(stmt, "isss",
+ SVN_ERR(svn_sqlite__bindf(stmt, "issbsssss",
dst_pdh->wcroot->wc_id, dst_relpath,
svn_relpath_dirname(dst_relpath,
scratch_pool),
- properties,
+ properties, props_size,
conflict_old, conflict_new,
conflict_working,
changelist, tree_conflict_data));
SVN_ERR(svn_sqlite__step(&have_row, stmt));
-Hyrum
RE: svn commit: r949964 - /subversion/trunk/subversion/libsvn_wc/wc_db.c
Posted by Bert Huijben <be...@vmoo.com>.
STMT_INSERT_ACTUAL_NODE does an insert or replace, which might be invalid in
this case. (It's documentation says that it is only used from entries.c). I
think we should create a similar statement (maybe with the missing rows)
that is always safe to use
Bert
From: hyrum@hyrumwright.org [mailto:hyrum@hyrumwright.org] On Behalf Of
Hyrum K. Wright
Sent: dinsdag 1 juni 2010 13:48
To: dev@subversion.apache.org
Cc: commits@subversion.apache.org
Subject: Re: svn commit: r949964 -
/subversion/trunk/subversion/libsvn_wc/wc_db.c
On Tue, Jun 1, 2010 at 4:33 AM, <ph...@apache.org> wrote:
Author: philip
Date: Tue Jun 1 08:33:08 2010
New Revision: 949964
URL: http://svn.apache.org/viewvc?rev=949964
<http://svn.apache.org/viewvc?rev=949964&view=rev> &view=rev
Log:
* subversion/libsvn_wc/wc_db.c
(temp_cross_db_copy): Bind all the parameters when copying the
ACTUAL_NODE, use a blob for properties.
Modified:
subversion/trunk/subversion/libsvn_wc/wc_db.c
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?r
ev=949964
<http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?
rev=949964&r1=949963&r2=949964&view=diff> &r1=949963&r2=949964&view=diff
============================================================================
==
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue Jun 1 08:33:08 2010
@@ -2267,14 +2267,16 @@ temp_cross_db_copy(svn_wc__db_t *db,
scratch_pool);
const char *tree_conflict_data = svn_sqlite__column_text(stmt, 5,
scratch_pool);
- const char *properties = svn_sqlite__column_text(stmt, 6,
scratch_pool);
+ apr_size_t props_size;
+ const char *properties = svn_sqlite__column_blob(stmt, 6,
&props_size,
+ scratch_pool);
Should we be using svn_sqlite__column_properties() here (and extending
_bindf() so that it knows about properties)? I understand we're just doing
a straight copy, so we don't necessarily need to parse and unparse the
properties, but if this is the case, perhaps a comment in the code would be
useful.
SVN_ERR(svn_sqlite__reset(stmt));
SVN_ERR(svn_sqlite__get_statement(&stmt, dst_pdh->wcroot->sdb,
STMT_INSERT_ACTUAL_NODE));
- SVN_ERR(svn_sqlite__bindf(stmt, "isss",
+ SVN_ERR(svn_sqlite__bindf(stmt, "issbsssss",
dst_pdh->wcroot->wc_id, dst_relpath,
svn_relpath_dirname(dst_relpath,
scratch_pool),
- properties,
+ properties, props_size,
conflict_old, conflict_new,
conflict_working,
changelist, tree_conflict_data));
SVN_ERR(svn_sqlite__step(&have_row, stmt));
-Hyrum
Re: svn commit: r949964 - /subversion/trunk/subversion/libsvn_wc/wc_db.c
Posted by Philip Martin <ph...@wandisco.com>.
"Hyrum K. Wright" <hy...@mail.utexas.edu> writes:
>> scratch_pool);
>> - const char *properties = svn_sqlite__column_text(stmt, 6,
>> scratch_pool);
>> + apr_size_t props_size;
>> + const char *properties = svn_sqlite__column_blob(stmt, 6,
>> &props_size,
>> + scratch_pool);
>>
>
> Should we be using svn_sqlite__column_properties() here (and extending
> _bindf() so that it knows about properties)? I understand we're just doing
> a straight copy, so we don't necessarily need to parse and unparse the
> properties, but if this is the case, perhaps a comment in the code would be
> useful.
Perhaps. It's all temporary and will disappear when we centralise.
--
Philip