You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Colin Fine <co...@pace.com> on 2009/08/04 14:53:13 UTC

Bug? tmp_file message in Client->log_msg callback.

The documentation for the Perl binding file SVN::Client contains the 
following:
-------------------------------------------
$ctx->log_msg(\&log_msg)

     Sets the log_msg callback for the client context to a code 
reference that you pass. It always returns the current codereference set.

The subroutine pointed to by this coderef will be called to get the log 
message for any operation that will commit a revision to the repo.

It receives 4 parameters. The first parameter is a reference to a scalar 
value in which the callback should place the log_msg. If you wish to 
cancel the commit you can set this scalar to undef. The 2nd value is a 
path to a temporary file which might be holding that log message, or 
undef if no such field exists (though, if log_msg is undef, this value 
is undefined).
---------------------------------------------
I took this to mean that you could provide the log message as either 
text, or a filename containing the text. (It wasn't clear to me whether 
it would accept both, and what it would do with the data if both were 
provided).

However, I found that providing an empty string for the first argument, 
and a filename for the second consistently failed to record any log 
message with the commit.

Looking for more information, I had a look at the source commit.c 
http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/commit.c?revision=38421&view=markup 
(on the trunk, rather than on the 1.5.4 that we're using, I must admit).

As far as I can see, the second argument to svn_client__get_log_msg, 
tmp_file, is completely ignored in svn_client_commit4. (It is used a bit 
in svn_client_import3, but as far as I can tell, for a completely 
different purpose).

It seems to me that this is two bugs: a missing feature in the code, and 
a huge this-is-lying-to-me bug in the documentation. I couldn't see 
anything in the issues list relating to it.
-- 


       *Colin Fine*
       *****Engineering Tools Group
       Pace plc*
       */Bringing Technology Home/*

       *Tel:    +44 1274 538038*
       *****Fax:   +44 1274 538056*
       *Victoria Road, Saltaire, West Yorkshire. BD18 3LF  _
       __www.pace.com_*

       */Think before you print! Please consider the environment before
       printing this e-mail./*


This E-mail and any attachments hereto are strictly confidential and intended solely for the addressee. If you are not the intended addressee please notify the sender by return and delete the message. You must not disclose, forward or copy this E-mail or attachments to any third party without the prior consent of the sender. Pace plc is registered in England and Wales (Company no. 1672847) and our Registered Office is at Victoria Road, Saltaire, West Yorkshire, BD18 3LF, UK. Tel +44 (0) 1274 532000 Fax +44 (0) 1274 532010. <http://www.pace.com>
Save where otherwise agreed in writing between you and Pace (i) all orders for goods and/or services placed by you are made pursuant to Pace's standard terms and conditions of sale which may have been provided to you, or in any event are available at http://www.pace.com/uktcsale.pdf (ii) all orders for goods and/or services placed by Pace are subject to Pace's standard terms and conditions of purchase which may have been provided to you, or in any event are available at http://www.pace.com/uktcpurch.pdf. All other inconsistent terms in any other documentation including without limitation any purchase order, reschedule instruction, order acknowledgement, delivery note or invoice are hereby excluded.



This message has been scanned for viruses by BlackSpider MailControl - www.blackspider.com

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2380042


Re: Bug? tmp_file message in Client->log_msg callback.

Posted by Julian Foad <ju...@btopenworld.com>.
On Tue, 2009-08-04 at 15:53 +0100, Colin Fine wrote:
> The documentation for the Perl binding file SVN::Client contains the 
> following:
> -------------------------------------------
> $ctx->log_msg(\&log_msg)
> 
>      Sets the log_msg callback for the client context to a code 
> reference that you pass. It always returns the current codereference set.
> 
> The subroutine pointed to by this coderef will be called to get the log 
> message for any operation that will commit a revision to the repo.
> 
> It receives 4 parameters. The first parameter is a reference to a scalar 
> value in which the callback should place the log_msg. If you wish to 
> cancel the commit you can set this scalar to undef. The 2nd value is a 
> path to a temporary file which might be holding that log message, or 
> undef if no such field exists (though, if log_msg is undef, this value 
> is undefined).
> ---------------------------------------------
> I took this to mean that you could provide the log message as either 
> text, or a filename containing the text.

No, that's wrong. It means the subroutine must provide the log message
as a scalar (text), unless it wants to abort the commit.

The description of the 2nd parameter got a bit garbled when translated
to Perl. Here is the original version from the C API:

[[[
 * Set @a *log_msg to the log message for the commit, allocated in @a
 * pool, or @c NULL if wish to abort the commit process.  Set @a *tmp_file
 * to the path of any temporary file which might be holding that log
 * message, or @c NULL if no such file exists (though, if @a *log_msg is
 * @c NULL, this value is undefined).  The log message MUST be a UTF8
 * string with LF line separators.
]]]

That means the subroutine should set the 2nd parameter to the path of
the file holding the log message, if any such file exists. I don't know
what the point of this is, given that the first parameter is mandatory,
but that's what it means.

I hope that clears it up. I fixed the typos in that doc string in
r38740.

- Julian


>  (It wasn't clear to me whether 
> it would accept both, and what it would do with the data if both were 
> provided).
> 
> However, I found that providing an empty string for the first argument, 
> and a filename for the second consistently failed to record any log 
> message with the commit.
> 
> Looking for more information, I had a look at the source commit.c 
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/commit.c?revision=38421&view=markup 
> (on the trunk, rather than on the 1.5.4 that we're using, I must admit).
> 
> As far as I can see, the second argument to svn_client__get_log_msg, 
> tmp_file, is completely ignored in svn_client_commit4. (It is used a bit 
> in svn_client_import3, but as far as I can tell, for a completely 
> different purpose).
> 
> It seems to me that this is two bugs: a missing feature in the code, and 
> a huge this-is-lying-to-me bug in the documentation. I couldn't see 
> anything in the issues list relating to it.

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2383485