You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/03/02 23:21:16 UTC
Re: svn commit: r918246 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c
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
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