You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@finemaltcoding.com> on 2009/03/17 00:20:57 UTC

Re: svn commit: r36592 - trunk/subversion/libsvn_wc

It'd be great if this change was accompanied by a unit test.

On Mon, Mar 16, 2009 at 8:03 AM, Hyrum K. Wright <hy...@hyrumwright.org> wrote:
> Author: hwright
> Date: Mon Mar 16 08:03:02 2009
> New Revision: 36592
>
> Log:
> Relax a few assertions in the wc_db APIs.  It turns out that we can have
> non-existent authors and changed dates, in the case of revprop-edited
> revisions.
>
> * subversion/libsvn_wc/wc_db.c
>  (svn_wc__db_base_add_directory, svn_wc__db_base_add_file,
>   svn_wc__db_baes_add_symlink, svn_wc__db_base_add_subdir):
>    Remove assertions on changed date and author.
>
> Modified:
>   trunk/subversion/libsvn_wc/wc_db.c
>
> Modified: trunk/subversion/libsvn_wc/wc_db.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/wc_db.c?pathrev=36592&r1=36591&r2=36592
> ==============================================================================
> --- trunk/subversion/libsvn_wc/wc_db.c  Mon Mar 16 07:34:22 2009        (r36591)
> +++ trunk/subversion/libsvn_wc/wc_db.c  Mon Mar 16 08:03:02 2009        (r36592)
> @@ -1330,8 +1330,6 @@ svn_wc__db_base_add_directory(svn_wc__db
>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>   SVN_ERR_ASSERT(props != NULL);
>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
> -  SVN_ERR_ASSERT(changed_date > 0);
> -  SVN_ERR_ASSERT(changed_author != NULL);
>   SVN_ERR_ASSERT(children != NULL);
>
>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
> @@ -1394,8 +1392,6 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>   SVN_ERR_ASSERT(props != NULL);
>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
> -  SVN_ERR_ASSERT(changed_date > 0);
> -  SVN_ERR_ASSERT(changed_author != NULL);
>   SVN_ERR_ASSERT(checksum != NULL);
>
>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
> @@ -1457,8 +1453,6 @@ svn_wc__db_base_add_symlink(svn_wc__db_t
>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>   SVN_ERR_ASSERT(props != NULL);
>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
> -  SVN_ERR_ASSERT(changed_date > 0);
> -  SVN_ERR_ASSERT(changed_author != NULL);
>   SVN_ERR_ASSERT(target != NULL);
>
>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
> @@ -1580,8 +1574,6 @@ svn_wc__db_temp_base_add_subdir(svn_wc__
>   SVN_ERR_ASSERT(repos_uuid != NULL);
>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
> -  SVN_ERR_ASSERT(changed_date > 0);
> -  SVN_ERR_ASSERT(changed_author != NULL);
>
>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>                               svn_sqlite__mode_readwrite,
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1333819
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1336220


Re: svn commit: r36592 - trunk/subversion/libsvn_wc

Posted by Daniel Rall <dl...@finemaltcoding.com>.
Pass nulls to the routines, probably in API-level tests -- if the
tests blow up, you failed. Such tests reinforce the doc strings (which
I didn't see change in this commit) declaring that nulls are allowed
values, and assure that subsequent refactoring doesn't change the
behavior.

On Mon, Mar 16, 2009 at 5:59 PM, Greg Stein <gs...@gmail.com> wrote:
> Eh? How would you test this?
>
> On Tue, Mar 17, 2009 at 01:20, Daniel Rall <dl...@finemaltcoding.com> wrote:
>> It'd be great if this change was accompanied by a unit test.
>>
>> On Mon, Mar 16, 2009 at 8:03 AM, Hyrum K. Wright <hy...@hyrumwright.org> wrote:
>>> Author: hwright
>>> Date: Mon Mar 16 08:03:02 2009
>>> New Revision: 36592
>>>
>>> Log:
>>> Relax a few assertions in the wc_db APIs.  It turns out that we can have
>>> non-existent authors and changed dates, in the case of revprop-edited
>>> revisions.
>>>
>>> * subversion/libsvn_wc/wc_db.c
>>>  (svn_wc__db_base_add_directory, svn_wc__db_base_add_file,
>>>   svn_wc__db_baes_add_symlink, svn_wc__db_base_add_subdir):
>>>    Remove assertions on changed date and author.
>>>
>>> Modified:
>>>   trunk/subversion/libsvn_wc/wc_db.c
>>>
>>> Modified: trunk/subversion/libsvn_wc/wc_db.c
>>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/wc_db.c?pathrev=36592&r1=36591&r2=36592
>>> ==============================================================================
>>> --- trunk/subversion/libsvn_wc/wc_db.c  Mon Mar 16 07:34:22 2009        (r36591)
>>> +++ trunk/subversion/libsvn_wc/wc_db.c  Mon Mar 16 08:03:02 2009        (r36592)
>>> @@ -1330,8 +1330,6 @@ svn_wc__db_base_add_directory(svn_wc__db
>>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>>>   SVN_ERR_ASSERT(props != NULL);
>>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
>>> -  SVN_ERR_ASSERT(changed_date > 0);
>>> -  SVN_ERR_ASSERT(changed_author != NULL);
>>>   SVN_ERR_ASSERT(children != NULL);
>>>
>>>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>>> @@ -1394,8 +1392,6 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
>>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>>>   SVN_ERR_ASSERT(props != NULL);
>>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
>>> -  SVN_ERR_ASSERT(changed_date > 0);
>>> -  SVN_ERR_ASSERT(changed_author != NULL);
>>>   SVN_ERR_ASSERT(checksum != NULL);
>>>
>>>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>>> @@ -1457,8 +1453,6 @@ svn_wc__db_base_add_symlink(svn_wc__db_t
>>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>>>   SVN_ERR_ASSERT(props != NULL);
>>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
>>> -  SVN_ERR_ASSERT(changed_date > 0);
>>> -  SVN_ERR_ASSERT(changed_author != NULL);
>>>   SVN_ERR_ASSERT(target != NULL);
>>>
>>>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>>> @@ -1580,8 +1574,6 @@ svn_wc__db_temp_base_add_subdir(svn_wc__
>>>   SVN_ERR_ASSERT(repos_uuid != NULL);
>>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
>>> -  SVN_ERR_ASSERT(changed_date > 0);
>>> -  SVN_ERR_ASSERT(changed_author != NULL);
>>>
>>>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>>>                               svn_sqlite__mode_readwrite,
>>>
>>> ------------------------------------------------------
>>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1333819
>>>
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1336220
>>
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1337767


Re: svn commit: r36592 - trunk/subversion/libsvn_wc

Posted by Greg Stein <gs...@gmail.com>.
Eh? How would you test this?

On Tue, Mar 17, 2009 at 01:20, Daniel Rall <dl...@finemaltcoding.com> wrote:
> It'd be great if this change was accompanied by a unit test.
>
> On Mon, Mar 16, 2009 at 8:03 AM, Hyrum K. Wright <hy...@hyrumwright.org> wrote:
>> Author: hwright
>> Date: Mon Mar 16 08:03:02 2009
>> New Revision: 36592
>>
>> Log:
>> Relax a few assertions in the wc_db APIs.  It turns out that we can have
>> non-existent authors and changed dates, in the case of revprop-edited
>> revisions.
>>
>> * subversion/libsvn_wc/wc_db.c
>>  (svn_wc__db_base_add_directory, svn_wc__db_base_add_file,
>>   svn_wc__db_baes_add_symlink, svn_wc__db_base_add_subdir):
>>    Remove assertions on changed date and author.
>>
>> Modified:
>>   trunk/subversion/libsvn_wc/wc_db.c
>>
>> Modified: trunk/subversion/libsvn_wc/wc_db.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/wc_db.c?pathrev=36592&r1=36591&r2=36592
>> ==============================================================================
>> --- trunk/subversion/libsvn_wc/wc_db.c  Mon Mar 16 07:34:22 2009        (r36591)
>> +++ trunk/subversion/libsvn_wc/wc_db.c  Mon Mar 16 08:03:02 2009        (r36592)
>> @@ -1330,8 +1330,6 @@ svn_wc__db_base_add_directory(svn_wc__db
>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>>   SVN_ERR_ASSERT(props != NULL);
>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
>> -  SVN_ERR_ASSERT(changed_date > 0);
>> -  SVN_ERR_ASSERT(changed_author != NULL);
>>   SVN_ERR_ASSERT(children != NULL);
>>
>>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>> @@ -1394,8 +1392,6 @@ svn_wc__db_base_add_file(svn_wc__db_t *d
>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>>   SVN_ERR_ASSERT(props != NULL);
>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
>> -  SVN_ERR_ASSERT(changed_date > 0);
>> -  SVN_ERR_ASSERT(changed_author != NULL);
>>   SVN_ERR_ASSERT(checksum != NULL);
>>
>>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>> @@ -1457,8 +1453,6 @@ svn_wc__db_base_add_symlink(svn_wc__db_t
>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>>   SVN_ERR_ASSERT(props != NULL);
>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
>> -  SVN_ERR_ASSERT(changed_date > 0);
>> -  SVN_ERR_ASSERT(changed_author != NULL);
>>   SVN_ERR_ASSERT(target != NULL);
>>
>>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>> @@ -1580,8 +1574,6 @@ svn_wc__db_temp_base_add_subdir(svn_wc__
>>   SVN_ERR_ASSERT(repos_uuid != NULL);
>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
>>   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(changed_rev));
>> -  SVN_ERR_ASSERT(changed_date > 0);
>> -  SVN_ERR_ASSERT(changed_author != NULL);
>>
>>   SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
>>                               svn_sqlite__mode_readwrite,
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1333819
>>
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1336220
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1336502