You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/08/17 13:59:26 UTC
Re: svn commit: r32265 - branches/issue-2843-dev/subversion/libsvn_wc
firemeteor@tigris.org writes:
> Log:
> On the issue-2843-dev branch.
>
> Fix a problem when trying to pull in an excluded target while it is actually
> gone in the repos.
>
> * subveraion/libsvn_wc/update_editor.c
> (complete_directory): Remove the target if it is both excluded locally and
> removed in repos.
>
> --- branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c
> +++ branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c
> @@ -450,6 +450,13 @@ make_dir_baton(struct dir_baton **d_p,
> }
>
>
> +/* Forward declaration. */
> +static svn_error_t *
> +do_entry_deletion(struct edit_baton *eb,
> + const char *parent_path,
> + const char *path,
> + int *log_number,
> + apr_pool_t *pool);
Is this forward declaration really necessary? Why not just put
the definition of do_entry_deletion() here instead? (I looked at its
body, and didn't see any circular dependencies that would require a
forward declaration... Was it the call to leftmod_error_chain() you were
worried about?)
> @@ -481,6 +488,7 @@ complete_directory(struct edit_baton *eb
> in. */
> if (eb->depth_is_sticky || *eb->target)
> {
> + svn_wc_adm_access_t *target_access;
> SVN_ERR(svn_wc_adm_retrieve(&adm_access,
> eb->adm_access, path, pool));
> SVN_ERR(svn_wc_entries_read(&entries, adm_access, TRUE, pool));
> @@ -489,6 +497,16 @@ complete_directory(struct edit_baton *eb
> {
> entry->depth = svn_depth_infinity;
> SVN_ERR(svn_wc__entries_write(entries, adm_access, pool));
> + /* There is a small chance that the target is gone in the
> + repository. We'd better get rid of the exclude flag now. */
> + SVN_ERR(svn_wc__adm_retrieve_internal
> + (&target_access, eb->adm_access, eb->target, pool));
> + if (!target_access)
> + {
> + int log_number = 0;
> + SVN_ERR(do_entry_deletion(eb, eb->anchor, eb->target,
> + &log_number, pool));
> + }
I clarified this comment a bit in r32511.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r32265 - branches/issue-2843-dev/subversion/libsvn_wc
Posted by Karl Fogel <kf...@red-bean.com>.
"Rui, Guo" <ti...@mail.ustc.edu.cn> writes:
>> Is this forward declaration really necessary? Why not just put
>> the definition of do_entry_deletion() here instead? (I looked at its
>> body, and didn't see any circular dependencies that would require a
>> forward declaration... Was it the call to leftmod_error_chain() you were
>> worried about?)
>>
> The forward declaration is not necessary but results in less modification. I
> did not think much about this, should I move the whole function altogether
> instead?
I think moving the whole function now would be fine, yes. Thanks!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r32265 -
branches/issue-2843-dev/subversion/libsvn_wc
Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
Karl, I am finally back from my trip and able to response to your revies.
See the comments below.
On Sun, Aug 17, 2008 at 09:59:26AM -0400, Karl Fogel wrote:
> firemeteor@tigris.org writes:
> > Log:
> > On the issue-2843-dev branch.
> >
> > Fix a problem when trying to pull in an excluded target while it is actually
> > gone in the repos.
> >
> > * subveraion/libsvn_wc/update_editor.c
> > (complete_directory): Remove the target if it is both excluded locally and
> > removed in repos.
> >
> > --- branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c
> > +++ branches/issue-2843-dev/subversion/libsvn_wc/update_editor.c
> > @@ -450,6 +450,13 @@ make_dir_baton(struct dir_baton **d_p,
> > }
> >
> >
> > +/* Forward declaration. */
> > +static svn_error_t *
> > +do_entry_deletion(struct edit_baton *eb,
> > + const char *parent_path,
> > + const char *path,
> > + int *log_number,
> > + apr_pool_t *pool);
>
> Is this forward declaration really necessary? Why not just put
> the definition of do_entry_deletion() here instead? (I looked at its
> body, and didn't see any circular dependencies that would require a
> forward declaration... Was it the call to leftmod_error_chain() you were
> worried about?)
>
The forward declaration is not necessary but results in less modification. I
did not think much about this, should I move the whole function altogether
instead?
Rui
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org