Re: [PATCH] fix progress callback for ra_svn

Daniel Shahaf wrote:
> Stefan Küng wrote on Sat, 2 Aug 2008 at 14:45 +0200:
>> Daniel Shahaf wrote:
>>>> An example (pseudo-checkout):
>>>> callback(200, -1) // for connecting
>>>> callback(300, -1) // for info packets
>>>> callback(512, 123000) // first file
>>>> callback(512, 123000) // first file
>>>> callback(512, 123000) // first file
>>>> ...
>>>> callback(30, 123000) // first file
>>>> callback(300, -1) // for info packets
>>>> callback(512, 234000) // second file
>>>> callback(512, 234000) // second file
>>>> callback(512, 234000) // second file
>>>> ...
>>>> callback(30, 234000) // second file
>>>>
>>>> Does this explain it more clear? If not, just ask.
>>>>
>>> I don't understand anything here. The 'total' values you provide are
>>> not very helpful -- they are neither a lower bound nor an upper bound on
>>> the number of bytes that will be transferred -- so what *do* they mean?
>>>
>>> (They are not a lower bound because the last callback() on the last file
>>> will still report that file's size as 'total'. They are not an upper
>>> bound because after the series of calls for the first file, with
>>> total=123000, comes another file.)
>>>
>>> For reference, svn_ra_progress_notify_func_t's says that "@a total is
>>> the total number of bytes to transfer or -1 if it's not known".
>> I use this in TSVN when big files are transferred. Imagine a file has 100MB in
>> size. Without that 'intermediate total' I get from neon, there would be
>> nothing to show to the user. But with that intermediate total, I can show a
>> progress bar so the user can see how long it will take to transfer that big
>> file (the progress bar is hidden again after that big file has been
>> transferred - I only show the bar for big file, not the small ones).
>>
>> I know it's not as good as an upper bound total, but that would require big
>> changes to the svn code to really get that number. And it's better than
>> nothing - I take what I can get.
>>
>
> Okay, thanks for the explanation. It makes sense, but it needs to be
> documented -- our docstring doesn't even say the word "file" right now.

new patch attached with hopefully better docstring.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

[[[
count the bytes transferred the same way as ra_serf and ra_neon:
* subversion/include/svn_ra.h: adjust the doc string to make it clear
                               how the callback works
* subversion/libsvn_ra_svn/marshal.c
  (writebuf_output)
  (readbuf_input): do not sum up the bytes transferred
]]]
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 32354)
+++ subversion/include/svn_ra.h (working copy)
@@ -183,9 +183,12 @@
 /**
  * Callback function type for progress notification.
  *
- * @a progress is the number of bytes already transferred, @a total is
- * the total number of bytes to transfer or -1 if it's not known, @a
- * baton is the callback baton.
+ * @a progress is the number of bytes transferred since the last call
+ * of the callback funtion, @a total is the total number of bytes to
+ * transfer or -1 if it's not known, @a baton is the callback baton.
+ * The total number of bytes to transfer can either be the real total
+ * of the whole command or just the total of a known piece, e.g., a
+ * file.
  *
  * @since New in 1.3.
  */
Index: subversion/libsvn_ra_svn/marshal.c
===================================================================
--- subversion/libsvn_ra_svn/marshal.c (revision 32354)
+++ subversion/libsvn_ra_svn/marshal.c (working copy)
@@ -175,7 +175,7 @@
       if (session)
         {
           const svn_ra_callbacks2_t *cb = session->callbacks;
- session->bytes_written += count;
+ session->bytes_written = count;
 
           if (cb && cb->progress_func)
             (cb->progress_func)(session->bytes_written + session->bytes_read,
@@ -266,7 +266,7 @@
   if (session)
     {
       const svn_ra_callbacks2_t *cb = session->callbacks;
- session->bytes_read += *len;
+ session->bytes_read = *len;
 
       if (cb && cb->progress_func)
         (cb->progress_func)(session->bytes_read + session->bytes_written,