Re: [PATCH] issue 2544 : Add support for 'blame' of WORKING revision

["Hyrum K. Wright" <hy...@mail.utexas.edu>]

Hyrum K. Wright wrote:
> Thomas Leclaire wrote:
>> Hello,
>>
>> Here's a patch for the issue 2544 : "Add support for 'blame' of WORKING
>> revision", which was "svn_client_blame3() and svn_opt_revision_working".
> 
> Has anybody had a chance to review and comment on Thomas's patch?  If nothing
> happens in the next few days, I'll go ahead and attach it to Issue #2544.

Thomas,
Thanks for the patch.  I've gone ahead and attached it to issue 2544 so
it doesn't get forgotten.

-Hyrum

>> [[[
>> Fix issue #2544: Add support for 'blame' of WORKING revision
>>
>> Add -r WORKING option in command line.
>> svn blame -rWORKING look at local modifications for the blame.
>>
>> * subversion/libsvn_subr/opt.c
>>   (revision_from_word): Add option -r WORKING in command line
>>
>> * subversion/include/svn_client.h
>>   (svn_client_blame3): Remove doc string which tells that
>> op_revision_working give error.
>>
>> * subversion/libsvn_client/blame.c
>>   (svn_client_blame3): Remove SVN_ERR_UNSUPPORTED_FEATURE for attempts
>>    to call 'blame' on the WORKING revision.  Add support for this function.
>>
>> * subversion/libsvn_client/cat.c
>>   (svn_client_cat2): Add option svn_opt_revision_working.  It's not for
>> this issue but i've not found an other issue and it's a real minor
>> update discovered during tests.
>>
>>
>> Found by: Stefan Küng <to...@gmail.com>
>> Suggested by: julianfoad, cmpilato, peter n lundblad
>>
>> ]]]
>>
>>
>> Index: subversion/libsvn_subr/opt.c
>> ===================================================================
>> --- subversion/libsvn_subr/opt.c        (revision 23078)
>> +++ subversion/libsvn_subr/opt.c        (working copy)
>> @@ -493,6 +493,8 @@
>>   *
>>   *   - For "committed", set REVISION->kind to svn_opt_revision_committed.
>>   *
>> + *   - For "working", set REVISION->kind to svn_opt_revision_working.
>> + *
>>   * If match, return 0, else return -1 and don't touch REVISION.
>>   */
>> static int
>> @@ -514,6 +516,10 @@
>>      {
>>        revision->kind = svn_opt_revision_committed;
>>      }
>> +  else if (strcasecmp(word, "working") == 0)
>> +    {
>> +      revision->kind = svn_opt_revision_working;
>> +    }
>>    else
>>      return -1;
>>
>>
>> Index: subversion/include/svn_client.h
>> ===================================================================
>> --- subversion/include/svn_client.h     (revision 23078)
>> +++ subversion/include/svn_client.h     (working copy)
>> @@ -1292,11 +1292,9 @@
>>   * WC targets.
>>   *
>>   * If @a start->kind or @a end->kind is @c svn_opt_revision_unspecified,
>> - * return the error @c SVN_ERR_CLIENT_BAD_REVISION.  If either are @c
>> - * svn_opt_revision_working, return the error @c
>> - * SVN_ERR_UNSUPPORTED_FEATURE.  If any of the revisions of @a
>> - * path_or_url have a binary mime-type, return the error @c
>> - * SVN_ERR_CLIENT_IS_BINARY_FILE, unless @a ignore_mime_type is TRUE,
>> + * return the error @c SVN_ERR_CLIENT_BAD_REVISION.  If any of the
>> + * revisions of @a path_or_url have a binary mime-type, return the error
>> + * @c SVN_ERR_CLIENT_IS_BINARY_FILE, unless @a ignore_mime_type is TRUE,
>>   * in which case blame information will be generated regardless of the
>>   * MIME types of the revisions.
>>   *
>>
>>
>> Index: subversion/libsvn_client/blame.c
>> ===================================================================
>> --- subversion/libsvn_client/blame.c    (revision 23078)
>> +++ subversion/libsvn_client/blame.c    (working copy)
>> @@ -515,11 +515,6 @@
>>        || end->kind == svn_opt_revision_unspecified)
>>      return svn_error_create
>>        (SVN_ERR_CLIENT_BAD_REVISION, NULL, NULL);
>> -  else if (start->kind == svn_opt_revision_working
>> -           || end->kind == svn_opt_revision_working)
>> -    return svn_error_create
>> -      (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
>> -       _("blame of the WORKING revision is not supported"));
>>    /* Get an RA plugin for this filesystem object. */
>>    SVN_ERR(svn_client__ra_session_from_path(&ra_session, &end_revnum,
>> @@ -573,6 +568,38 @@
>>    SVN_ERR(err);
>> +  /* Add the blame corresponding to the locale working copy
>> +     for the option -r WORKING */
>> +  if (start->kind == svn_opt_revision_working
>> +     || end->kind == svn_opt_revision_working)
>> +  {
>> +    svn_wc_adm_access_t *adm_access;
>> +    svn_wc_status2_t *status;
>> +
>> +    /* test if the locale file is modified */
>> +    SVN_ERR(svn_wc_adm_open3(&adm_access, NULL,
>> +                             svn_path_dirname(target, pool),
>> +                             FALSE, 0, ctx->cancel_func,
>> +                             ctx->cancel_baton, pool));
>> +    SVN_ERR(svn_wc_status2(&status, target, adm_access, pool));
>> +    if (status->text_status != svn_wc_status_normal)
>> +    {
>> +      frb.rev = apr_palloc(pool, sizeof(*(frb.rev)));
>> +      /* Maybe it could be possible to have 1234M as in local cat
>> +      *  However blame show revision number and next user
>> +      *  (here : "(local)"), so we see immediately that it is modified
>> +      *  but maybe a problem for svn clients (tortoise...) ?? */
>> +      frb.rev->revision = frb.end_rev;
>> +      frb.rev->author = apr_pstrdup(pool, "(local)");
>> +      frb.rev->date = apr_pstrdup(pool, "today"); /* useful ?? */
>> +      frb.rev->next = NULL;
>> +      frb.rev->path = target;
>> +
>> +      SVN_ERR(add_file_blame(frb.last_filename, target, &frb));
>> +      frb.last_filename = target;
>> +    }
>> +  }
>> +
>>    /* Report the blame to the caller. */
>>    /* The callback has to have been called at least once. */
>>
>>
>> Index: subversion/libsvn_client/cat.c
>> ===================================================================
>> --- subversion/libsvn_client/cat.c      (revision 23078)
>> +++ subversion/libsvn_client/cat.c      (working copy)
>> @@ -177,9 +178,11 @@
>>    if (! svn_path_is_url(path_or_url)
>>        && (peg_revision->kind == svn_opt_revision_base
>>            || peg_revision->kind == svn_opt_revision_committed
>> +          || peg_revision->kind == svn_opt_revision_working
>>            || peg_revision->kind == svn_opt_revision_unspecified)
>>        && (revision->kind == svn_opt_revision_base
>>            || revision->kind == svn_opt_revision_committed
>> +          || revision->kind == svn_opt_revision_working
>>            || revision->kind == svn_opt_revision_unspecified))
>>      {
>>        svn_wc_adm_access_t *adm_access;
>>
>>
>>
>> I tested this on command line with different configurations of svn
>> repository and it seems work. However i'm not an usual user of svn and i
>> don't know all ways to use blame. I realized this patch for a project
>> during my studies.
>>
>> Especially, I put the revision number for local version equal to last
>> revision updated. The M added as in cat function is more complex to
>> implements here, also in blame function we have rev_number and next
>> (local) so the M isn't necessary. So it's not a problem for command line
>> -rworking but maybe it causes trouble for some others uses.
>>
>> Also, the option -r WORKING is okay for blame and cat. diff give an
>> adapted error but for others i don't know.
>>
>> Thanks and sorry for my english! ;)
>>
>> -- 
>> Thomas Leclaire