Greg Stein <gstein@lyra.org> writes:
> On Tue, Jan 08, 2002 at 02:18:23PM -0600, sussman@tigris.org wrote:
> >...
> > * mod_dav_svn/update.c (close_helper): if the author or date are NULL,
> > then output "" as the property value (xml cdata).
>
> The property just shouldn't be sent. If the values are NULL, then the
> properties don't exist. "" is an empty author.
No, actually, the property should always be sent; if it's not sent,
the client could end up with a valid CR and 'stale' (non-matching)
author/date props in the entries file.
However, I agree with you that if the value in the fs is NULL, we
shouldn't be sending an empty prop value; we should be sending a
<remove-prop>. I'll fix that now.
> > * libsvn_wc/get_editor.c (change_dir_prop, change_file_prop): if the
> > update editor receives an 'svn:wc:entry:" property with NULL value,
> > then set the entry attribute's value to "".
>
> I would recommend just omitting the property rather than substituting an
> empty string. The two seem to be very different.
As I said above, we always have to send the 3 props, even if some of
them are non-existent. We don't want the 3 props to ever be
out-of-sync with each other in the entries file.
So why not remove the entry attribute when we receive a NULL value?
Karl and I had a chat about this. Here's the deal: the wc update
editor's close_file() writes a zillion log commands, then runs the
log. Unfortunately, our log command that tweaks the entries file
doesn't have the facility to remove entry attributes -- believe it or
not! I don't think we've ever had entry fields that have 'vanished'
before. So we decided -- rather than rewrite this interface -- that a
NULL value for a date or author prop should just be changed to a "" in
the entry attribute. Really, the entries file isn't a true props
database; there's no reason that we have to obey the "NULL implies
delete" rule. The keyword substitution engine certainly won't care if
the value of the author attribute is "".
>
> >...
> > +++ NEW/trunk/subversion/libsvn_wc/get_editor.c Tue Jan 8 14:18:21 2002
> >...
> > - /* push the property into the hash */
> > - apr_hash_set (att_hash, local_name->data, local_name->len, local_value);
> > + /* push the property into the att hash. */
> > + if (local_value)
> > + apr_hash_set (att_hash, local_name->data,
> > + local_name->len, local_value);
> > + else
> > + apr_hash_set (att_hash, local_name->data,
> > + local_name->len, "");
>
> Per my comment above, I'd think that reverting this little bit would get the
> property removed when the value is NULL.
Sorry, our interface doesn't work that way. The absence of a property
in the hash doesn't cause our entry-changing-routine to remove
'unmatched' attributes from the entry. The only way to remove an
entry attribute is to list it specifically in a varargs list, which
our log commands can't currently do. See previous comment.
> Alternatively, you might want to say, "woah. something could already be
> there and we don't want to remove it." In that case, just don't do the set.
> However, I would think you could end up with stale data in that case, so
> removing the property when you get a NULL is probably the best behavior.
Agreed, we want to avoid non-matching, stale data. So for now,
instead of removing the entry attribute, we interpret a NULL value as
setting the attribute to "" -- as I mentioned above.
> > + if (committed_date)
> > + {
> > + name = svn_stringbuf_create (SVN_PROP_ENTRY_COMMITTED_DATE, subpool);
> > + value = svn_stringbuf_create_from_string (committed_date, subpool);
> > + }
> > SVN_ERR ((*pset_func) (real_baton, name, value));
>
> If committed_date == NULL, then you'll end up setting the REV again. The
> pset_func call ought to go inside the "if" block.
Yeah, heh, Philip noticed this bug too. I'm fixing it now. :-)
Thank you for reviewing, guys. I really appreciate it!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:55 2006