You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/05/11 18:53:03 UTC
Re: svn commit: r943219 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
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
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