You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Steven Brown <sw...@ucsd.edu> on 2003/11/04 04:16:07 UTC
svnversion weirdness with svn:externals
Hello, I recently moved to 0.32.1 from 0.27, and now 'svnversion .' in a
working copy that has a svn:externals directory pulled in reports the
version of the external and the version of the working copy like a mixed
copy, e.g., '106:429'. It used to ignore the external and report '429'.
This new behavior seems horribly wrong, and I assume is a bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: svnversion weirdness with svn:externals
Posted by "C. Michael Pilato" <cm...@collab.net>.
Julian Foad <ju...@btopenworld.com> writes:
> > Ack. Scratch that. This one's cooler. It not only ignores statuses
> > after externals start getting handled -- it uses the cancellation
> > callback to trigger an immediate (or at least much-more-nearly
> > immediate) halting of the status operation after that point.
>
> This depends on the status report sending everything else before the
> externals. Is that guaranteed by its API?
It will be soon. :-)
Seriously, you've made a good point. We let our command-line client
assume too much about the internals of the client libraries with
respect to notification. We really should document the notification
ordering in our client API where it makes sense to do. A careful
client coder today would note that there are no promises about the
notification order, and likely disregard all notifications as a
result.
> Perhaps the optimisation of stopping as soon as you get the first
> external report is over-kill, as the likelihood of someone having a
> very large number of externals is low.
Honestly, I just felt like it would be cool as an example of how to
use the status, notification, and cancellation callbacks in concert.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: svnversion weirdness with svn:externals
Posted by Julian Foad <ju...@btopenworld.com>.
C. Michael Pilato wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>
>>"C. Michael Pilato" <cm...@collab.net> writes:
>>
>>>"Steven Brown" <sw...@ucsd.edu> writes:
>>>
>>>>Hello, I recently moved to 0.32.1 from 0.27, and now 'svnversion .' in a
>>>>working copy that has a svn:externals directory pulled in reports the
>>>>version of the external and the version of the working copy like a mixed
>>>>copy, e.g., '106:429'. It used to ignore the external and report '429'.
>>>>This new behavior seems horribly wrong, and I assume is a bug.
>>>
>>>It's a bug. svnversion uses svn_client_status() under-the-hood, and
>>>in the newer Subversion, the status operation has learned about
>>>svn:externals. For svnversion to work correctly, svn_client_status()
>>>should take a boolean flag for including/ignoring svn:externals (which
>>>is set to "ignore" when called by svnversion).
>>
>>Steven, would you mind testing the following patch to see if it fixes
>>this problem for you?
>
> Ack. Scratch that. This one's cooler. It not only ignores statuses
> after externals start getting handled -- it uses the cancellation
> callback to trigger an immediate (or at least much-more-nearly
> immediate) halting of the status operation after that point.
This depends on the status report sending everything else before the externals. Is that guaranteed by its API?
I hope that the status report will become alphabetically ordered. It might be acceptable to make an exception for externals, and still have them all grouped at the end. Note the recent talk about how the externals might well become integrated into the Entries file after version 1.0.
Perhaps the optimisation of stopping as soon as you get the first external report is over-kill, as the likelihood of someone having a very large number of externals is low.
- Julian
> Start ignoring statuses (and in fact, cancel the rest of the status
> operation) at the first sign of those pesky svn:externals. They will
> taint our version range calculation.
>
> * subversion/svnversion/main.c
> (struct status_baton): add 'done' parameter.
> (analyze_status): Return immediately if we're done.
> (notify, cancel): New.
> (main): Initialize the baton's new 'done' parameter, and register
> the new notification and cancellation functions/batons. Also,
> check for the cancellation error return from svn_client_status(),
> which is non-fatal.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] Re: svnversion weirdness with svn:externals
Posted by Steven Brown <sw...@ucsd.edu>.
> -----Original Message-----
> From: cmpilato@localhost.localdomain
> [mailto:cmpilato@localhost.localdomain] On Behalf Of C. Michael Pilato
> Sent: Tuesday, November 04, 2003 10:55 PM
> To: Steven Brown
> Cc: Subversion Developers
> Subject: [PATCH] Re: svnversion weirdness with svn:externals
>
>
> "C. Michael Pilato" <cm...@collab.net> writes:
>
> > "C. Michael Pilato" <cm...@collab.net> writes:
> >
> > > "Steven Brown" <sw...@ucsd.edu> writes:
> > >
> > > > Hello, I recently moved to 0.32.1 from 0.27, and now
> 'svnversion .' in a
> > > > working copy that has a svn:externals directory pulled
> in reports the
> > > > version of the external and the version of the working
> copy like a mixed
> > > > copy, e.g., '106:429'. It used to ignore the external
> and report '429'.
> > > > This new behavior seems horribly wrong, and I assume is a bug.
> > >
> > > It's a bug. svnversion uses svn_client_status()
> under-the-hood, and
> > > in the newer Subversion, the status operation has learned about
> > > svn:externals. For svnversion to work correctly,
> svn_client_status()
> > > should take a boolean flag for including/ignoring
> svn:externals (which
> > > is set to "ignore" when called by svnversion).
> >
> > Steven, would you mind testing the following patch to see
> if it fixes
> > this problem for you?
>
> Ack. Scratch that. This one's cooler. It not only ignores statuses
> after externals start getting handled -- it uses the cancellation
> callback to trigger an immediate (or at least much-more-nearly
> immediate) halting of the status operation after that point.
Looks good here; getting '431' rather than the oddball '106:431' now.
> -----
>
> Start ignoring statuses (and in fact, cancel the rest of the status
> operation) at the first sign of those pesky svn:externals. They will
> taint our version range calculation.
>
> * subversion/svnversion/main.c
> (struct status_baton): add 'done' parameter.
> (analyze_status): Return immediately if we're done.
> (notify, cancel): New.
> (main): Initialize the baton's new 'done' parameter, and register
> the new notification and cancellation functions/batons. Also,
> check for the cancellation error return from svn_client_status(),
> which is non-fatal.
>
> Index: subversion/svnversion/main.c
> ===================================================================
> --- subversion/svnversion/main.c (revision 7634)
> +++ subversion/svnversion/main.c (working copy)
> @@ -30,6 +30,7 @@
> svn_boolean_t modified; /* is anything modified? */
> const char *wc_path; /* path whose URL we're looking for. */
> const char *wc_url; /* URL for the path whose URL
> we're looking for. */
> + svn_boolean_t done; /* note completion of our task. */
> apr_pool_t *pool; /* pool in which to store
> alloc-needy things. */
> };
>
> @@ -42,6 +43,9 @@
> svn_wc_status_t *status)
> {
> struct status_baton *sb = baton;
> +
> + if (sb->done)
> + return;
>
> if (! status->entry)
> return;
> @@ -70,7 +74,38 @@
> sb->wc_url = apr_pstrdup (sb->pool, status->entry->url);
> }
>
> +
> +/* This implements `svn_wc_notify_func_t'. */
> static void
> +notify (void *baton,
> + const char *path,
> + svn_wc_notify_action_t action,
> + svn_node_kind_t kind,
> + const char *mime_type,
> + svn_wc_notify_state_t content_state,
> + svn_wc_notify_state_t prop_state,
> + svn_revnum_t revision)
> +{
> + struct status_baton *sb = baton;
> + if ((action == svn_wc_notify_status_external)
> + || (action == svn_wc_notify_status_completed))
> + sb->done = TRUE;
> +}
> +
> +
> +/* This implements `svn_cancel_func_t'. */
> +static svn_error_t *
> +cancel (void *baton)
> +{
> + struct status_baton *sb = baton;
> + if (sb->done)
> + return svn_error_create (SVN_ERR_CANCELLED, NULL, "Finished");
> + else
> + return SVN_NO_ERROR;
> +}
> +
> +
> +static void
> usage(void)
> {
> fprintf(stderr,
> @@ -117,6 +152,7 @@
> svn_client_ctx_t ctx = { 0 };
> struct status_baton sb;
> svn_opt_revision_t rev;
> + svn_error_t *err;
>
> if (argc != 2 && argc != 3)
> {
> @@ -168,11 +204,24 @@
> sb.max_rev = SVN_INVALID_REVNUM;
> sb.wc_path = wc_path;
> sb.wc_url = NULL;
> + sb.done = FALSE;
> sb.pool = pool;
> rev.kind = svn_opt_revision_unspecified;
> - SVN_INT_ERR (svn_client_status (&youngest, wc_path, &rev,
> analyze_status,
> - &sb, TRUE, TRUE, FALSE,
> FALSE, &ctx, pool));
>
> + /* Setup the notification and cancellation callbacks, and their
> + shared baton (which is also shared with the status function). */
> + ctx.notify_func = notify;
> + ctx.notify_baton = &sb;
> + ctx.cancel_func = cancel;
> + ctx.cancel_baton = &sb;
> +
> + err = svn_client_status (&youngest, wc_path, &rev, analyze_status,
> + &sb, TRUE, TRUE, FALSE, FALSE,
> &ctx, pool);
> + if (err && (err->apr_err == SVN_ERR_CANCELLED))
> + svn_error_clear (err);
> + else
> + SVN_INT_ERR (err);
> +
> if ((! sb.switched ) && (argc == 3))
> {
> /* If the trailing part of the URL of the working copy
> directory
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] Re: svnversion weirdness with svn:externals
Posted by "C. Michael Pilato" <cm...@collab.net>.
"C. Michael Pilato" <cm...@collab.net> writes:
> "C. Michael Pilato" <cm...@collab.net> writes:
>
> > "Steven Brown" <sw...@ucsd.edu> writes:
> >
> > > Hello, I recently moved to 0.32.1 from 0.27, and now 'svnversion .' in a
> > > working copy that has a svn:externals directory pulled in reports the
> > > version of the external and the version of the working copy like a mixed
> > > copy, e.g., '106:429'. It used to ignore the external and report '429'.
> > > This new behavior seems horribly wrong, and I assume is a bug.
> >
> > It's a bug. svnversion uses svn_client_status() under-the-hood, and
> > in the newer Subversion, the status operation has learned about
> > svn:externals. For svnversion to work correctly, svn_client_status()
> > should take a boolean flag for including/ignoring svn:externals (which
> > is set to "ignore" when called by svnversion).
>
> Steven, would you mind testing the following patch to see if it fixes
> this problem for you?
Ack. Scratch that. This one's cooler. It not only ignores statuses
after externals start getting handled -- it uses the cancellation
callback to trigger an immediate (or at least much-more-nearly
immediate) halting of the status operation after that point.
-----
Start ignoring statuses (and in fact, cancel the rest of the status
operation) at the first sign of those pesky svn:externals. They will
taint our version range calculation.
* subversion/svnversion/main.c
(struct status_baton): add 'done' parameter.
(analyze_status): Return immediately if we're done.
(notify, cancel): New.
(main): Initialize the baton's new 'done' parameter, and register
the new notification and cancellation functions/batons. Also,
check for the cancellation error return from svn_client_status(),
which is non-fatal.
Index: subversion/svnversion/main.c
===================================================================
--- subversion/svnversion/main.c (revision 7634)
+++ subversion/svnversion/main.c (working copy)
@@ -30,6 +30,7 @@
svn_boolean_t modified; /* is anything modified? */
const char *wc_path; /* path whose URL we're looking for. */
const char *wc_url; /* URL for the path whose URL we're looking for. */
+ svn_boolean_t done; /* note completion of our task. */
apr_pool_t *pool; /* pool in which to store alloc-needy things. */
};
@@ -42,6 +43,9 @@
svn_wc_status_t *status)
{
struct status_baton *sb = baton;
+
+ if (sb->done)
+ return;
if (! status->entry)
return;
@@ -70,7 +74,38 @@
sb->wc_url = apr_pstrdup (sb->pool, status->entry->url);
}
+
+/* This implements `svn_wc_notify_func_t'. */
static void
+notify (void *baton,
+ const char *path,
+ svn_wc_notify_action_t action,
+ svn_node_kind_t kind,
+ const char *mime_type,
+ svn_wc_notify_state_t content_state,
+ svn_wc_notify_state_t prop_state,
+ svn_revnum_t revision)
+{
+ struct status_baton *sb = baton;
+ if ((action == svn_wc_notify_status_external)
+ || (action == svn_wc_notify_status_completed))
+ sb->done = TRUE;
+}
+
+
+/* This implements `svn_cancel_func_t'. */
+static svn_error_t *
+cancel (void *baton)
+{
+ struct status_baton *sb = baton;
+ if (sb->done)
+ return svn_error_create (SVN_ERR_CANCELLED, NULL, "Finished");
+ else
+ return SVN_NO_ERROR;
+}
+
+
+static void
usage(void)
{
fprintf(stderr,
@@ -117,6 +152,7 @@
svn_client_ctx_t ctx = { 0 };
struct status_baton sb;
svn_opt_revision_t rev;
+ svn_error_t *err;
if (argc != 2 && argc != 3)
{
@@ -168,11 +204,24 @@
sb.max_rev = SVN_INVALID_REVNUM;
sb.wc_path = wc_path;
sb.wc_url = NULL;
+ sb.done = FALSE;
sb.pool = pool;
rev.kind = svn_opt_revision_unspecified;
- SVN_INT_ERR (svn_client_status (&youngest, wc_path, &rev, analyze_status,
- &sb, TRUE, TRUE, FALSE, FALSE, &ctx, pool));
+ /* Setup the notification and cancellation callbacks, and their
+ shared baton (which is also shared with the status function). */
+ ctx.notify_func = notify;
+ ctx.notify_baton = &sb;
+ ctx.cancel_func = cancel;
+ ctx.cancel_baton = &sb;
+
+ err = svn_client_status (&youngest, wc_path, &rev, analyze_status,
+ &sb, TRUE, TRUE, FALSE, FALSE, &ctx, pool);
+ if (err && (err->apr_err == SVN_ERR_CANCELLED))
+ svn_error_clear (err);
+ else
+ SVN_INT_ERR (err);
+
if ((! sb.switched ) && (argc == 3))
{
/* If the trailing part of the URL of the working copy directory
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svnversion weirdness with svn:externals
Posted by "C. Michael Pilato" <cm...@collab.net>.
"C. Michael Pilato" <cm...@collab.net> writes:
> "Steven Brown" <sw...@ucsd.edu> writes:
>
> > Hello, I recently moved to 0.32.1 from 0.27, and now 'svnversion .' in a
> > working copy that has a svn:externals directory pulled in reports the
> > version of the external and the version of the working copy like a mixed
> > copy, e.g., '106:429'. It used to ignore the external and report '429'.
> > This new behavior seems horribly wrong, and I assume is a bug.
>
> It's a bug. svnversion uses svn_client_status() under-the-hood, and
> in the newer Subversion, the status operation has learned about
> svn:externals. For svnversion to work correctly, svn_client_status()
> should take a boolean flag for including/ignoring svn:externals (which
> is set to "ignore" when called by svnversion).
Steven, would you mind testing the following patch to see if it fixes
this problem for you?
----------
Start ignoring statuses at the first sign of those pesky
svn:externals. They will taint our version range calculation.
* subversion/svnversion/main.c
(struct status_baton): add 'ignore' parameter.
(analyze_status): Return immediately if we're told to ignore
statuses.
(notify): New.
(main): Populate the baton's new 'ignore' parameter, and register
the new notification function and baton.
Index: subversion/svnversion/main.c
===================================================================
--- subversion/svnversion/main.c (revision 7634)
+++ subversion/svnversion/main.c (working copy)
@@ -30,6 +30,7 @@
svn_boolean_t modified; /* is anything modified? */
const char *wc_path; /* path whose URL we're looking for. */
const char *wc_url; /* URL for the path whose URL we're looking for. */
+ svn_boolean_t ignore; /* ignore status until further notice. */
apr_pool_t *pool; /* pool in which to store alloc-needy things. */
};
@@ -42,6 +43,9 @@
svn_wc_status_t *status)
{
struct status_baton *sb = baton;
+
+ if (sb->ignore)
+ return;
if (! status->entry)
return;
@@ -70,7 +74,26 @@
sb->wc_url = apr_pstrdup (sb->pool, status->entry->url);
}
+
+/* This implements `svn_wc_notify_func_t'. */
static void
+notify (void *baton,
+ const char *path,
+ svn_wc_notify_action_t action,
+ svn_node_kind_t kind,
+ const char *mime_type,
+ svn_wc_notify_state_t content_state,
+ svn_wc_notify_state_t prop_state,
+ svn_revnum_t revision)
+{
+ struct status_baton *sb = baton;
+ if ((action == svn_wc_notify_status_external)
+ || (action == svn_wc_notify_status_completed))
+ sb->ignore = TRUE;
+}
+
+
+static void
usage(void)
{
fprintf(stderr,
@@ -168,8 +191,15 @@
sb.max_rev = SVN_INVALID_REVNUM;
sb.wc_path = wc_path;
sb.wc_url = NULL;
+ sb.ignore = FALSE;
sb.pool = pool;
rev.kind = svn_opt_revision_unspecified;
+
+ /* Setup the notification callback, and its baton (which is shared
+ with the status function). */
+ ctx.notify_func = notify;
+ ctx.notify_baton = &sb;
+
SVN_INT_ERR (svn_client_status (&youngest, wc_path, &rev, analyze_status,
&sb, TRUE, TRUE, FALSE, FALSE, &ctx, pool));
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svnversion weirdness with svn:externals
Posted by "C. Michael Pilato" <cm...@collab.net>.
"Steven Brown" <sw...@ucsd.edu> writes:
> Hello, I recently moved to 0.32.1 from 0.27, and now 'svnversion .' in a
> working copy that has a svn:externals directory pulled in reports the
> version of the external and the version of the working copy like a mixed
> copy, e.g., '106:429'. It used to ignore the external and report '429'.
> This new behavior seems horribly wrong, and I assume is a bug.
It's a bug. svnversion uses svn_client_status() under-the-hood, and
in the newer Subversion, the status operation has learned about
svn:externals. For svnversion to work correctly, svn_client_status()
should take a boolean flag for including/ignoring svn:externals (which
is set to "ignore" when called by svnversion).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org