You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/05/11 20:29:11 UTC
svn commit: r943219 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
Author: julianfoad
Date: Tue May 11 18:29:11 2010
New Revision: 943219
URL: http://svn.apache.org/viewvc?rev=943219&view=rev
Log:
* subversion/libsvn_wc/update_editor.c
(choose_base_paths): Rename to get_pristine_base_path; update doc string.
(get_revert_base_checksum): Rename to get_pristine_base_checksum.
(apply_textdelta, close_file): Adjust callers.
Modified:
subversion/trunk/subversion/libsvn_wc/update_editor.c
Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=943219&r1=943218&r2=943219&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue May 11 18:29:11 2010
@@ -4026,16 +4026,17 @@ open_file(const char *path,
/* For the given LOCAL_ABSPATH, set *OLD_TEXT_BASE_ABSPATH to the path of
the text-base file it should revert to: that is the (permanent)
text-base path, or (if the entry is replaced with history) the (permanent)
- revert-base path.
+ revert-base path. Note: This function deals with the WC-1 concept of a
+ pair of deterministic paths for the two possible pristine texts of a node.
- Use SCRATCH_POOL for temporary allocation and for *CHECKSUM_P (if
- applicable), but allocate OLD_TEXT_BASE_ABSPATH in RESULT_POOL. */
+ Allocate OLD_TEXT_BASE_ABSPATH in RESULT_POOL. Use SCRATCH_POOL for
+ temporary allocation. */
static svn_error_t *
-choose_base_paths(const char **old_text_base_abspath,
- svn_wc__db_t *db,
- const char *local_abspath,
- apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
+get_pristine_base_path(const char **old_text_base_abspath,
+ svn_wc__db_t *db,
+ const char *local_abspath,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
const svn_wc_entry_t *entry;
svn_boolean_t replaced;
@@ -4061,11 +4062,11 @@ choose_base_paths(const char **old_text_
* checksum is not known. Allocate *MD5_CHECKSUM in RESULT_POOL.
* LOCAL_ABSPATH must be a file. */
static svn_error_t *
-get_revert_base_checksum(const char **md5_checksum,
- svn_wc__db_t *db,
- const char *local_abspath,
- apr_pool_t *result_pool,
- apr_pool_t *scratch_pool)
+get_pristine_base_checksum(const char **md5_checksum,
+ svn_wc__db_t *db,
+ const char *local_abspath,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
svn_error_t *err;
const svn_checksum_t *checksum;
@@ -4121,17 +4122,17 @@ apply_textdelta(void *file_baton,
text base hasn't been corrupted, and that its checksum
matches the expected base checksum. */
- SVN_ERR(choose_base_paths(&fb->text_base_abspath,
- fb->edit_baton->db, fb->local_abspath,
- fb->pool, pool));
+ SVN_ERR(get_pristine_base_path(&fb->text_base_abspath,
+ fb->edit_baton->db, fb->local_abspath,
+ fb->pool, pool));
/* The incoming delta is targeted against BASE_CHECKSUM. Make sure that
it matches our recorded checksum. (In WC-1, we could not do this test
for replaced nodes because we didn't store the checksum of the "revert
base". In WC-NG, we do and we can.) */
- SVN_ERR(get_revert_base_checksum(&checksum,
- fb->edit_baton->db, fb->local_abspath,
- fb->pool, pool));
+ SVN_ERR(get_pristine_base_checksum(&checksum,
+ fb->edit_baton->db, fb->local_abspath,
+ fb->pool, pool));
if (checksum && base_checksum
&& strcmp(base_checksum, checksum) != 0)
{
@@ -4729,9 +4730,9 @@ close_file(void *file_baton,
&& fb->copied_text_base_abspath);
/* Set up the base paths like apply_textdelta does. */
- SVN_ERR(choose_base_paths(&fb->text_base_abspath,
- eb->db, fb->local_abspath,
- fb->pool, pool));
+ SVN_ERR(get_pristine_base_path(&fb->text_base_abspath,
+ eb->db, fb->local_abspath,
+ fb->pool, pool));
new_text_base_md5_checksum = fb->copied_text_base_md5_checksum;
new_text_base_sha1_checksum = fb->copied_text_base_sha1_checksum;
Re: svn commit: r943219 -
/subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2010-05-13 at 17:10 -0500, Hyrum K. Wright wrote:
> On Thu, May 13, 2010 at 12:21 PM, Julian Foad <ju...@wandisco.com>wrote:
>
> > On Thu, 2010-05-13 at 12:04 -0400, Greg Stein wrote:
> > > On Thu, May 13, 2010 at 08:53, Julian Foad <ju...@wandisco.com>
> > wrote:
> > > >...
> > > > I was trying to do two things: avoid using plain "base" because in
> > > > traditional usage (which is still widespread) it means "WORKING_NODE if
> > > > present else BASE_NODE"; and also identify that it refers to the *text*
> > > > of the BASE_NODE rather than, say, its properties.
> > > >
> > > > Thinking about this now, "text" would be better than "pristine", so I
> > >
> > > hmm? "pristine" has a specific meaning in wc_db. Are you proposing to
> > > change that? Or is this somehow a different concept?
> >
> > "Pristine" has a specific English meaning too, which is more general
> > than the WC-NG meaning, and I'm trying to balance the two.
>
> That may be true, but we should decide what it means for Subversion, and
> then stick with that definition. Balancing multiple definitions is really
> just an exercise in semantics, and is usually more trouble than it is worth.
s/English/the-rest-of-Subversion/: "pristine" has a meaning which was
present before WC-NG and will remain entrenched in the code base and the
developers' minds. For example:
{
...
/** The actual status of the text compared to the pristine base of the
* file. This value isn't masked by other working copy statuses.
* @c pristine_text_status is #svn_wc_status_none if this value was
* not calculated during the status walk.
* @since New in 1.6
*/
enum svn_wc_status_kind pristine_text_status;
/** The actual status of the properties compared to the pristine base of
* the node. This value isn't masked by other working copy statuses.
* @c pristine_prop_status is #svn_wc_status_none if this value was
* not calculated during the status walk.
* @since New in 1.6
*/
enum svn_wc_status_kind pristine_prop_status;
} svn_wc_status2_t;
- Julian
Re: svn commit: r943219 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Thu, May 13, 2010 at 12:21 PM, Julian Foad <ju...@wandisco.com>wrote:
> On Thu, 2010-05-13 at 12:04 -0400, Greg Stein wrote:
> > On Thu, May 13, 2010 at 08:53, Julian Foad <ju...@wandisco.com>
> wrote:
> > >...
> > > I was trying to do two things: avoid using plain "base" because in
> > > traditional usage (which is still widespread) it means "WORKING_NODE if
> > > present else BASE_NODE"; and also identify that it refers to the *text*
> > > of the BASE_NODE rather than, say, its properties.
> > >
> > > Thinking about this now, "text" would be better than "pristine", so I
> >
> > hmm? "pristine" has a specific meaning in wc_db. Are you proposing to
> > change that? Or is this somehow a different concept?
>
> "Pristine" has a specific English meaning too, which is more general
> than the WC-NG meaning, and I'm trying to balance the two.
>
That may be true, but we should decide what it means for Subversion, and
then stick with that definition. Balancing multiple definitions is really
just an exercise in semantics, and is usually more trouble than it is worth.
-Hyrum
Re: svn commit: r943219 -
/subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2010-05-13 at 12:04 -0400, Greg Stein wrote:
> On Thu, May 13, 2010 at 08:53, Julian Foad <ju...@wandisco.com> wrote:
> >...
> > I was trying to do two things: avoid using plain "base" because in
> > traditional usage (which is still widespread) it means "WORKING_NODE if
> > present else BASE_NODE"; and also identify that it refers to the *text*
> > of the BASE_NODE rather than, say, its properties.
> >
> > Thinking about this now, "text" would be better than "pristine", so I
>
> hmm? "pristine" has a specific meaning in wc_db. Are you proposing to
> change that? Or is this somehow a different concept?
"Pristine" has a specific English meaning too, which is more general
than the WC-NG meaning, and I'm trying to balance the two.
> > propose "get_base_text_{checksum,path}", or even
> > "get_base_node_text_{checksum,path}".
> >
> > Actually I intend to replace these local functions with one or more
> > library-scope functions, perhaps like
> >
> > svn_wc__get_base_node_text_info(OUT abspath,
> > OUT sha1_checksum,
> > OUT md5_checksum,
> > OUT file_size,
> > IN db, local_abspath, pools);
> >
> > where the OUT params are optional outputs. Any comments on that?
>
> What's the abspath for? The location in the pristine database?
Yes
> We
> really don't want to throw that around the library. That got us into
> trouble, and we don't want to go back there It is best to stick to
> readonly streams.
It's not optional, within libsvn_wc, at least not yet. We can work on
getting rid of it but we're not there yet.
> What's the "file_size" ... is that "translated_size"? If so, then use
> that name. (stop changing names!)
Not sure yet, actually. If it is going to be 'translated_size' then
I'll call it 'translated_size'. But maybe we'll want pristine text
size. Or both.
> If the translated_size is in this API, then why not last_mod_time?
>
> How is this function different from svn_wc__db_base_get_info() ? Why
> not just use that function?
Good questions. Conceptually it sounds like it makes sense to do so.
/me hesitates to embark on adding more params to that function.
Thanks for the thoughts.
- Julian
Re: svn commit: r943219 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Greg Stein <gs...@gmail.com>.
On Thu, May 13, 2010 at 08:53, Julian Foad <ju...@wandisco.com> wrote:
>...
> I was trying to do two things: avoid using plain "base" because in
> traditional usage (which is still widespread) it means "WORKING_NODE if
> present else BASE_NODE"; and also identify that it refers to the *text*
> of the BASE_NODE rather than, say, its properties.
>
> Thinking about this now, "text" would be better than "pristine", so I
hmm? "pristine" has a specific meaning in wc_db. Are you proposing to
change that? Or is this somehow a different concept?
> propose "get_base_text_{checksum,path}", or even
> "get_base_node_text_{checksum,path}".
>
> Actually I intend to replace these local functions with one or more
> library-scope functions, perhaps like
>
> svn_wc__get_base_node_text_info(OUT abspath,
> OUT sha1_checksum,
> OUT md5_checksum,
> OUT file_size,
> IN db, local_abspath, pools);
>
> where the OUT params are optional outputs. Any comments on that?
What's the abspath for? The location in the pristine database? We
really don't want to throw that around the library. That got us into
trouble, and we don't want to go back there It is best to stick to
readonly streams.
What's the "file_size" ... is that "translated_size"? If so, then use
that name. (stop changing names!)
If the translated_size is in this API, then why not last_mod_time?
How is this function different from svn_wc__db_base_get_info() ? Why
not just use that function?
Cheers,
-g
Re: svn commit: r943219 -
/subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-05-11, Greg Stein wrote:
> On Tue, May 11, 2010 at 14:29, <ju...@apache.org> wrote:
> > Author: julianfoad
> > Date: Tue May 11 18:29:11 2010
> > New Revision: 943219
> >
> > URL: http://svn.apache.org/viewvc?rev=943219&view=rev
> > Log:
> > * subversion/libsvn_wc/update_editor.c
> > (choose_base_paths): Rename to get_pristine_base_path; update doc string.
> > (get_revert_base_checksum): Rename to get_pristine_base_checksum.
> > (apply_textdelta, close_file): Adjust callers.
>
> How is "pristine base" different from simply talking about the BASE
> tree? Seems like you're introducing redundant terms.
I was trying to do two things: avoid using plain "base" because in
traditional usage (which is still widespread) it means "WORKING_NODE if
present else BASE_NODE"; and also identify that it refers to the *text*
of the BASE_NODE rather than, say, its properties.
Thinking about this now, "text" would be better than "pristine", so I
propose "get_base_text_{checksum,path}", or even
"get_base_node_text_{checksum,path}".
Actually I intend to replace these local functions with one or more
library-scope functions, perhaps like
svn_wc__get_base_node_text_info(OUT abspath,
OUT sha1_checksum,
OUT md5_checksum,
OUT file_size,
IN db, local_abspath, pools);
where the OUT params are optional outputs. Any comments on that?
- Julian
Re: svn commit: r943219 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, May 11, 2010 at 14:29, <ju...@apache.org> wrote:
> Author: julianfoad
> Date: Tue May 11 18:29:11 2010
> New Revision: 943219
>
> URL: http://svn.apache.org/viewvc?rev=943219&view=rev
> Log:
> * subversion/libsvn_wc/update_editor.c
> (choose_base_paths): Rename to get_pristine_base_path; update doc string.
> (get_revert_base_checksum): Rename to get_pristine_base_checksum.
> (apply_textdelta, close_file): Adjust callers.
How is "pristine base" different from simply talking about the BASE
tree? Seems like you're introducing redundant terms.
Cheers,
-g