You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2010/06/28 22:04:23 UTC
svn commit: r958698 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c
wc_db.c
Author: rhuijben
Date: Mon Jun 28 20:04:22 2010
New Revision: 958698
URL: http://svn.apache.org/viewvc?rev=958698&view=rev
Log:
Allow svn_wc__db_add_directory() to attach a directory to its parent, to
allow removing another use case of entries from svn_wc_add4().
* subversion/libsvn_wc/adm_ops.c
(svn_wc_add4): Move for copyfrom_url in repos_root out
of the entries handling. Directly remove incomplete entry from new
directory when we only need the directory for local additions. Use
svn_wc__db_op_add_directory() for directory additions.
* subversion/libsvn_wc/wc_db.c
(svn_wc__db_op_add_directory): Hook directory to parent directory
instead of just ignoring the parent when the directories are not
attached. Ignoring the parent is never valid as a top level wcroot
can never be added.
Modified:
subversion/trunk/subversion/libsvn_wc/adm_ops.c
subversion/trunk/subversion/libsvn_wc/wc_db.c
Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=958698&r1=958697&r2=958698&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Mon Jun 28 20:04:22 2010
@@ -1276,6 +1276,12 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
db, parent_abspath,
scratch_pool, scratch_pool));
}
+
+ if (copyfrom_url
+ && !svn_uri_is_ancestor(repos_root_url, copyfrom_url))
+ return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
+ _("The URL '%s' has a different repository "
+ "root than its parent"), copyfrom_url);
}
/* Verify that we can actually integrate the inner working copy */
@@ -1332,6 +1338,11 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
repos_root_url, repos_root_url,
repos_uuid, 0,
depth, scratch_pool));
+
+ /* ### The entries based code still needs the incomplete base record,
+ ### remove it for the direct db code. */
+ if (!copyfrom_url)
+ SVN_ERR(svn_wc__db_base_remove(db, local_abspath, scratch_pool));
}
#endif
@@ -1388,6 +1399,11 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
scratch_pool));
}
}
+ else if (!copyfrom_url)
+ {
+ SVN_ERR(svn_wc__db_op_add_directory(db, local_abspath, NULL,
+ scratch_pool));
+ }
else
{
svn_wc_entry_t tmp_entry;
@@ -1408,7 +1424,7 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
/* Init the modify flags. */
tmp_entry.schedule = svn_wc_schedule_add;
- tmp_entry.kind = kind;
+ tmp_entry.kind = svn_node_dir;
modify_flags = SVN_WC__ENTRY_MODIFY_SCHEDULE | SVN_WC__ENTRY_MODIFY_KIND;
/* If a copy ancestor was given, make sure the copyfrom URL is in the same
@@ -1416,12 +1432,6 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
entry */
if (copyfrom_url)
{
- if (repos_root_url
- && ! svn_uri_is_ancestor(repos_root_url, copyfrom_url))
- return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
- _("The URL '%s' has a different "
- "repository root than its parent"),
- copyfrom_url);
tmp_entry.copyfrom_url = copyfrom_url;
tmp_entry.copyfrom_rev = copyfrom_rev;
tmp_entry.copied = TRUE;
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=958698&r1=958697&r2=958698&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Jun 28 20:04:22 2010
@@ -3071,12 +3071,20 @@ svn_wc__db_op_add_directory(svn_wc__db_t
err = navigate_to_parent(&pdh, db, pdh, svn_sqlite__mode_readwrite,
scratch_pool);
- if (err)
+ if (err && err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
{
- /* Prolly fell off the top of the wcroot. Just call it a day. */
+ /* Not registered in the parent; register as addition */
svn_error_clear(err);
- return SVN_NO_ERROR;
+
+ SVN_ERR(svn_wc__db_pdh_parse_local_abspath(&pdh, &local_relpath, db,
+ svn_dirent_dirname(local_abspath, scratch_pool),
+ svn_sqlite__mode_readwrite, scratch_pool,
+ scratch_pool));
+
+ VERIFY_USABLE_PDH(pdh);
}
+ else
+ SVN_ERR(err);
blank_iwb(&iwb);
Re: svn commit: r958698 - in /subversion/trunk/subversion/libsvn_wc:
adm_ops.c wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
Not on my game today.
Thanks!
On Mon, Jun 28, 2010 at 16:26, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: Greg Stein [mailto:gstein@gmail.com]
>> Sent: maandag 28 juni 2010 22:22
>> To: dev@subversion.apache.org
>> Subject: Re: svn commit: r958698 - in
>> /subversion/trunk/subversion/libsvn_wc: adm_ops.c wc_db.c
>>
>> On Mon, Jun 28, 2010 at 16:04, <rh...@apache.org> wrote:
>> >...
>> > +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Mon Jun 28
>> 20:04:22 2010
>> >...
>> > @@ -1332,6 +1338,11 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
>> > repos_root_url,
>> repos_root_url,
>> > repos_uuid, 0,
>> > depth, scratch_pool));
>> > +
>> > + /* ### The entries based code still needs the incomplete base
>> record,
>> > + ### remove it for the direct db code. */
>> > + if (!copyfrom_url)
>> > + SVN_ERR(svn_wc__db_base_remove(db, local_abspath,
>> scratch_pool));
>>
>> "direct db" ?? ... did you mean single db?
>>
>> And I think the comment should explain that area just a bit more: that
>> we create an administrative area with a single row in BASE_NODE, and
>> then you wipe out that row, leaving a database with no node
>> information.
>
> If you had used a diff tool which showed more context you would have seen:
>
> /* Make sure this new directory has an admistrative subdirectory
> created inside of it.
>
> This creates a BASE_NODE for an added directory, really
> it should create a WORKING_NODE. It gets removed by the
> next modify2 call. That is why we don't have to provide a
> valid url */
> SVN_ERR(svn_wc__internal_ensure_adm(db, local_abspath,
> repos_root_url, repos_root_url,
> repos_uuid, 0,
> depth, scratch_pool));
>
>
>
> I'm not going to repeat that 5 lines below ;-)
>
>
>>
>> >...
>> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Jun 28 20:04:22
>> 2010
>> > @@ -3071,12 +3071,20 @@ svn_wc__db_op_add_directory(svn_wc__db_t
>> >
>> > err = navigate_to_parent(&pdh, db, pdh,
>> svn_sqlite__mode_readwrite,
>> > scratch_pool);
>> > - if (err)
>> > + if (err && err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
>> > {
>> > - /* Prolly fell off the top of the wcroot. Just call it a
>> day. */
>> > + /* Not registered in the parent; register as addition */
>> > svn_error_clear(err);
>> > - return SVN_NO_ERROR;
>> > +
>> > + SVN_ERR(svn_wc__db_pdh_parse_local_abspath(&pdh,
>> &local_relpath, db,
>> > + svn_dirent_dirname(local_abspath,
>> scratch_pool),
>> > + svn_sqlite__mode_readwrite,
>> scratch_pool,
>> > + scratch_pool));
>> > +
>> > + VERIFY_USABLE_PDH(pdh);
>> > }
>> > + else
>> > + SVN_ERR(err);
>>
>> I don't understand this part. navigate_to_parent() doesn't look for
>> the child in the parent, so "not registered in the parent" is an
>> incorrect statement.
>
> Navigate to parent does:
>
> /* Check that the parent has an entry for the child */
> SVN_ERR(svn_sqlite__get_statement(&stmt, (*parent_pdh)->wcroot->sdb,
> STMT_SELECT_SUBDIR));
> SVN_ERR(svn_sqlite__bindf(stmt, "is", (*parent_pdh)->wcroot->wc_id,
> svn_dirent_basename(child_pdh->local_abspath,
> NULL)));
> SVN_ERR(svn_sqlite__step(&got_row, stmt));
> SVN_ERR(svn_sqlite__reset(stmt));
>
> if (!got_row)
> return svn_error_createf(SVN_ERR_WC_NOT_WORKING_COPY, NULL,
> _("'%s' does not have a parent."),
>
> svn_dirent_local_style(child_pdh->local_abspath,
> scratch_pool));
>
> Bert
>
>
RE: svn commit: r958698 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c wc_db.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: maandag 28 juni 2010 22:22
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r958698 - in
> /subversion/trunk/subversion/libsvn_wc: adm_ops.c wc_db.c
>
> On Mon, Jun 28, 2010 at 16:04, <rh...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Mon Jun 28
> 20:04:22 2010
> >...
> > @@ -1332,6 +1338,11 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
> > repos_root_url,
> repos_root_url,
> > repos_uuid, 0,
> > depth, scratch_pool));
> > +
> > + /* ### The entries based code still needs the incomplete base
> record,
> > + ### remove it for the direct db code. */
> > + if (!copyfrom_url)
> > + SVN_ERR(svn_wc__db_base_remove(db, local_abspath,
> scratch_pool));
>
> "direct db" ?? ... did you mean single db?
>
> And I think the comment should explain that area just a bit more: that
> we create an administrative area with a single row in BASE_NODE, and
> then you wipe out that row, leaving a database with no node
> information.
If you had used a diff tool which showed more context you would have seen:
/* Make sure this new directory has an admistrative subdirectory
created inside of it.
This creates a BASE_NODE for an added directory, really
it should create a WORKING_NODE. It gets removed by the
next modify2 call. That is why we don't have to provide a
valid url */
SVN_ERR(svn_wc__internal_ensure_adm(db, local_abspath,
repos_root_url, repos_root_url,
repos_uuid, 0,
depth, scratch_pool));
I'm not going to repeat that 5 lines below ;-)
>
> >...
> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Jun 28 20:04:22
> 2010
> > @@ -3071,12 +3071,20 @@ svn_wc__db_op_add_directory(svn_wc__db_t
> >
> > err = navigate_to_parent(&pdh, db, pdh,
> svn_sqlite__mode_readwrite,
> > scratch_pool);
> > - if (err)
> > + if (err && err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
> > {
> > - /* Prolly fell off the top of the wcroot. Just call it a
> day. */
> > + /* Not registered in the parent; register as addition */
> > svn_error_clear(err);
> > - return SVN_NO_ERROR;
> > +
> > + SVN_ERR(svn_wc__db_pdh_parse_local_abspath(&pdh,
> &local_relpath, db,
> > + svn_dirent_dirname(local_abspath,
> scratch_pool),
> > + svn_sqlite__mode_readwrite,
> scratch_pool,
> > + scratch_pool));
> > +
> > + VERIFY_USABLE_PDH(pdh);
> > }
> > + else
> > + SVN_ERR(err);
>
> I don't understand this part. navigate_to_parent() doesn't look for
> the child in the parent, so "not registered in the parent" is an
> incorrect statement.
Navigate to parent does:
/* Check that the parent has an entry for the child */
SVN_ERR(svn_sqlite__get_statement(&stmt, (*parent_pdh)->wcroot->sdb,
STMT_SELECT_SUBDIR));
SVN_ERR(svn_sqlite__bindf(stmt, "is", (*parent_pdh)->wcroot->wc_id,
svn_dirent_basename(child_pdh->local_abspath,
NULL)));
SVN_ERR(svn_sqlite__step(&got_row, stmt));
SVN_ERR(svn_sqlite__reset(stmt));
if (!got_row)
return svn_error_createf(SVN_ERR_WC_NOT_WORKING_COPY, NULL,
_("'%s' does not have a parent."),
svn_dirent_local_style(child_pdh->local_abspath,
scratch_pool));
Bert
Re: svn commit: r958698 - in /subversion/trunk/subversion/libsvn_wc:
adm_ops.c wc_db.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Jun 28, 2010 at 16:04, <rh...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Mon Jun 28 20:04:22 2010
>...
> @@ -1332,6 +1338,11 @@ svn_wc_add4(svn_wc_context_t *wc_ctx,
> repos_root_url, repos_root_url,
> repos_uuid, 0,
> depth, scratch_pool));
> +
> + /* ### The entries based code still needs the incomplete base record,
> + ### remove it for the direct db code. */
> + if (!copyfrom_url)
> + SVN_ERR(svn_wc__db_base_remove(db, local_abspath, scratch_pool));
"direct db" ?? ... did you mean single db?
And I think the comment should explain that area just a bit more: that
we create an administrative area with a single row in BASE_NODE, and
then you wipe out that row, leaving a database with no node
information.
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Jun 28 20:04:22 2010
> @@ -3071,12 +3071,20 @@ svn_wc__db_op_add_directory(svn_wc__db_t
>
> err = navigate_to_parent(&pdh, db, pdh, svn_sqlite__mode_readwrite,
> scratch_pool);
> - if (err)
> + if (err && err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
> {
> - /* Prolly fell off the top of the wcroot. Just call it a day. */
> + /* Not registered in the parent; register as addition */
> svn_error_clear(err);
> - return SVN_NO_ERROR;
> +
> + SVN_ERR(svn_wc__db_pdh_parse_local_abspath(&pdh, &local_relpath, db,
> + svn_dirent_dirname(local_abspath, scratch_pool),
> + svn_sqlite__mode_readwrite, scratch_pool,
> + scratch_pool));
> +
> + VERIFY_USABLE_PDH(pdh);
> }
> + else
> + SVN_ERR(err);
I don't understand this part. navigate_to_parent() doesn't look for
the child in the parent, so "not registered in the parent" is an
incorrect statement.
The only real error is that you moved out of the working copy. I think
there is something more subtle going on here, and your comments and
code are not following that. Your explicit call to parse_local_abspath
is relatively bogus: navigate_ot_parent does that internally.
Please re-investigate this change. I don't think it should be necessary at all.
Cheers,
-g
RE: svn commit: r958698 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c wc_db.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Julian Foad [mailto:julian.foad@wandisco.com]
> Sent: dinsdag 29 juni 2010 10:01
> To: Bert Huijben
> Cc: Subversion Dev
> Subject: Re: svn commit: r958698 - in
> /subversion/trunk/subversion/libsvn_wc: adm_ops.c wc_db.c
>
> Bert Huijben wrote:
> > Log:
> > Allow svn_wc__db_add_directory() to attach a directory to its parent,
> to
> > allow removing another use case of entries from svn_wc_add4().
> >
> > * subversion/libsvn_wc/adm_ops.c
> > (svn_wc_add4): Move for copyfrom_url in repos_root out
> > of the entries handling. Directly remove incomplete entry from
> new
> > directory when we only need the directory for local additions.
> Use
> > svn_wc__db_op_add_directory() for directory additions.
> >
> > * subversion/libsvn_wc/wc_db.c
> > (svn_wc__db_op_add_directory): Hook directory to parent directory
> > instead of just ignoring the parent when the directories are not
> > attached. Ignoring the parent is never valid as a top level
> wcroot
> > can never be added.
>
> Hi Bert. That comment about svn_wc__db_op_add_directory() sounds like
> it would be useful to have in the function's doc string.
That is an internal detail on a bugfix and on how wc_db operates internally *now*, which will be invalid when we move to a single DB (because you don't have stubs then). The function simply doesn't work as advertized when you don't apply this rule.
Bert
Re: svn commit: r958698 - in
/subversion/trunk/subversion/libsvn_wc: adm_ops.c wc_db.c
Posted by Julian Foad <ju...@wandisco.com>.
Bert Huijben wrote:
> Log:
> Allow svn_wc__db_add_directory() to attach a directory to its parent, to
> allow removing another use case of entries from svn_wc_add4().
>
> * subversion/libsvn_wc/adm_ops.c
> (svn_wc_add4): Move for copyfrom_url in repos_root out
> of the entries handling. Directly remove incomplete entry from new
> directory when we only need the directory for local additions. Use
> svn_wc__db_op_add_directory() for directory additions.
>
> * subversion/libsvn_wc/wc_db.c
> (svn_wc__db_op_add_directory): Hook directory to parent directory
> instead of just ignoring the parent when the directories are not
> attached. Ignoring the parent is never valid as a top level wcroot
> can never be added.
Hi Bert. That comment about svn_wc__db_op_add_directory() sounds like
it would be useful to have in the function's doc string.
- Julian