You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sean Farrell <sf...@wv.mentorg.com> on 2012/10/24 21:28:24 UTC
Issue with Revision 955787
I'm seeing odd behavior when using svn_client_status2 with version 1.7.7.
1. I have two checkouts of a single directory, both fully up-to-date.
2. In the one checkout I add/commit a new file within the directory.
3. From the other checkout I call "svn_client_status2" on the
directory and see the following status structure
("svn_wc_status2_t") filled with:
text_status = svn_wc_status_modified
prop_status = svn_wc_status_normal
repo_text_status = svn_wc_status_modified
repo_prop_status = svn_wc_status_none
pristine_text_status = svn_wc_status_normal
pristine_ prop_status = svn_wc_status_normal
I expect the "repo_text_status" to be modified as the directory now
contains the new file in the repository.
However, locally, the "text_status" shows "svn_wc_status_modified" when
I think it should show "svn_wc_status_normal".
When debugging this I see that "svn_wc__status2_from_3" is called prior
to my "svn_wc_status_func2_t" callback being called. The
"svn_wc_status3_t" structure being converted is filled with:
node_status = svn_wc_status_normal
text_status = svn_wc_status_normal
prop_status = svn_wc_status_normal
repo_node_status = svn_wc_status_modified
repo_text_status = svn_wc_status_modified
repo_prop_status = svn_wc_status_none
Here the "text_status" is "svn_wc_status_normal" as I expect. So
somewhere within "svn_wc__status2_from_3" the conversion is messing up.
Looking at this function in more detail shows what appears to be the
problem:
svn_error_t *
svn_wc__status2_from_3(svn_wc_status2_t **status,
const svn_wc_status3_t *old_status,
svn_wc_context_t *wc_ctx,
const char *local_abspath,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
...
(*status)->text_status = old_status->node_status;
(*status)->prop_status = old_status->prop_status;
(*status)->repos_text_status = old_status->repos_node_status;
(*status)->repos_prop_status = old_status->repos_prop_status;
/* Some values might be inherited from properties */
if (old_status->node_status == svn_wc_status_modified
|| old_status->node_status == svn_wc_status_conflicted)
(*status)->text_status = old_status->text_status;
/* (Currently a no-op, but just make sure it is ok) */
if (old_status->repos_node_status == svn_wc_status_modified
|| old_status->repos_node_status == svn_wc_status_conflicted)
(*status)->text_status = old_status->repos_text_status;
...
}
As you can see "text_status" and "repos_text_status" follow similar
logic, however the last line appears to be a copy & paste bug, where
"text_status" is used it should instead be "repos_text_status". This
explains why the "svn_wc_status2_t->text_status" has the value
"svn_wc_status_modified" because it's being set to the value of
"svn_wc_status3_t->repos_text_status".
This section of code was committed with revision 955787 by rhuijben.
Can anyone confirm my findings are correct, or wrong?
Thank you,
Sean
Re: Issue with Revision 955787
Posted by Philip Martin <ph...@wandisco.com>.
Sean Farrell <sf...@wv.mentorg.com> writes:
> Will this fix go out in a 1.7.8 and/or 1.8?
I've committed it to trunk (1.8) and proposed it for 1.7.
> Also, I'm trying to track down what releases have this bug. It looks
> like starting with 1.6.12 on up, which includes all of 1.7.x
> (i.e. 1.7.0 - 1.7.7). Is that correct?
Every 1.7 release so far has the bug. It's in the 1.6 compatibility
code that is new in 1.7 and so doesn't affect 1.6 releases.
--
Join us this October at Subversion Live 2012
http://www.wandisco.com/svn-live-2012
Re: Issue with Revision 955787
Posted by Sean Farrell <sf...@wv.mentorg.com>.
I've verified the fix. The "text_status" for the "svn_wc_status2_t"
object now has the value of "svn_wc_status_normal".
Will this fix go out in a 1.7.8 and/or 1.8?
Also, I'm trying to track down what releases have this bug. It looks
like starting with 1.6.12 on up, which includes all of 1.7.x (i.e. 1.7.0
- 1.7.7). Is that correct?
Thanks,
Sean
Philip Martin wrote:
> Sean Farrell <sf...@wv.mentorg.com> writes:
>
>
>> (*status)->text_status = old_status->node_status;
>> (*status)->prop_status = old_status->prop_status;
>>
>> (*status)->repos_text_status = old_status->repos_node_status;
>> (*status)->repos_prop_status = old_status->repos_prop_status;
>>
>> /* Some values might be inherited from properties */
>> if (old_status->node_status == svn_wc_status_modified
>> || old_status->node_status == svn_wc_status_conflicted)
>> (*status)->text_status = old_status->text_status;
>>
>> /* (Currently a no-op, but just make sure it is ok) */
>> if (old_status->repos_node_status == svn_wc_status_modified
>> || old_status->repos_node_status == svn_wc_status_conflicted)
>> (*status)->text_status = old_status->repos_text_status;
>> ...
>> }
>>
>> As you can see "text_status" and "repos_text_status" follow similar logic, however the last line appears to be a copy & paste bug, where "text_status" is used it should instead be "repos_text_status". This explains why the "svn_wc_status2_t->text_status" has the value "svn_wc_status_modified" because it's being set to the value of "svn_wc_status3_t->repos_text_status".
>>
>> This section of code was committed with revision 955787 by rhuijben.
>>
>> Can anyone confirm my findings are correct, or wrong?
>>
>
> I think you are correct. Can you confirm that this is the correct fix:
>
> Index: ../src/subversion/libsvn_wc/util.c
> ===================================================================
> --- ../src/subversion/libsvn_wc/util.c (revision 1402519)
> +++ ../src/subversion/libsvn_wc/util.c (working copy)
> @@ -469,7 +469,7 @@
> /* (Currently a no-op, but just make sure it is ok) */
> if (old_status->repos_node_status == svn_wc_status_modified
> || old_status->repos_node_status == svn_wc_status_conflicted)
> - (*status)->text_status = old_status->repos_text_status;
> + (*status)->repos_text_status = old_status->repos_text_status;
>
> if (old_status->node_status == svn_wc_status_added)
> (*status)->prop_status = svn_wc_status_none; /* No separate info */
>
>
>
Re: Issue with Revision 955787
Posted by Philip Martin <ph...@wandisco.com>.
Sean Farrell <sf...@wv.mentorg.com> writes:
> (*status)->text_status = old_status->node_status;
> (*status)->prop_status = old_status->prop_status;
>
> (*status)->repos_text_status = old_status->repos_node_status;
> (*status)->repos_prop_status = old_status->repos_prop_status;
>
> /* Some values might be inherited from properties */
> if (old_status->node_status == svn_wc_status_modified
> || old_status->node_status == svn_wc_status_conflicted)
> (*status)->text_status = old_status->text_status;
>
> /* (Currently a no-op, but just make sure it is ok) */
> if (old_status->repos_node_status == svn_wc_status_modified
> || old_status->repos_node_status == svn_wc_status_conflicted)
> (*status)->text_status = old_status->repos_text_status;
> ...
> }
>
> As you can see "text_status" and "repos_text_status" follow similar logic, however the last line appears to be a copy & paste bug, where "text_status" is used it should instead be "repos_text_status". This explains why the "svn_wc_status2_t->text_status" has the value "svn_wc_status_modified" because it's being set to the value of "svn_wc_status3_t->repos_text_status".
>
> This section of code was committed with revision 955787 by rhuijben.
>
> Can anyone confirm my findings are correct, or wrong?
I think you are correct. Can you confirm that this is the correct fix:
Index: ../src/subversion/libsvn_wc/util.c
===================================================================
--- ../src/subversion/libsvn_wc/util.c (revision 1402519)
+++ ../src/subversion/libsvn_wc/util.c (working copy)
@@ -469,7 +469,7 @@
/* (Currently a no-op, but just make sure it is ok) */
if (old_status->repos_node_status == svn_wc_status_modified
|| old_status->repos_node_status == svn_wc_status_conflicted)
- (*status)->text_status = old_status->repos_text_status;
+ (*status)->repos_text_status = old_status->repos_text_status;
if (old_status->node_status == svn_wc_status_added)
(*status)->prop_status = svn_wc_status_none; /* No separate info */
--
Join us this October at Subversion Live 2012
http://www.wandisco.com/svn-live-2012