You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2011/10/24 14:35:21 UTC
Committing incomplete directories
A user reported an assertion failure in 1.7:
http://subversion.tigris.org/issues/show_bug.cgi?id=4042
caused by committing a property change on a presence=incomplete
directory. It's not possible to prevent property modifications on
incomplete directories, since the modification could be made before the
directory becomes incomplete.
In 1.6 such a commit was allowed and, provided the server accepted the
commit, the directory remained incomplete in the working copy.
Should we support that in 1.7? Or should we simply have the client
refuse to commit incomplete nodes. Most incomplete nodes occur after an
interrupted update, but wc could have incomplete working nodes resulting
from a wc-wc copy.
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: Committing incomplete directories
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> Julian Foad writes:
> > Philip Martin wrote:
> >> http://subversion.tigris.org/issues/show_bug.cgi?id=4042
> > [...] Should we support that in 1.7? Or should we simply have the
> >> client refuse to commit incomplete nodes. Most incomplete nodes
> >> occur after an interrupted update, but wc could have incomplete
> >> working nodes resulting from a wc-wc copy.
> >
> > I think we should start refusing to commit changes to or in an
> > incomplete' directory, in order to keep things simple. I don't see a
> > practical reason why such a commit should be supported, only the
> > eternal desire to keep backward-compatibility. Back-compat is very
> > important to me in general, but here it appears to me that we have an
> > example of a behaviour which isn't wanted and merely happened to
> > exist. (I mean the ability to commit a dir while it's marked
> > incomplete' is not important, I don't mean nobody ever finds a use for
> > it.) Please speak up if I'm wrong about that.
>
> I think it's relatively simple to make 1.7 behave the same
> as 1.6:
I have no objection to you fixing it up in the short term. My point can be considered separately, longer term.
- Julian
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> SVN_ERR_ASSERT(status == svn_wc__db_status_normal
> + || status == svn_wc__db_status_incomplete
> || status == svn_wc__db_status_added);
>
> Index: subversion/libsvn_wc/wc_db.c
> ===================================================================
>
> + old_presence = svn_sqlite__column_token(stmt_info, 3, presence_map);
> + /* ### other stuff? */
>
[...]
> - /* ### other presences? or reserve that for separate functions? */
> - new_presence = svn_wc__db_status_normal;
> + /* Preserve any incomplete status */
> + new_presence = (old_presence == svn_wc__db_status_incomplete
> + ? svn_wc__db_status_incomplete
> + : svn_wc__db_status_normal);
Re: Committing incomplete directories
Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@btopenworld.com> writes:
> Philip Martin <ph...@wandisco.com> wrote:
>> http://subversion.tigris.org/issues/show_bug.cgi?id=4042
> [...] Should we support that in 1.7? Or should we simply have the
>> client refuse to commit incomplete nodes. Most incomplete nodes
>> occur after an interrupted update, but wc could have incomplete
>> working nodes resulting from a wc-wc copy.
>
> I think we should start refusing to commit changes to or in an
> incomplete' directory, in order to keep things simple. I don't see a
> practical reason why such a commit should be supported, only the
> eternal desire to keep backward-compatibility. Back-compat is very
> important to me in general, but here it appears to me that we have an
> example of a behaviour which isn't wanted and merely happened to
> exist. (I mean the ability to commit a dir while it's marked
> incomplete' is not important, I don't mean nobody ever finds a use for
> it.) Please speak up if I'm wrong about that.
I think it's relatively simple to make 1.7 behave the same as 1.6:
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 1188176)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -174,6 +174,7 @@
}
SVN_ERR_ASSERT(status == svn_wc__db_status_normal
+ || status == svn_wc__db_status_incomplete
|| status == svn_wc__db_status_added);
if (kind != svn_kind_dir)
Index: subversion/libsvn_wc/wc_db.c
===================================================================
--- subversion/libsvn_wc/wc_db.c (revision 1188176)
+++ subversion/libsvn_wc/wc_db.c (working copy)
@@ -8567,6 +8567,7 @@
apr_int64_t repos_id;
const char *repos_relpath;
apr_int64_t op_depth;
+ svn_wc__db_status_t old_presence;
/* If we are adding a file or directory, then we need to get
repository information from the parent node since "this node" does
@@ -8629,6 +8630,8 @@
if (cb->keep_changelist && have_act)
changelist = svn_sqlite__column_text(stmt_act, 1, scratch_pool);
+ old_presence = svn_sqlite__column_token(stmt_info, 3, presence_map);
+
/* ### other stuff? */
SVN_ERR(svn_sqlite__reset(stmt_info));
@@ -8684,8 +8687,10 @@
else
parent_relpath = svn_relpath_dirname(local_relpath, scratch_pool);
- /* ### other presences? or reserve that for separate functions? */
- new_presence = svn_wc__db_status_normal;
+ /* Preserve any incomplete status */
+ new_presence = (old_presence == svn_wc__db_status_incomplete
+ ? svn_wc__db_status_incomplete
+ : svn_wc__db_status_normal);
SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
STMT_APPLY_CHANGES_TO_BASE_NODE));
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: Committing incomplete directories
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@wandisco.com> writes:
> What does refusing to commit incomplete directories mean. Given:
>
> $ svn st wc
> ! wc/A/B
> M wc/A/B/C/f
> !M wc/X/Y
Typo, I meant
! wc/X/Y
> M wc/X/B/C/f
> M wc/P/f
>
> Do these fail due to incomplete:
>
> $ svn ci wc/A/B/C/f # explict inside incomplete
>
> $ svn ci wc/A # implicit inside incomplete
>
> $ svn ci wc/X # implicit beside incomplete
>
> $ svn ci wc/P # incomplete in some other part of wc
--
Philip
Re: Committing incomplete directories
Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@btopenworld.com> writes:
> Philip Martin <ph...@wandisco.com> wrote:
>> http://subversion.tigris.org/issues/show_bug.cgi?id=4042
> [...] Should we support that in 1.7? Or should we simply have the
>> client refuse to commit incomplete nodes. Most incomplete nodes
>> occur after an interrupted update, but wc could have incomplete
>> working nodes resulting from a wc-wc copy.
>
> I think we should start refusing to commit changes to or in an
> incomplete' directory, in order to keep things simple. I don't see a
> practical reason why such a commit should be supported, only the
> eternal desire to keep backward-compatibility. Back-compat is very
> important to me in general, but here it appears to me that we have an
> example of a behaviour which isn't wanted and merely happened to
> exist. (I mean the ability to commit a dir while it's marked
> incomplete' is not important, I don't mean nobody ever finds a use for
> it.) Please speak up if I'm wrong about that.
What does refusing to commit incomplete directories mean. Given:
$ svn st wc
! wc/A/B
M wc/A/B/C/f
!M wc/X/Y
M wc/X/B/C/f
M wc/P/f
Do these fail due to incomplete:
$ svn ci wc/A/B/C/f # explict inside incomplete
$ svn ci wc/A # implicit inside incomplete
$ svn ci wc/X # implicit beside incomplete
$ svn ci wc/P # incomplete in some other part of wc
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Re: Committing incomplete directories
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin <ph...@wandisco.com> wrote:
> http://subversion.tigris.org/issues/show_bug.cgi?id=4042
[...]
> Should we support that in 1.7? Or should we simply have the client
> refuse to commit incomplete nodes. Most incomplete nodes occur
> after an interrupted update, but wc could have incomplete working
> nodes resulting from a wc-wc copy.
I think we should start refusing to commit changes to or in an 'incomplete' directory, in order to keep things simple. I don't see a practical reason why such a commit should be supported, only the eternal desire to keep backward-compatibility. Back-compat is very important to me in general, but here it appears to me that we have an example of a behaviour which isn't wanted and merely happened to exist. (I mean the ability to commit a dir while it's marked 'incomplete' is not important, I don't mean nobody ever finds a use for it.) Please speak up if I'm wrong about that.
- Julian