You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/07/09 11:35:12 UTC
svn commit: r1501163 - in /subversion/trunk/subversion:
include/private/svn_wc_private.h libsvn_client/checkout.c
libsvn_wc/adm_files.c
Author: stsp
Date: Tue Jul 9 09:35:12 2013
New Revision: 1501163
URL: http://svn.apache.org/r1501163
Log:
Allow 'svn checkout' to work within a working copy that is locked.
Fixes a regression from 1.7.
Reported by: Frank Loeffler <knarf_at_cct lsu edu>
See http://svn.haxx.se/users/archive-2013-07/0066.shtml
* subversion/include/private/svn_wc_private.h
(svn_wc__init_adm): Declare.
* subversion/libsvn_client/checkout.c
(initialize_area): Use svn_wc__init_adm() instead of svn_wc_ensure_adm4().
The latter scans upwards for an existing admin area to check for existing
working copies, which we don't need to do when creating a new WC.
* subversion/libsvn_wc/adm_files.c
(svn_wc__init_adm): New function, a thin wrapper around init_adm().
This creates a new admin area at a specified local abspath, without
first scanning upwards for an existing admin area. We could also have
created svn_wc_ensure_adm5() with a new 'is_checkout' argument, but
we're trying to reduce the public set of libsvn_wc API functions.
Modified:
subversion/trunk/subversion/include/private/svn_wc_private.h
subversion/trunk/subversion/libsvn_client/checkout.c
subversion/trunk/subversion/libsvn_wc/adm_files.c
Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_wc_private.h?rev=1501163&r1=1501162&r2=1501163&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_wc_private.h Tue Jul 9 09:35:12 2013
@@ -1863,6 +1863,36 @@ svn_wc__acquire_write_lock_for_resolve(c
const char *local_abspath,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
+
+
+/** Create a new administrative area for @a local_abspath, so
+ * that @a local_abspath is a working copy subdir based on @a url at @a
+ * revision, with depth @a depth, and with repository UUID @a repos_uuid
+ * and repository root URL @a repos_root_url.
+ *
+ * @a depth must be a definite depth, it cannot be #svn_depth_unknown.
+ * @a repos_uuid and @a repos_root_url MUST NOT be @c NULL, and
+ * @a repos_root_url must be a prefix of @a url.
+ *
+ * If the administrative area already exists, raise an error.
+ *
+ * Do not ensure existence of @a local_abspath itself; if @a local_abspath
+ * does not exist, return error.
+ *
+ * Use @a scratch_pool for temporary allocations.
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_wc__init_adm(svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ const char *url,
+ const char *repos_root_url,
+ const char *repos_uuid,
+ svn_revnum_t revision,
+ svn_depth_t depth,
+ apr_pool_t *scratch_pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Modified: subversion/trunk/subversion/libsvn_client/checkout.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/checkout.c?rev=1501163&r1=1501162&r2=1501163&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/checkout.c (original)
+++ subversion/trunk/subversion/libsvn_client/checkout.c Tue Jul 9 09:35:12 2013
@@ -58,9 +58,9 @@ initialize_area(const char *local_abspat
depth = svn_depth_infinity;
/* Make the unversioned directory into a versioned one. */
- SVN_ERR(svn_wc_ensure_adm4(ctx->wc_ctx, local_abspath, pathrev->url,
- pathrev->repos_root_url, pathrev->repos_uuid,
- pathrev->rev, depth, pool));
+ SVN_ERR(svn_wc__init_adm(ctx->wc_ctx, local_abspath, pathrev->url,
+ pathrev->repos_root_url, pathrev->repos_uuid,
+ pathrev->rev, depth, pool));
return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/libsvn_wc/adm_files.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_files.c?rev=1501163&r1=1501162&r2=1501163&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_files.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_files.c Tue Jul 9 09:35:12 2013
@@ -517,6 +517,32 @@ svn_wc_ensure_adm4(svn_wc_context_t *wc_
}
svn_error_t *
+svn_wc__init_adm(svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ const char *url,
+ const char *repos_root_url,
+ const char *repos_uuid,
+ svn_revnum_t revision,
+ svn_depth_t depth,
+ apr_pool_t *scratch_pool)
+
+{
+ const char *repos_relpath = svn_uri_skip_ancestor(repos_root_url, url,
+ scratch_pool);
+
+ SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
+ SVN_ERR_ASSERT(url != NULL);
+ SVN_ERR_ASSERT(repos_root_url != NULL);
+ SVN_ERR_ASSERT(repos_uuid != NULL);
+ SVN_ERR_ASSERT(repos_relpath != NULL);
+ SVN_ERR_ASSERT(depth != svn_depth_unknown);
+
+ return svn_error_trace(init_adm(wc_ctx->db, local_abspath,
+ repos_relpath, repos_root_url, repos_uuid,
+ revision, depth, scratch_pool));
+}
+
+svn_error_t *
svn_wc__adm_destroy(svn_wc__db_t *db,
const char *dir_abspath,
svn_cancel_func_t cancel_func,
Re: svn commit: r1501163 - in /subversion/trunk/subversion:
include/private/svn_wc_private.h libsvn_client/checkout.c
libsvn_wc/adm_files.c
Posted by Stefan Sperling <st...@apache.org>.
On Tue, Jul 09, 2013 at 01:48:22PM +0200, Bert Huijben wrote:
> The proper fix would be:
>
> Running 'svn cleanup'
> Or fixing the reason why he needs to run 'svn cleanup.
>
> Creating potentially obstructing working copies with less checking is never
> a solution. Certainly not if we want to start storing multiple working
> copies in a single wc.db file.
>
> The error needs cleanup is an error that shouldn't be ignored: a user
> encountering this error should fix his broken scenario, or we should fix the
> reason why he needs to run cleanup.
See my other reply I just sent. The error requesting a cleanup is bogus.
RE: svn commit: r1501163 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/checkout.c libsvn_wc/adm_files.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@apache.org]
> Sent: dinsdag 9 juli 2013 12:44
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1501163 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h libsvn_client/checkout.c
> libsvn_wc/adm_files.c
>
> On Tue, Jul 09, 2013 at 12:25:33PM +0200, Bert Huijben wrote:
> > I haven't tested this, but this currently appears to remove the safety
net
> around:
> > $ rm trunk
> > $ svn co URL trunk
> > (which would produce an error and now two working copies)
>
> Why shouldn't it produce two working copies?
> The inner working copy is seen as an obstruction in the outer WC.
> I don't see why this would be a problem. It's a silly use case, but
> we don't need to strictly prevent it.
>
> > Or:
> > $ svn rm trunk
> > $ svn co URL trunk
> > (which will now produce two working copies, with the first partially
> obstructed)
>
> Same point as above.
>
> >
> > Or even:
> > $ svn up --set-depth excluded trunk
> > $ svn co URL trunk
>
> Same point as above.
>
> > In all these cases 1.6 would have behaved one way, and with single-db we
> behave in a different way as we don't just attach subdirectories in the
parent
> wc.db.
> >
> > This patch might fix a few use cases, but I don't think it solves a real
> problem... And it might create a whole heap of new propblems
> >
>
> Can you please suggest a better way of fixing the original issue then?
The proper fix would be:
Running 'svn cleanup'
Or fixing the reason why he needs to run 'svn cleanup.
Creating potentially obstructing working copies with less checking is never
a solution. Certainly not if we want to start storing multiple working
copies in a single wc.db file.
The error needs cleanup is an error that shouldn't be ignored: a user
encountering this error should fix his broken scenario, or we should fix the
reason why he needs to run cleanup.
Bert
Re: svn commit: r1501163 - in /subversion/trunk/subversion:
include/private/svn_wc_private.h libsvn_client/checkout.c
libsvn_wc/adm_files.c
Posted by Stefan Sperling <st...@apache.org>.
On Tue, Jul 09, 2013 at 12:25:33PM +0200, Bert Huijben wrote:
> I haven't tested this, but this currently appears to remove the safety net around:
> $ rm trunk
> $ svn co URL trunk
> (which would produce an error and now two working copies)
Why shouldn't it produce two working copies?
The inner working copy is seen as an obstruction in the outer WC.
I don't see why this would be a problem. It's a silly use case, but
we don't need to strictly prevent it.
> Or:
> $ svn rm trunk
> $ svn co URL trunk
> (which will now produce two working copies, with the first partially obstructed)
Same point as above.
>
> Or even:
> $ svn up --set-depth excluded trunk
> $ svn co URL trunk
Same point as above.
> In all these cases 1.6 would have behaved one way, and with single-db we behave in a different way as we don't just attach subdirectories in the parent wc.db.
>
> This patch might fix a few use cases, but I don't think it solves a real problem... And it might create a whole heap of new propblems
>
Can you please suggest a better way of fixing the original issue then?
RE: svn commit: r1501163 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/checkout.c libsvn_wc/adm_files.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@apache.org]
> Sent: dinsdag 9 juli 2013 14:23
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1501163 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h libsvn_client/checkout.c
> libsvn_wc/adm_files.c
>
> On Tue, Jul 09, 2013 at 12:39:30PM +0200, Bert Huijben wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bert Huijben [mailto:bert@qqmail.nl]
> > > Sent: dinsdag 9 juli 2013 12:26
> > > To: dev@subversion.apache.org; stsp@apache.org
> > > Subject: RE: svn commit: r1501163 - in /subversion/trunk/subversion:
> > > include/private/svn_wc_private.h libsvn_client/checkout.c
> > > libsvn_wc/adm_files.c
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: stsp@apache.org [mailto:stsp@apache.org]
> > > > Sent: dinsdag 9 juli 2013 11:35
> > > > To: commits@subversion.apache.org
> > > > Subject: svn commit: r1501163 - in /subversion/trunk/subversion:
> > > > include/private/svn_wc_private.h libsvn_client/checkout.c
> > > > libsvn_wc/adm_files.c
> > > >
> > > > Author: stsp
> > > > Date: Tue Jul 9 09:35:12 2013
> > > > New Revision: 1501163
> > > >
> > > > URL: http://svn.apache.org/r1501163
> > > > Log:
> > > > Allow 'svn checkout' to work within a working copy that is locked.
> > > > Fixes a regression from 1.7.
> > > >
> > > > Reported by: Frank Loeffler <knarf_at_cct lsu edu>
> > > > See http://svn.haxx.se/users/archive-2013-07/0066.shtml
> > > >
> > > > * subversion/include/private/svn_wc_private.h
> > > > (svn_wc__init_adm): Declare.
> > > >
> > > > * subversion/libsvn_client/checkout.c
> > > > (initialize_area): Use svn_wc__init_adm() instead of
> > > > svn_wc_ensure_adm4().
> > > > The latter scans upwards for an existing admin area to check for
> existing
> > > > working copies, which we don't need to do when creating a new WC.
> > > >
> > > > * subversion/libsvn_wc/adm_files.c
> > > > (svn_wc__init_adm): New function, a thin wrapper around
init_adm().
> > > > This creates a new admin area at a specified local abspath,
without
> > > > first scanning upwards for an existing admin area. We could also
have
> > > > created svn_wc_ensure_adm5() with a new 'is_checkout' argument,
> but
> > > > we're trying to reduce the public set of libsvn_wc API functions.
> > >
> > > I haven't tested this, but this currently appears to remove the safety
net
> > > around:
> > > $ rm trunk
> > > $ svn co URL trunk
> > > (which would produce an error and now two working copies)
> > >
> > > Or:
> > > $ svn rm trunk
> > > $ svn co URL trunk
> > > (which will now produce two working copies, with the first partially
> > > obstructed)
> > >
> > > Or even:
> > > $ svn up --set-depth excluded trunk
> > > $ svn co URL trunk
> > >
> > > In all these cases 1.6 would have behaved one way, and with single-db
> we
> > > behave in a different way as we don't just attach subdirectories in
the
> parent
> > > wc.db.
> > >
> > > This patch might fix a few use cases, but I don't think it solves a
real
> > > problem... And it might create a whole heap of new proplems
> >
> > And detecting any of these in a user friendly way would require reading
the
> parent working copy...
> >
> > ... but the only reason you added this patch (as far as I can tell) was
> because reading the parent working copy wasn't possible?
> >
> >
> > I think the recommended approach is making sure the parent working copy
> is readable, instead of trying to do things 100% without the parent
working
> copy.
> >
> > Doing things without the parent working copy just gives you all kinds of
> obstruction scenarios where a simple local_abspath doesn't work as that
will
> just describe a path that is now part of two working copies.
> >
> > I don't think want to go back to the access batons/wc.db per directory
> where we could describe these paths that are part of two working copy
> scenarios.
> >
> > Bert
>
> Bert, I think you're missing the point.
>
> All the "scary" obstruction scenarios you are worried about are already
> possible without my patch if the parent working copy has no outstanding
> work queue items. My patch has an effect only in the situation where
> the parent working copy has outstanding work queue items. This is not
> obvious from the diff. It is the result of avoiding the call to
> svn_wc__internal_check_wc() in svn_wc_ensure_adm4().
Yes, but after your patch we cannot fix any of these 'scary' scenarios
because anything that would just touch the parent working copy would cause a
regression of this 'fix'.
> Here's what's happening without my patch. I'm running commands in
> a working copy that is currently being checked out. I've got a
> breakpoint set at svn_wc__db_wcroot_parse_local_abspath in the
> svn client that runs the enclosing checkout operation. The enclosing
> working copy is created bit by bit, and as soon as the parent gets
> oustanding work queue items, the nested checkout stops working.
> But it works up to a point, even with the enclosing WC already locked.
Yes, this behavior is part of the WC-NG design. I think some of these
restrictions can be lifted in 1.8 since we finally converted the remaining
workqueue operations that changed db state.
Before we fixed this the DB state was assumed to be unstable until the
workqueue was empty. (E.g. there could be nodes that should have been
deleted in the db).
We can now assume the db to be always stable (guarded by sqlite
transactions), but some in-wc-state can still be lagging behind until the
workqueue is cleared.
> $ svn st
> ! L .
> $ svn co file:///tmp/svn-sandbox/repos/trunk
> A trunk/alpha
> A trunk/epsilon
> A trunk/epsilon/zeta
> A trunk/beta
> A trunk/gamma
> A trunk/gamma/delta
> Checked out revision 2.
>
> (let the parent checkout proceed a bit but don't let it finish)
>
> $ rm -rf trunk
> $ svn st
> ! L .
> $ svn co file:///tmp/svn-sandbox/repos/trunk
> subversion/svn/checkout-cmd.c:168,
> subversion/libsvn_client/checkout.c:197,
> subversion/libsvn_client/checkout.c:123,
> subversion/libsvn_client/checkout.c:63,
> subversion/libsvn_wc/adm_files.c:516,
> subversion/libsvn_wc/adm_files.c:422,
> subversion/libsvn_wc/lock.c:103,
> subversion/libsvn_wc/wc_db.c:12781,
> subversion/libsvn_wc/wc_db_wcroot.c:694,
> subversion/libsvn_wc/wc_db_wcroot.c:310,
> subversion/libsvn_wc/wc_db_wcroot.c:160:
> (apr_err=SVN_ERR_WC_CLEANUP_REQUIRED)
> svn: E155037: Previous operation has not finished; run 'cleanup' if it was
inter
> rupted
>
> So the success of the checkout operation depends on private state (work
> queue items) of an unrelated enclosing working copy. That is bogus.
The design of WC-NG disagrees...
In the old entries world we were always allowed to read the entries, but the
current wc-ng code was designed to explicitly not allow this.
(If you use exclusive locking things get even worse)
The fact that a 'scheduled for delete' directory no longer lives on disk,
appears to be unknown by the checkout code in libsvn_client. It currently
only checks for pre-existing working copies if the directory exists on disk.
(Different issue, but should also be fixed)
>
> While my patch fixes the bogus error, I'm not saying my patch is perfect.
> What it does is that it skips the format check performed by
> svn_wc_ensure_adm4(), which only works correctly if there is no working
> copy with outstanding work queue items in the way. Perhaps a better fix
> would be applied within svn_wc_ensure_adm4(). But I haven't yet found
> a good way of fixing it without passing down a flag that says "we're
> doing a checkout operation right now" into the WC layer.
>
> Does this make sense?
I think the only fix that really makes sense is lifting the restriction that
you can't read from a wc that has workqueue items left.
We should be able to fix this by moving this check to where we obtain a
wc-write-lock. And at this point we are also free to just run the workqueue,
so the combination should reduce the number of times the user sees errors
quite drastically.
Bert
Re: svn commit: r1501163 - in /subversion/trunk/subversion:
include/private/svn_wc_private.h libsvn_client/checkout.c
libsvn_wc/adm_files.c
Posted by Stefan Sperling <st...@apache.org>.
On Tue, Jul 09, 2013 at 12:39:30PM +0200, Bert Huijben wrote:
>
>
> > -----Original Message-----
> > From: Bert Huijben [mailto:bert@qqmail.nl]
> > Sent: dinsdag 9 juli 2013 12:26
> > To: dev@subversion.apache.org; stsp@apache.org
> > Subject: RE: svn commit: r1501163 - in /subversion/trunk/subversion:
> > include/private/svn_wc_private.h libsvn_client/checkout.c
> > libsvn_wc/adm_files.c
> >
> >
> >
> > > -----Original Message-----
> > > From: stsp@apache.org [mailto:stsp@apache.org]
> > > Sent: dinsdag 9 juli 2013 11:35
> > > To: commits@subversion.apache.org
> > > Subject: svn commit: r1501163 - in /subversion/trunk/subversion:
> > > include/private/svn_wc_private.h libsvn_client/checkout.c
> > > libsvn_wc/adm_files.c
> > >
> > > Author: stsp
> > > Date: Tue Jul 9 09:35:12 2013
> > > New Revision: 1501163
> > >
> > > URL: http://svn.apache.org/r1501163
> > > Log:
> > > Allow 'svn checkout' to work within a working copy that is locked.
> > > Fixes a regression from 1.7.
> > >
> > > Reported by: Frank Loeffler <knarf_at_cct lsu edu>
> > > See http://svn.haxx.se/users/archive-2013-07/0066.shtml
> > >
> > > * subversion/include/private/svn_wc_private.h
> > > (svn_wc__init_adm): Declare.
> > >
> > > * subversion/libsvn_client/checkout.c
> > > (initialize_area): Use svn_wc__init_adm() instead of
> > > svn_wc_ensure_adm4().
> > > The latter scans upwards for an existing admin area to check for existing
> > > working copies, which we don't need to do when creating a new WC.
> > >
> > > * subversion/libsvn_wc/adm_files.c
> > > (svn_wc__init_adm): New function, a thin wrapper around init_adm().
> > > This creates a new admin area at a specified local abspath, without
> > > first scanning upwards for an existing admin area. We could also have
> > > created svn_wc_ensure_adm5() with a new 'is_checkout' argument, but
> > > we're trying to reduce the public set of libsvn_wc API functions.
> >
> > I haven't tested this, but this currently appears to remove the safety net
> > around:
> > $ rm trunk
> > $ svn co URL trunk
> > (which would produce an error and now two working copies)
> >
> > Or:
> > $ svn rm trunk
> > $ svn co URL trunk
> > (which will now produce two working copies, with the first partially
> > obstructed)
> >
> > Or even:
> > $ svn up --set-depth excluded trunk
> > $ svn co URL trunk
> >
> > In all these cases 1.6 would have behaved one way, and with single-db we
> > behave in a different way as we don't just attach subdirectories in the parent
> > wc.db.
> >
> > This patch might fix a few use cases, but I don't think it solves a real
> > problem... And it might create a whole heap of new proplems
>
> And detecting any of these in a user friendly way would require reading the parent working copy...
>
> ... but the only reason you added this patch (as far as I can tell) was because reading the parent working copy wasn't possible?
>
>
> I think the recommended approach is making sure the parent working copy is readable, instead of trying to do things 100% without the parent working copy.
>
> Doing things without the parent working copy just gives you all kinds of obstruction scenarios where a simple local_abspath doesn't work as that will just describe a path that is now part of two working copies.
>
> I don't think want to go back to the access batons/wc.db per directory where we could describe these paths that are part of two working copy scenarios.
>
> Bert
Bert, I think you're missing the point.
All the "scary" obstruction scenarios you are worried about are already
possible without my patch if the parent working copy has no outstanding
work queue items. My patch has an effect only in the situation where
the parent working copy has outstanding work queue items. This is not
obvious from the diff. It is the result of avoiding the call to
svn_wc__internal_check_wc() in svn_wc_ensure_adm4().
Here's what's happening without my patch. I'm running commands in
a working copy that is currently being checked out. I've got a
breakpoint set at svn_wc__db_wcroot_parse_local_abspath in the
svn client that runs the enclosing checkout operation. The enclosing
working copy is created bit by bit, and as soon as the parent gets
oustanding work queue items, the nested checkout stops working.
But it works up to a point, even with the enclosing WC already locked.
$ svn st
! L .
$ svn co file:///tmp/svn-sandbox/repos/trunk
A trunk/alpha
A trunk/epsilon
A trunk/epsilon/zeta
A trunk/beta
A trunk/gamma
A trunk/gamma/delta
Checked out revision 2.
(let the parent checkout proceed a bit but don't let it finish)
$ rm -rf trunk
$ svn st
! L .
$ svn co file:///tmp/svn-sandbox/repos/trunk
subversion/svn/checkout-cmd.c:168,
subversion/libsvn_client/checkout.c:197,
subversion/libsvn_client/checkout.c:123,
subversion/libsvn_client/checkout.c:63,
subversion/libsvn_wc/adm_files.c:516,
subversion/libsvn_wc/adm_files.c:422,
subversion/libsvn_wc/lock.c:103,
subversion/libsvn_wc/wc_db.c:12781,
subversion/libsvn_wc/wc_db_wcroot.c:694,
subversion/libsvn_wc/wc_db_wcroot.c:310,
subversion/libsvn_wc/wc_db_wcroot.c:160: (apr_err=SVN_ERR_WC_CLEANUP_REQUIRED)
svn: E155037: Previous operation has not finished; run 'cleanup' if it was inter
rupted
So the success of the checkout operation depends on private state (work
queue items) of an unrelated enclosing working copy. That is bogus.
While my patch fixes the bogus error, I'm not saying my patch is perfect.
What it does is that it skips the format check performed by
svn_wc_ensure_adm4(), which only works correctly if there is no working
copy with outstanding work queue items in the way. Perhaps a better fix
would be applied within svn_wc_ensure_adm4(). But I haven't yet found
a good way of fixing it without passing down a flag that says "we're
doing a checkout operation right now" into the WC layer.
Does this make sense?
RE: svn commit: r1501163 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/checkout.c libsvn_wc/adm_files.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: dinsdag 9 juli 2013 12:26
> To: dev@subversion.apache.org; stsp@apache.org
> Subject: RE: svn commit: r1501163 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h libsvn_client/checkout.c
> libsvn_wc/adm_files.c
>
>
>
> > -----Original Message-----
> > From: stsp@apache.org [mailto:stsp@apache.org]
> > Sent: dinsdag 9 juli 2013 11:35
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1501163 - in /subversion/trunk/subversion:
> > include/private/svn_wc_private.h libsvn_client/checkout.c
> > libsvn_wc/adm_files.c
> >
> > Author: stsp
> > Date: Tue Jul 9 09:35:12 2013
> > New Revision: 1501163
> >
> > URL: http://svn.apache.org/r1501163
> > Log:
> > Allow 'svn checkout' to work within a working copy that is locked.
> > Fixes a regression from 1.7.
> >
> > Reported by: Frank Loeffler <knarf_at_cct lsu edu>
> > See http://svn.haxx.se/users/archive-2013-07/0066.shtml
> >
> > * subversion/include/private/svn_wc_private.h
> > (svn_wc__init_adm): Declare.
> >
> > * subversion/libsvn_client/checkout.c
> > (initialize_area): Use svn_wc__init_adm() instead of
> > svn_wc_ensure_adm4().
> > The latter scans upwards for an existing admin area to check for existing
> > working copies, which we don't need to do when creating a new WC.
> >
> > * subversion/libsvn_wc/adm_files.c
> > (svn_wc__init_adm): New function, a thin wrapper around init_adm().
> > This creates a new admin area at a specified local abspath, without
> > first scanning upwards for an existing admin area. We could also have
> > created svn_wc_ensure_adm5() with a new 'is_checkout' argument, but
> > we're trying to reduce the public set of libsvn_wc API functions.
>
> I haven't tested this, but this currently appears to remove the safety net
> around:
> $ rm trunk
> $ svn co URL trunk
> (which would produce an error and now two working copies)
>
> Or:
> $ svn rm trunk
> $ svn co URL trunk
> (which will now produce two working copies, with the first partially
> obstructed)
>
> Or even:
> $ svn up --set-depth excluded trunk
> $ svn co URL trunk
>
> In all these cases 1.6 would have behaved one way, and with single-db we
> behave in a different way as we don't just attach subdirectories in the parent
> wc.db.
>
> This patch might fix a few use cases, but I don't think it solves a real
> problem... And it might create a whole heap of new proplems
And detecting any of these in a user friendly way would require reading the parent working copy...
... but the only reason you added this patch (as far as I can tell) was because reading the parent working copy wasn't possible?
I think the recommended approach is making sure the parent working copy is readable, instead of trying to do things 100% without the parent working copy.
Doing things without the parent working copy just gives you all kinds of obstruction scenarios where a simple local_abspath doesn't work as that will just describe a path that is now part of two working copies.
I don't think want to go back to the access batons/wc.db per directory where we could describe these paths that are part of two working copy scenarios.
Bert
RE: svn commit: r1501163 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/checkout.c libsvn_wc/adm_files.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: dinsdag 9 juli 2013 11:35
> To: commits@subversion.apache.org
> Subject: svn commit: r1501163 - in /subversion/trunk/subversion:
> include/private/svn_wc_private.h libsvn_client/checkout.c
> libsvn_wc/adm_files.c
>
> Author: stsp
> Date: Tue Jul 9 09:35:12 2013
> New Revision: 1501163
>
> URL: http://svn.apache.org/r1501163
> Log:
> Allow 'svn checkout' to work within a working copy that is locked.
> Fixes a regression from 1.7.
>
> Reported by: Frank Loeffler <knarf_at_cct lsu edu>
> See http://svn.haxx.se/users/archive-2013-07/0066.shtml
>
> * subversion/include/private/svn_wc_private.h
> (svn_wc__init_adm): Declare.
>
> * subversion/libsvn_client/checkout.c
> (initialize_area): Use svn_wc__init_adm() instead of
> svn_wc_ensure_adm4().
> The latter scans upwards for an existing admin area to check for existing
> working copies, which we don't need to do when creating a new WC.
>
> * subversion/libsvn_wc/adm_files.c
> (svn_wc__init_adm): New function, a thin wrapper around init_adm().
> This creates a new admin area at a specified local abspath, without
> first scanning upwards for an existing admin area. We could also have
> created svn_wc_ensure_adm5() with a new 'is_checkout' argument, but
> we're trying to reduce the public set of libsvn_wc API functions.
I haven't tested this, but this currently appears to remove the safety net around:
$ rm trunk
$ svn co URL trunk
(which would produce an error and now two working copies)
Or:
$ svn rm trunk
$ svn co URL trunk
(which will now produce two working copies, with the first partially obstructed)
Or even:
$ svn up --set-depth excluded trunk
$ svn co URL trunk
In all these cases 1.6 would have behaved one way, and with single-db we behave in a different way as we don't just attach subdirectories in the parent wc.db.
This patch might fix a few use cases, but I don't think it solves a real problem... And it might create a whole heap of new propblems
Bert