You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by iv...@apache.org on 2013/04/16 16:36:07 UTC
svn commit: r1468439 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Author: ivan
Date: Tue Apr 16 14:36:06 2013
New Revision: 1468439
URL: http://svn.apache.org/r1468439
Log:
Fix assertion on checkout from pre-1.8 server over ra_serf with
http-bulk-updates=off.
* subversion/libsvn_ra_serf/update.c
(open_updated_file): Do not walk through file properties. They can be
not fetched yet.
(close_updated_file): Walk through file properties here.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/update.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/update.c?rev=1468439&r1=1468438&r2=1468439&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/update.c Tue Apr 16 14:36:06 2013
@@ -1024,26 +1024,6 @@ open_updated_file(report_info_t *info,
if (info->lock_token)
check_lock(info);
- /* Set all of the properties we received */
- SVN_ERR(svn_ra_serf__walk_all_props(info->props,
- info->base_name,
- info->base_rev,
- set_file_props, info,
- scratch_pool));
- SVN_ERR(svn_ra_serf__walk_all_props(info->dir->removed_props,
- info->base_name,
- info->base_rev,
- remove_file_props, info,
- scratch_pool));
- if (info->fetch_props)
- {
- SVN_ERR(svn_ra_serf__walk_all_props(info->props,
- info->url,
- ctx->target_rev,
- set_file_props, info,
- scratch_pool));
- }
-
/* Get (maybe) a textdelta window handler for transmitting file
content changes. */
if (info->fetch_file || force_apply_textdelta)
@@ -1064,6 +1044,28 @@ static svn_error_t *
close_updated_file(report_info_t *info,
apr_pool_t *scratch_pool)
{
+ report_context_t *ctx = info->dir->report_context;
+
+ /* Set all of the properties we received */
+ SVN_ERR(svn_ra_serf__walk_all_props(info->props,
+ info->base_name,
+ info->base_rev,
+ set_file_props, info,
+ scratch_pool));
+ SVN_ERR(svn_ra_serf__walk_all_props(info->dir->removed_props,
+ info->base_name,
+ info->base_rev,
+ remove_file_props, info,
+ scratch_pool));
+ if (info->fetch_props)
+ {
+ SVN_ERR(svn_ra_serf__walk_all_props(info->props,
+ info->url,
+ ctx->target_rev,
+ set_file_props, info,
+ scratch_pool));
+ }
+
/* Close the file via the editor. */
SVN_ERR(info->dir->report_context->update_editor->close_file(
info->file_baton, info->final_checksum, scratch_pool));
Re: svn commit: r1468439 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Wed, Apr 17, 2013 at 3:21 PM, Bert Huijben <be...@qqmail.nl> wrote:
[...]
>
> Completely unrelated to this patch, but the usage of
> both info->props and info->dir->removed_props looks incorrect.
>
> Both props and removed_props are documented to be stored in
> the directory, but while removed_props is used that way the props
> hash sometimes is and sometimes isn't.
>
> Is the documentation partially outdated, or are we just 'lucky' that
> we don't test property removals that often in our testsuite?
>
I'm not familiar with pool usage in ra_serf update editor. Most likely
we are just 'lucky' because pool handling extremely complicated. So
you can just fix pool usage if you think current implementation has
problems.
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
RE: svn commit: r1468439 - /subversion/trunk/subversion/libsvn_ra_serf/update.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: ivan@apache.org [mailto:ivan@apache.org]
> Sent: dinsdag 16 april 2013 16:36
> To: commits@subversion.apache.org
> Subject: svn commit: r1468439 -
> /subversion/trunk/subversion/libsvn_ra_serf/update.c
>
> Author: ivan
> Date: Tue Apr 16 14:36:06 2013
> New Revision: 1468439
>
> URL: http://svn.apache.org/r1468439
> Log:
> Fix assertion on checkout from pre-1.8 server over ra_serf with
> http-bulk-updates=off.
>
> * subversion/libsvn_ra_serf/update.c
> (open_updated_file): Do not walk through file properties. They can be
> not fetched yet.
> (close_updated_file): Walk through file properties here.
>
> Modified:
> subversion/trunk/subversion/libsvn_ra_serf/update.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/
> update.c?rev=1468439&r1=1468438&r2=1468439&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/update.c Tue Apr 16
> 14:36:06 2013
> @@ -1024,26 +1024,6 @@ open_updated_file(report_info_t *info,
> if (info->lock_token)
> check_lock(info);
>
> - /* Set all of the properties we received */
> - SVN_ERR(svn_ra_serf__walk_all_props(info->props,
> - info->base_name,
> - info->base_rev,
> - set_file_props, info,
> - scratch_pool));
> - SVN_ERR(svn_ra_serf__walk_all_props(info->dir->removed_props,
> - info->base_name,
> - info->base_rev,
> - remove_file_props, info,
> - scratch_pool));
Completely unrelated to this patch, but the usage of both info->props and info->dir->removed_props looks incorrect.
Both props and removed_props are documented to be stored in the directory, but while removed_props is used that way the props hash sometimes is and sometimes isn't.
Is the documentation partially outdated, or are we just 'lucky' that we don't test property removals that often in our testsuite?
Bert