Re: svn commit: r946676 - in /subversion/trunk/subversion/libsvn_wc: adm_files.c adm_files.h adm_ops.c diff.c update_editor.c workqueue.c

[Greg Stein <gs...@gmail.com>]

On Thu, May 20, 2010 at 11:47,  <ju...@apache.org> wrote:
> Author: julianfoad
> Date: Thu May 20 15:47:19 2010
> New Revision: 946676
>
> URL: http://svn.apache.org/viewvc?rev=946676&view=rev
> Log:
> Replace read-only uses of svn_wc__text_base_path() and similar with new
> functions that are named to indicate this usage.  The new functions simply
> forward the call to the original functions, for now, but separating the
> read-only uses from the writes should make it easier to migrate them to use
> the WC-NG Pristine Store; at least it helps me to comprehend that process.
>
> * subversion/libsvn_wc/adm_files.h,
>  subversion/libsvn_wc/adm_files.c
>  (svn_wc__text_revert_path_to_read, svn_wc__ultimate_text_base_path,
>   svn_wc__ultimate_text_base_path_to_read): New functions.
>  (svn_wc__get_pristine_base_contents, svn_wc__get_pristine_contents): Use
>    the new functions.
>
> * subversion/libsvn_wc/adm_ops.c
>  (process_committed_leaf, svn_wc_get_pristine_copy_path): Use the new
>    functions.
>
> * subversion/libsvn_wc/diff.c
>  (get_nearest_pristine_text_as_file, report_wc_file_as_added): Use the new
>    functions.
>
> * subversion/libsvn_wc/update_editor.c
>  (get_pristine_base_path): Move+rename to svn_wc__ultimate_text_base_path().
>  (merge_file, close_file): Use the new functions.
>
> * subversion/libsvn_wc/workqueue.c
>  (verify_pristine_present, log_do_committed): Use the new functions.

Most of these changes are now using svn_wc__text_base_path_to_read(),
which is not one of these "new functions". So... either the log
message needs tweaking because it implies something else, or you
actually intended to use the new functions, but the code didn't get
that far.

(and there is that "ultimate" again)

I know you simply ported existing code over, but with your series of
changes the (newly-named) ultimate path thingy can check for replaced
using svn_wc__internal_is_replaced() rather than fetching an entry. It
has the same semantics if a node is present. It may throw
SVN_ERR_WC_PATH_NOT_FOUND, which you'd need to catch to replace that
"entry != NULL" condition.

Cheers,
-g

Re: svn commit: r946676 - in /subversion/trunk/subversion/libsvn_wc: adm_files.c adm_files.h adm_ops.c diff.c update_editor.c workqueue.c

[Julian Foad <ju...@wandisco.com>]

On Thu, 2010-05-20, Greg Stein wrote:
> On Thu, May 20, 2010 at 11:47,  <ju...@apache.org> wrote:
> > Author: julianfoad
> > Date: Thu May 20 15:47:19 2010
> > New Revision: 946676
> >
> > URL: http://svn.apache.org/viewvc?rev=946676&view=rev
> > Log:
> > Replace read-only uses of svn_wc__text_base_path() and similar with new
> > functions that are named to indicate this usage.  The new functions simply
> > forward the call to the original functions, for now, but separating the
> > read-only uses from the writes should make it easier to migrate them to use
> > the WC-NG Pristine Store; at least it helps me to comprehend that process.
> >
> > * subversion/libsvn_wc/adm_files.h,
> >  subversion/libsvn_wc/adm_files.c
> >  (svn_wc__text_revert_path_to_read, svn_wc__ultimate_text_base_path,
> >   svn_wc__ultimate_text_base_path_to_read): New functions.
> >  (svn_wc__get_pristine_base_contents, svn_wc__get_pristine_contents): Use
> >    the new functions.
> >
> > * subversion/libsvn_wc/adm_ops.c
> >  (process_committed_leaf, svn_wc_get_pristine_copy_path): Use the new
> >    functions.
> >
> > * subversion/libsvn_wc/diff.c
> >  (get_nearest_pristine_text_as_file, report_wc_file_as_added): Use the new
> >    functions.
> >
> > * subversion/libsvn_wc/update_editor.c
> >  (get_pristine_base_path): Move+rename to svn_wc__ultimate_text_base_path().
> >  (merge_file, close_file): Use the new functions.
> >
> > * subversion/libsvn_wc/workqueue.c
> >  (verify_pristine_present, log_do_committed): Use the new functions.
> 
> Most of these changes are now using svn_wc__text_base_path_to_read(),
> which is not one of these "new functions". So... either the log
> message needs tweaking because it implies something else, or you
> actually intended to use the new functions, but the code didn't get
> that far.

I did mean "Use the new functions introduced here and the one introduced
previously".  I have substantially rewritten the log message.

> (and there is that "ultimate" again)

Yup.  For now, when you see "ultimate base", read "WC-NG BASE_NODE
table".  I'll replace it with something else as soon as we can find
something better.  Or even sooner.  I agree it sucks.  I thought it was
clearer than "pristine base" which it replaced.

> I know you simply ported existing code over, but with your series of
> changes the (newly-named) ultimate path thingy can check for replaced
> using svn_wc__internal_is_replaced() rather than fetching an entry. It
> has the same semantics if a node is present. It may throw
> SVN_ERR_WC_PATH_NOT_FOUND, which you'd need to catch to replace that
> "entry != NULL" condition.

Excellent.  Will do.

Thanks.
- Julian