One IRC:
<peterS> is it me or is it apache.org: pburba's mail "[PATCH]: Allow
'svn log' on an uncommitted copy destination" has no patch attached.
(I know it's a moot point now, he already committed the patch.)
<pburba> peterS: Hmm, I see a patch
<pburba> peterS: Oh, that attachment is a *.diff, rather than *.diff.txt
<peterS> pburba: in the copy you got back from apache.org, or in your outbox?
<pburba> In my outbox.
<peterS> in my inbox there's no attachment at all, of any content
type, just the message with the [[[ ]]] log at the bottom
<markphip> I think I only see the log message
Trying again with a *.diff.txt extension rather than *.diff, seeing is
that makes any difference.
Paul
On Wed, Jan 20, 2010 at 1:06 PM, Paul Burba <[email protected]> wrote:
> Any reason why we shouldn't be able to run 'svn log' on an uncommitted copy?
>
> For example (this is with tr...@901239):
>
> ### A WC with no local changes:
>
> >svn st
>
> ### Check the log of a file:
>
> >svn log A\D\H\psi
> ------------------------------------------------------------------------
> r4 | jrandom | 2010-01-20 09:23:21 -0500 (Wed, 20 Jan 2010) | 1 line
> log msg
> ------------------------------------------------------------------------
> r1 | jrandom | 2010-01-20 09:23:12 -0500 (Wed, 20 Jan 2010) | 1 line
> Log message for revision 1.
> ------------------------------------------------------------------------
>
> ### Move that same file:
>
> >svn move A\D\H\psi A\D\H\psi_moved
> A A\D\H\psi_moved
> D A\D\H\psi
>
> >svn st
> A + A\D\H\psi_moved
> D A\D\H\psi
>
> ### Run 'svn log' on the move destination. Shouldn't this
> ### give us the log of 'psi'?
>
> >svn log A\D\H\psi_moved
> ..\..\..\subversion\svn\log-cmd.c:600: (apr_err=160013)
> ..\..\..\subversion\libsvn_client\log.c:627: (apr_err=160013)
> ..\..\..\subversion\libsvn_repos\log.c:1449: (apr_err=160013)
> ..\..\..\subversion\libsvn_repos\log.c:1092: (apr_err=160013)
> ..\..\..\subversion\libsvn_fs_fs\tree.c:2818: (apr_err=160013)
> svn: File not found: revision 13, path '/A/D/H/psi_moved'
>
>
> The reason the above doesn't work appears due to a broken API promise
> in svn_client_log5; specifically the part which states:
>
> "@a peg_revision indicates in which revision @a targets are valid.
> If @a peg_revision is #svn_opt_revision_unspecified, it defaults to
> #svn_opt_revision_head for URLs or #svn_opt_revision_working for WC paths"
> ^^^^^^^^^^^^^^^^^^^^^^
>
> Thing is, we don't currently default to a peg rev of
> svn_opt_revision_working in this case. The attached patch makes this
> default take place. It passes all tests and in the above example DTRT
> IMO:
>
> ### tr...@901239 PATCHED:
>
> >svn log A\D\H\psi_moved
> ------------------------------------------------------------------------
> r4 | jrandom | 2010-01-20 09:23:21 -0500 (Wed, 20 Jan 2010) | 1 line
> log msg
> ------------------------------------------------------------------------
> r1 | jrandom | 2010-01-20 09:23:12 -0500 (Wed, 20 Jan 2010) | 1 line
> Log message for revision 1.
> ------------------------------------------------------------------------
>
> Unless anyone has an objection to this, I'll add a test to the
> attached patch and commit...it just seems so obvious that this should
> work that I'm suspicious there isn't a good reason it does not :-P
>
> Paul
>
> [[[
> Allow 'svn log' on an uncommitted copy/move destination.
>
> * subversion/libsvn_client/log.c
>
> (svn_client_log5): Keep API promise that a peg revision
> svn_opt_revision_unspecified defaults to svn_opt_revision_working for
> WC paths.
> ]]]
>
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 901210)
+++ subversion/libsvn_client/log.c (working copy)
@@ -322,6 +322,7 @@
pre_15_receiver_baton_t rb = {0};
apr_pool_t *iterpool;
int i;
+ svn_opt_revision_t peg_rev;
if (revision_ranges->nelts == 0)
{
@@ -330,6 +331,11 @@
_("Missing required revision specification"));
}
+ /* Make a copy of PEG_REVISION, we may need to change it to a
+ default value. */
+ peg_rev.kind = peg_revision->kind;
+ peg_rev.value = peg_revision->value;
+
/* Use the passed URL, if there is one. */
url_or_path = APR_ARRAY_IDX(targets, 0, const char *);
session_opt_rev.kind = svn_opt_revision_unspecified;
@@ -358,7 +364,7 @@
/* Default to any specified peg revision. Otherwise, if the
* first target is an URL, then we default to HEAD:0. Lastly,
* the default is BASE:0 since w...@head may not exist. */
- if (peg_revision->kind == svn_opt_revision_unspecified)
+ if (peg_rev.kind == svn_opt_revision_unspecified)
{
if (svn_path_is_url(url_or_path))
range->start.kind = svn_opt_revision_head;
@@ -366,7 +372,7 @@
range->start.kind = svn_opt_revision_base;
}
else
- range->start = *peg_revision;
+ range->start = peg_rev;
if (range->end.kind == svn_opt_revision_unspecified)
{
@@ -439,6 +445,11 @@
_("When specifying working copy paths, only "
"one target may be given"));
+ /* An unspecified PEG_REVISION for a working copy path defautls
+ to svn_opt_revision_working. */
+ if (peg_rev.kind == svn_opt_revision_unspecified)
+ peg_rev.kind = svn_opt_revision_working;
+
/* Get URLs for each target */
target_urls = apr_array_make(pool, 1, sizeof(const char *));
real_targets = apr_array_make(pool, 1, sizeof(const char *));
@@ -486,14 +497,14 @@
/* If this is a revision type that requires access to the working copy,
* we use our initial target path to figure out where to root the RA
* session, otherwise we use our URL. */
- if (SVN_CLIENT__REVKIND_NEEDS_WC(peg_revision->kind))
+ if (SVN_CLIENT__REVKIND_NEEDS_WC(peg_rev.kind))
SVN_ERR(svn_path_condense_targets(&ra_target, NULL, targets, TRUE,
pool));
else
ra_target = url_or_path;
SVN_ERR(svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
&actual_url, ra_target, NULL,
- peg_revision, &session_opt_rev,
+ &peg_rev, &session_opt_rev,
ctx, pool));
SVN_ERR(svn_ra_has_capability(ra_session, &has_log_revprops,
