You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2006/12/21 10:43:33 UTC
[PATCH]operational logging for checkout and update should log revnum
too.
Hi All,
Find the attached patch and log.
With regards
Kamesh Jayachandran
Re: [PATCH]operational logging for checkout and update should log revnum too.
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 29 Jan 2007, Daniel Rall wrote:
...
> Conceptually, I'm really happy with this patch. I made a few tweaks
> as described below (patch attached).
>
> I do have some concerns about the inconsistency in the format of the
> log output. I've tweaked davautocheck.sh to enable operational
> logging (see the "ops" file); here's a snippet:
>
> [29/Jan/2007:17:28:36 -0800] jrandom diff-or-merge '/' r2:1
> [29/Jan/2007:17:31:35 -0800] jrandom diff-or-merge '/A/B/E/alpha@3' '/A2/B/E/alpha@4'
> [29/Jan/2007:17:00:16 -0800] jrandom switch '/iota@1' '/A/D/gamma@1'
> [29/Jan/2007:17:00:17 -0800] jrandom remote-status '/' r1
> [29/Jan/2007:17:00:19 -0800] jrandom checkout-or-export '/' r1
> [29/Jan/2007:16:59:08 -0800] jrandom update '/' r1
> [29/Jan/2007:17:00:21 -0800] jrandom remote-status '/' r1
> [29/Jan/2007:17:00:21 -0800] jrandom commit r2
> [29/Jan/2007:17:00:22 -0800] jrandom checkout-or-export '/' r1
> [29/Jan/2007:17:00:23 -0800] jrandom unlock '/A/D'
> [29/Jan/2007:17:00:24 -0800] jrandom checkout-or-export '/A/D' r1
> [29/Jan/2007:17:00:27 -0800] jrandom remote-status '/' r1
> [29/Jan/2007:17:00:27 -0800] jrandom unlock '/A/D/gamma'
> [29/Jan/2007:17:30:00 -0800] jrandom list-dir '/A/B/T/'
>
> Several differences in revision specification:
> - Single revision, e.g. r1
> - Revision range, e.g. r2:1
> - Peg revision, e.g. /A/B/E/alpha@3
> - No revision (for lines which probably should include a revision)
>
> Any way we make things a little more consistent?
...
After chatting with Ben on IRC, we reached the conclusion that the
first three cases are necessary to handle our various use cases. The
last case isn't typically meaningful, except for 'lock' and 'unlock'.
I went ahead and implemented revision logging for 'list-dir', and have
committed the patch to trunk.
- Dan
Re: [PATCH]operational logging for checkout and update should log
revnum too.
Posted by Kamesh Jayachandran <ka...@collab.net>.
Hi All,
Sorry for a week of delay. Needed some time to understand the context of
'switch/diff-or-merge' commands. That kept me away from completeing this
activity.
Now it is done.
Find the attached patch and log.
With regards
Kamesh Jayachandran
Daniel Rall wrote:
> I responded to this patch on Fri, 29 Dec:
>
> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=122483
>
> I'm basically waiting for a response and/or adjusted patch, but do
> think something like this should be committed.
>
>
> Kamesh sent in another similar patch as well:
>
> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=122383
>
> I like the idea of this one too, but haven't tested it out. It would
> be great if 'davautocheck' would activate operational logging, and we
> had tests which validated that the log file was getting written as
> expected. I'm not sure how this would work with vanilla 'davcheck' --
> perhaps we just start requiring that operational logging be active?
>
> - Dan
>
>
> On Thu, 04 Jan 2007, Hyrum K. Wright wrote:
>
>
>> Kamesh Jayachandran wrote:
>>
>>> Hi All,
>>> Find the attached patch and log.
>>>
>>> With regards
>>> Kamesh Jayachandran
>>>
>> Ping...
>>
>> Anybody had a chance to look at this yet? I'll file an issue if nothing
>> happens in the next few days.
>>
>> -Hyrum
>>
>>
>>> ------------------------------------------------------------------------
>>>
>>> [[[
>>>
>>> Checkout/export and Update should should log the revnum too.
>>>
>>> * subversion/mod_dav_svn/reports/update.c
>>> (dav_svn__update_report):
>>> Log revnum for checkout/export and update.
>>>
>>> Patch by: kameshj
>>> Suggested by: Honey George <ge...@collab.net>
>>>
>>> ]]]
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> Index: subversion/mod_dav_svn/reports/update.c
>>> ===================================================================
>>> --- subversion/mod_dav_svn/reports/update.c (revision 22777)
>>> +++ subversion/mod_dav_svn/reports/update.c (working copy)
>>> @@ -1280,15 +1280,16 @@
>>> reports it (and it alone) to the server as being empty. */
>>> if (entry_counter == 1 && entry_is_empty)
>>> action = apr_psprintf(resource->pool,
>>> - "checkout-or-export '%s'",
>>> - svn_path_uri_encode(spath, resource->pool));
>>> + "checkout-or-export '%s' r%" SVN_REVNUM_T_FMT,
>>> + svn_path_uri_encode(spath, resource->pool),
>>> + revnum);
>>> else
>>> {
>>> if (text_deltas)
>>> action = apr_psprintf(resource->pool,
>>> - "update '%s'",
>>> - svn_path_uri_encode(spath,
>>> - resource->pool));
>>> + "update '%s' r%" SVN_REVNUM_T_FMT,
>>> + svn_path_uri_encode(spath, resource->pool),
>>> + revnum);
>>> else
>>> action = apr_psprintf(resource->pool,
>>> "remote-status '%s'",
>>>
>
>
>
Re: [PATCH]operational logging for checkout and update should log revnum too.
Posted by Daniel Rall <dl...@collab.net>.
I responded to this patch on Fri, 29 Dec:
http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=122483
I'm basically waiting for a response and/or adjusted patch, but do
think something like this should be committed.
Kamesh sent in another similar patch as well:
http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=122383
I like the idea of this one too, but haven't tested it out. It would
be great if 'davautocheck' would activate operational logging, and we
had tests which validated that the log file was getting written as
expected. I'm not sure how this would work with vanilla 'davcheck' --
perhaps we just start requiring that operational logging be active?
- Dan
On Thu, 04 Jan 2007, Hyrum K. Wright wrote:
> Kamesh Jayachandran wrote:
> > Hi All,
> > Find the attached patch and log.
> >
> > With regards
> > Kamesh Jayachandran
>
> Ping...
>
> Anybody had a chance to look at this yet? I'll file an issue if nothing
> happens in the next few days.
>
> -Hyrum
>
> > ------------------------------------------------------------------------
> >
> > [[[
> >
> > Checkout/export and Update should should log the revnum too.
> >
> > * subversion/mod_dav_svn/reports/update.c
> > (dav_svn__update_report):
> > Log revnum for checkout/export and update.
> >
> > Patch by: kameshj
> > Suggested by: Honey George <ge...@collab.net>
> >
> > ]]]
> >
> >
> > ------------------------------------------------------------------------
> >
> > Index: subversion/mod_dav_svn/reports/update.c
> > ===================================================================
> > --- subversion/mod_dav_svn/reports/update.c (revision 22777)
> > +++ subversion/mod_dav_svn/reports/update.c (working copy)
> > @@ -1280,15 +1280,16 @@
> > reports it (and it alone) to the server as being empty. */
> > if (entry_counter == 1 && entry_is_empty)
> > action = apr_psprintf(resource->pool,
> > - "checkout-or-export '%s'",
> > - svn_path_uri_encode(spath, resource->pool));
> > + "checkout-or-export '%s' r%" SVN_REVNUM_T_FMT,
> > + svn_path_uri_encode(spath, resource->pool),
> > + revnum);
> > else
> > {
> > if (text_deltas)
> > action = apr_psprintf(resource->pool,
> > - "update '%s'",
> > - svn_path_uri_encode(spath,
> > - resource->pool));
> > + "update '%s' r%" SVN_REVNUM_T_FMT,
> > + svn_path_uri_encode(spath, resource->pool),
> > + revnum);
> > else
> > action = apr_psprintf(resource->pool,
> > "remote-status '%s'",
>
Re: [PATCH]operational logging for checkout and update should log
revnum too.
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Kamesh Jayachandran wrote:
> Hi All,
> Find the attached patch and log.
>
> With regards
> Kamesh Jayachandran
Ping...
Anybody had a chance to look at this yet? I'll file an issue if nothing
happens in the next few days.
-Hyrum
> ------------------------------------------------------------------------
>
> [[[
>
> Checkout/export and Update should should log the revnum too.
>
> * subversion/mod_dav_svn/reports/update.c
> (dav_svn__update_report):
> Log revnum for checkout/export and update.
>
> Patch by: kameshj
> Suggested by: Honey George <ge...@collab.net>
>
> ]]]
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/mod_dav_svn/reports/update.c
> ===================================================================
> --- subversion/mod_dav_svn/reports/update.c (revision 22777)
> +++ subversion/mod_dav_svn/reports/update.c (working copy)
> @@ -1280,15 +1280,16 @@
> reports it (and it alone) to the server as being empty. */
> if (entry_counter == 1 && entry_is_empty)
> action = apr_psprintf(resource->pool,
> - "checkout-or-export '%s'",
> - svn_path_uri_encode(spath, resource->pool));
> + "checkout-or-export '%s' r%" SVN_REVNUM_T_FMT,
> + svn_path_uri_encode(spath, resource->pool),
> + revnum);
> else
> {
> if (text_deltas)
> action = apr_psprintf(resource->pool,
> - "update '%s'",
> - svn_path_uri_encode(spath,
> - resource->pool));
> + "update '%s' r%" SVN_REVNUM_T_FMT,
> + svn_path_uri_encode(spath, resource->pool),
> + revnum);
> else
> action = apr_psprintf(resource->pool,
> "remote-status '%s'",