You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2010/03/03 00:09:54 UTC
svn commit: r918246 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Author: cmpilato
Date: Tue Mar 2 23:09:53 2010
New Revision: 918246
URL: http://svn.apache.org/viewvc?rev=918246&view=rev
Log:
* subversion/libsvn_wc/adm_ops.c
(mark_tree_deleted): Replace usage of svn_wc_entry_t-slinging APIs
with WC-NG hotness.
Modified:
subversion/trunk/subversion/libsvn_wc/adm_ops.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=918246&r1=918245&r2=918246&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar 2 23:09:53 2010
@@ -806,7 +806,7 @@
{
apr_pool_t *iterpool = svn_pool_create(pool);
const apr_array_header_t *children;
- const svn_wc_entry_t *entry;
+ svn_wc__db_status_t status;
int i;
/* Read the entries file for this directory. */
@@ -854,15 +854,18 @@
}
/* Handle "this dir" for states that need it done post-recursion. */
- SVN_ERR(svn_wc__get_entry(&entry, db, dir_abspath, FALSE,
- svn_node_dir, FALSE, iterpool, iterpool));
-
+ SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL,
+ db, dir_abspath, iterpool, iterpool));
/* Uncommitted directories (schedule add) that are to be scheduled for
deletion are a special case, they don't need to be changed as they
will be removed from their parent's entry list.
The files and directories are left on the disk in this special
case, so KEEP_LOCAL doesn't need to be set either. */
- if (entry->schedule != svn_wc_schedule_add)
+ if (!(status == svn_wc__db_status_added ||
+ status == svn_wc__db_status_obstructed_add))
{
SVN_ERR(svn_wc__db_temp_op_delete(db, dir_abspath, iterpool));
Re: svn commit: r918246 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
ACK
(That's the best I can offer at the moment. Digging self out of pile of
TODO items.)
Greg Stein wrote:
> On Tue, Mar 2, 2010 at 18:09, <cm...@apache.org> wrote:
>> ...
>> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar 2 23:09:53 2010
>> ...
>> @@ -854,15 +854,18 @@
>> }
>>
>> /* Handle "this dir" for states that need it done post-recursion. */
>> - SVN_ERR(svn_wc__get_entry(&entry, db, dir_abspath, FALSE,
>> - svn_node_dir, FALSE, iterpool, iterpool));
>> -
>> + SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
>> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
>> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
>> + NULL, NULL,
>> + db, dir_abspath, iterpool, iterpool));
>> /* Uncommitted directories (schedule add) that are to be scheduled for
>> deletion are a special case, they don't need to be changed as they
>> will be removed from their parent's entry list.
>> The files and directories are left on the disk in this special
>> case, so KEEP_LOCAL doesn't need to be set either. */
>> - if (entry->schedule != svn_wc_schedule_add)
>> + if (!(status == svn_wc__db_status_added ||
>> + status == svn_wc__db_status_obstructed_add))
>> {
>> SVN_ERR(svn_wc__db_temp_op_delete(db, dir_abspath, iterpool));
>
> In the old way of doing thigns, if the schedule was
> svn_wc_schedule_replace, then wc_db is going to return
> svn_wc__db_status_added for that condition. There are other
> considerations for determining "was this a schedule_replace of a plain
> schedule_add?"
>
> I've gotta run to a lunch. But if you look at
> questions.c::internal_is_replaced(), then you'll see that determining
> schedule_replace is a difficult problem. And the original
> schedule!=add condition *may* be looking for schedule_replace.
>
> But that is maybe the trick here, and why your testing did not find
> the code. Maybe it is only possible to see
> schedule_(normal|add|delete), and never a replace? That may narrow the
> amount of querying needed against wc_db. I can't take a look right
> now, but the comments suggest there may be very restricted conditions
> here.
>
> Cheers,
> -g
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r918246 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Mar 8, 2010 at 14:42, C. Michael Pilato <cm...@collab.net> wrote:
> Greg Stein wrote:
>...
>> In the old way of doing thigns, if the schedule was
>> svn_wc_schedule_replace, then wc_db is going to return
>> svn_wc__db_status_added for that condition. There are other
>> considerations for determining "was this a schedule_replace of a plain
>> schedule_add?"
>>
>> I've gotta run to a lunch. But if you look at
>> questions.c::internal_is_replaced(), then you'll see that determining
>> schedule_replace is a difficult problem. And the original
>> schedule!=add condition *may* be looking for schedule_replace.
>>
>> But that is maybe the trick here, and why your testing did not find
>> the code. Maybe it is only possible to see
>> schedule_(normal|add|delete), and never a replace? That may narrow the
>> amount of querying needed against wc_db. I can't take a look right
>> now, but the comments suggest there may be very restricted conditions
>> here.
>
> My attempts to get a mental handle on this situation have led me to more
> questions. (Big surprise, right?)
>
> In the 1.6.x code, mark_tree() (which has become mark_tree_deleted()) did a
> similar iteration over children -- skipping the "this dir" entry -- before
> acting on the directory itself. To what degree does the "this dir" paradigm
> continue to invade WC-NG? Obviously once we have a single DB per working
> copy, there will be no more need to distinguish between "this dir" in
> some/dir/.svn/entries and "dir" in some/.svn/entries. But until then?
For the most part, we don't worry much about "this dir". The wc_db
conceptually has just one block of metadata associated with
"/some/path/to/dir". Implementation-wise, however, we *do* still have
two blocks of metadata. Thus, we have some internal goings-on to deal
with that, and a few temporary APIs like db_temp_is_dir_deleted().
The differences *do* come up when you are messing around with access
batons, since those define the reference point. But as we eliminate
the batons, we also remove the need to distinguish *which* reference
point you're using.
> I *think* mark_tree_deleted() is effectively doing the same thing -- it
> iterates over children, but while it doesn't explicitly have code to skip a
> "this dir" entry, my reading of db_read_children() leads me to believe that
> it never returns that kind of fake entry anyway. In other words,
> db_read_children() appears to be ready for the single-db lifestyle. Is that
> accurate?
db_read_children() only returns children (per the name!), and never "."
Cheers,
-g
Re: svn commit: r918246 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by "C. Michael Pilato" <cm...@collab.net>.
Greg Stein wrote:
> On Tue, Mar 2, 2010 at 18:09, <cm...@apache.org> wrote:
>> ...
>> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar 2 23:09:53 2010
>> ...
>> @@ -854,15 +854,18 @@
>> }
>>
>> /* Handle "this dir" for states that need it done post-recursion. */
>> - SVN_ERR(svn_wc__get_entry(&entry, db, dir_abspath, FALSE,
>> - svn_node_dir, FALSE, iterpool, iterpool));
>> -
>> + SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
>> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
>> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
>> + NULL, NULL,
>> + db, dir_abspath, iterpool, iterpool));
>> /* Uncommitted directories (schedule add) that are to be scheduled for
>> deletion are a special case, they don't need to be changed as they
>> will be removed from their parent's entry list.
>> The files and directories are left on the disk in this special
>> case, so KEEP_LOCAL doesn't need to be set either. */
>> - if (entry->schedule != svn_wc_schedule_add)
>> + if (!(status == svn_wc__db_status_added ||
>> + status == svn_wc__db_status_obstructed_add))
>> {
>> SVN_ERR(svn_wc__db_temp_op_delete(db, dir_abspath, iterpool));
>
> In the old way of doing thigns, if the schedule was
> svn_wc_schedule_replace, then wc_db is going to return
> svn_wc__db_status_added for that condition. There are other
> considerations for determining "was this a schedule_replace of a plain
> schedule_add?"
>
> I've gotta run to a lunch. But if you look at
> questions.c::internal_is_replaced(), then you'll see that determining
> schedule_replace is a difficult problem. And the original
> schedule!=add condition *may* be looking for schedule_replace.
>
> But that is maybe the trick here, and why your testing did not find
> the code. Maybe it is only possible to see
> schedule_(normal|add|delete), and never a replace? That may narrow the
> amount of querying needed against wc_db. I can't take a look right
> now, but the comments suggest there may be very restricted conditions
> here.
My attempts to get a mental handle on this situation have led me to more
questions. (Big surprise, right?)
In the 1.6.x code, mark_tree() (which has become mark_tree_deleted()) did a
similar iteration over children -- skipping the "this dir" entry -- before
acting on the directory itself. To what degree does the "this dir" paradigm
continue to invade WC-NG? Obviously once we have a single DB per working
copy, there will be no more need to distinguish between "this dir" in
some/dir/.svn/entries and "dir" in some/.svn/entries. But until then?
I *think* mark_tree_deleted() is effectively doing the same thing -- it
iterates over children, but while it doesn't explicitly have code to skip a
"this dir" entry, my reading of db_read_children() leads me to believe that
it never returns that kind of fake entry anyway. In other words,
db_read_children() appears to be ready for the single-db lifestyle. Is that
accurate?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r918246 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 2, 2010 at 18:09, <cm...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar 2 23:09:53 2010
>...
> @@ -854,15 +854,18 @@
> }
>
> /* Handle "this dir" for states that need it done post-recursion. */
> - SVN_ERR(svn_wc__get_entry(&entry, db, dir_abspath, FALSE,
> - svn_node_dir, FALSE, iterpool, iterpool));
> -
> + SVN_ERR(svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL,
> + db, dir_abspath, iterpool, iterpool));
> /* Uncommitted directories (schedule add) that are to be scheduled for
> deletion are a special case, they don't need to be changed as they
> will be removed from their parent's entry list.
> The files and directories are left on the disk in this special
> case, so KEEP_LOCAL doesn't need to be set either. */
> - if (entry->schedule != svn_wc_schedule_add)
> + if (!(status == svn_wc__db_status_added ||
> + status == svn_wc__db_status_obstructed_add))
> {
> SVN_ERR(svn_wc__db_temp_op_delete(db, dir_abspath, iterpool));
In the old way of doing thigns, if the schedule was
svn_wc_schedule_replace, then wc_db is going to return
svn_wc__db_status_added for that condition. There are other
considerations for determining "was this a schedule_replace of a plain
schedule_add?"
I've gotta run to a lunch. But if you look at
questions.c::internal_is_replaced(), then you'll see that determining
schedule_replace is a difficult problem. And the original
schedule!=add condition *may* be looking for schedule_replace.
But that is maybe the trick here, and why your testing did not find
the code. Maybe it is only possible to see
schedule_(normal|add|delete), and never a replace? That may narrow the
amount of querying needed against wc_db. I can't take a look right
now, but the comments suggest there may be very restricted conditions
here.
Cheers,
-g