You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/09/02 17:51:17 UTC

Re: Useless explicit mergeinfo records

"Peter Wemm" <pe...@wemm.org> writes:
> On Fri, Aug 29, 2008 at 2:22 AM, Vincent Lefevre <vi...@vinc17.org> wrote:
>> On 2008-08-27 17:00:26 +0400, Danil Shopyrin wrote:
>>> 1) Merge touches all files with explicit mergeinfo even if content of
>>> these files isn't touched by merged revisions. This can result in
>>> inability to commit changes after merge.
>>
>> Yes, I noted this problem in the users ML:
>>
>>  "svn merge" scans the whole wc and deletes svn:mergeinfo properties
>>
>> This is really annoying. Even if there are technical reasons, this
>> should be regarded as a bug IMHO.
>
> Not only that, but it is horribly slow due to the wc.  20-30 minutes
> to do a merge operation really messed with our plans.

I agree, and it seems like a pretty basic bug.  Can you file an issue
about it, linking to this thread (and the one you mention above)?

Danil, a workaround for this problem...

> When user merges any change from a branch to the trunk, mergenfo for
> $/trunk/core/corefile.txt will be changed (even if corefile.txt isn't
> somehow touched by this change). And if user doesn't have write access
> to the core folder then he will not be able to commit this change.

... is to simply exclude the authz-blocked path from the commit, by
naming commit targets explicitly.  That's cumbersome, but it can be
done until this situation is fixed.

By the way, I also looked in the issue tracker for an issue about this,
and didn't see any.  FWIW, I specifically checked these:

   http://subversion.tigris.org/issues/show_bug.cgi?id=3056
   http://subversion.tigris.org/issues/show_bug.cgi?id=2838
   http://subversion.tigris.org/issues/show_bug.cgi?id=2898
   http://subversion.tigris.org/issues/show_bug.cgi?id=2970
   http://subversion.tigris.org/issues/show_bug.cgi?id=2833
   http://subversion.tigris.org/issues/show_bug.cgi?id=3067

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Karl Fogel <kf...@red-bean.com>.
"Mark Phippard" <ma...@gmail.com> writes:
> On Wed, Sep 3, 2008 at 2:22 PM, Danil Shopyrin <da...@visualsvn.com> wrote:
>>> However, Danil (or anyone else), if you have any ideas for how to fix
>>> this in the current 1.5 working copy or client code, please say them.
>>
>> Currently I'm working on set of patches for 1.5.x. I'm not very
>> familiar with internal Subversion code base :) That's why it's not
>> very fast process.
>
> Great.  It might be worthwhile to present your ideas and design before
> investing too much work though.  And of course all patches must be
> against trunk and there is no guarantee they will be backported for
> 1.5.x.

+1  We're happy to answer questions...

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Mark Phippard <ma...@gmail.com>.
On Wed, Sep 3, 2008 at 2:22 PM, Danil Shopyrin <da...@visualsvn.com> wrote:
>> However, Danil (or anyone else), if you have any ideas for how to fix
>> this in the current 1.5 working copy or client code, please say them.
>
> Currently I'm working on set of patches for 1.5.x. I'm not very
> familiar with internal Subversion code base :) That's why it's not
> very fast process.

Great.  It might be worthwhile to present your ideas and design before
investing too much work though.  And of course all patches must be
against trunk and there is no guarantee they will be backported for
1.5.x.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Danil Shopyrin <da...@visualsvn.com>.
> However, Danil (or anyone else), if you have any ideas for how to fix
> this in the current 1.5 working copy or client code, please say them.

Currently I'm working on set of patches for 1.5.x. I'm not very
familiar with internal Subversion code base :) That's why it's not
very fast process.

--
Danil

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Karl Fogel <kf...@red-bean.com>.
"Mark Phippard" <ma...@gmail.com> writes:
> Given that you make a product that profits from SVN, then I look
> forward to seeing you more active in the mail list and to receiving
> your patches to help us make it more usable.  We are always looking
> for new contributors and it would be great to have you on board.

Well, to be fair, Danil has been more and more active lately -- the
bottleneck has more been our bandwidth to review patches!

> I think this problem is going to be tough for you to solve.  I do not
> know if the client has a way of checking whether it can write to a
> patch in the repos, short of actually trying to do it.  You could
> argue that we ought to not allow a user to add/delete and edit files
> in this folder either.  And the fact that we do is also a bug.  So I
> am guessing we will need new API to ask the server this information
> and then put it in the appropriate places.  Of course if each time we
> update mergeinfo we have to make a server call to see if the client
> can write to that location, then that is not going to help performance
> either.  Anyway, feel free to start a new thread to discuss your
> proposals for solving this problem.

I think the real solution to this has to come with the new WC, which
will be able to do real property inheritance -- that is, if the
properties explicitly attached to path CHILD don't change, then even if
CHILD is authz-protected, we can still affect its properties by changing
the properties of one of its parents.

However, Danil (or anyone else), if you have any ideas for how to fix
this in the current 1.5 working copy or client code, please say them.

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Mark Phippard <ma...@gmail.com>.
On Wed, Sep 3, 2008 at 5:08 AM, Danil Shopyrin <da...@visualsvn.com> wrote:
>> How could merge tracking be merge tracking if it does not do this?  It
>> has to scan the working copy to find out if you have any subtree
>> mergeinfo.
>
> I don't agree. The authz-blocked path isn't somehow touched in the
> merging branch and it's surprising behavior when the user-visible
> properties of the file are changed. And it's unacceptable that commit
> is blocked because of this.

As I said to Karl, I was not responding to this part of the thread (I
forgot about it).  I was talking about the performance hit of crawling
the WC, and just saying there is no way to avoid that.

>> I do not think it makes sense to try and change this.  If we really
>> implement a new WC, this checking will become trivial as it will no
>> longer be crawling the WC to find this information.  That seems like a
>> better solution given that the code is working as intended.
>
> From my point of view, the whole Subversion usability is seriously
> damaged since merge tracking was released in 1.5. I work with
> Subversion for several years and I was really disappointed by released
> behavior (and most of my friends too). Looks like a big broken window.
>
> That's why I think it's a good job to make merge tracking better in 1.5.x.
>
> --
> With best regards,
> Danil Shopyrin
> VisualSVN Team

Given that you make a product that profits from SVN, then I look
forward to seeing you more active in the mail list and to receiving
your patches to help us make it more usable.  We are always looking
for new contributors and it would be great to have you on board.

I think this problem is going to be tough for you to solve.  I do not
know if the client has a way of checking whether it can write to a
patch in the repos, short of actually trying to do it.  You could
argue that we ought to not allow a user to add/delete and edit files
in this folder either.  And the fact that we do is also a bug.  So I
am guessing we will need new API to ask the server this information
and then put it in the appropriate places.  Of course if each time we
update mergeinfo we have to make a server call to see if the client
can write to that location, then that is not going to help performance
either.  Anyway, feel free to start a new thread to discuss your
proposals for solving this problem.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Danil Shopyrin <da...@visualsvn.com>.
> How could merge tracking be merge tracking if it does not do this?  It
> has to scan the working copy to find out if you have any subtree
> mergeinfo.

I don't agree. The authz-blocked path isn't somehow touched in the
merging branch and it's surprising behavior when the user-visible
properties of the file are changed. And it's unacceptable that commit
is blocked because of this.

As stated in $/www/merge-tracking/summit.html:
[[
...Users are often very average developers/maintenance programmers.
]]
(I agree with that statement). The discussed behavior isn't a critical
problem for an experienced developer but it can be critical for a
junior. And merge tracking supposed to simplify things for juniors.

> I do not think it makes sense to try and change this.  If we really
> implement a new WC, this checking will become trivial as it will no
> longer be crawling the WC to find this information.  That seems like a
> better solution given that the code is working as intended.

Re: Useless explicit mergeinfo records

Posted by Karl Fogel <kf...@red-bean.com>.
"Mark Phippard" <ma...@gmail.com> writes:
> All that said, I thought your reponse was saying crawling the WC is a
> bug and I was responding to that specifically.

The only things that are ever bugs are user-visible behaviors.
Everything else is an implementation choice :-).

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Sep 2, 2008 at 1:57 PM, Karl Fogel <kf...@red-bean.com> wrote:
> "Mark Phippard" <ma...@gmail.com> writes:
>>>> Not only that, but it is horribly slow due to the wc.  20-30 minutes
>>>> to do a merge operation really messed with our plans.
>>>
>>> I agree, and it seems like a pretty basic bug.  Can you file an issue
>>> about it, linking to this thread (and the one you mention above)?
>>
>> How could merge tracking be merge tracking if it does not do this?  It
>> has to scan the working copy to find out if you have any subtree
>> mergeinfo.
>>
>> I do not think it makes sense to try and change this.  If we really
>> implement a new WC, this checking will become trivial as it will no
>> longer be crawling the WC to find this information.  That seems like a
>> better solution given that the code is working as intended.
>
> We have a real problem: metadata *about* files is being treated (for
> authz purposes) as data *in* files, simply because we implemented the
> mergeinfo as a property.
>
> Thus a merge which does not actually change authz-protected files cannot
> be committed, because we have no way of committing statements about
> those files without committing changes to the files themselves.
>
> The new WC will make things easier; it may open the door to totally
> changing how we record mergeinfo (since, among other things, it will
> make property inheritance much easier).
>
> Unrelatedly, the slowness is still a bug, of course.

So I think the answer is to look for a way to implement this feature
that does not use properties.  In this specific case, the user will
have to omit the item from the commit.

All that said, I thought your reponse was saying crawling the WC is a
bug and I was responding to that specifically.



-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Karl Fogel <kf...@red-bean.com>.
"Mark Phippard" <ma...@gmail.com> writes:
>>> Not only that, but it is horribly slow due to the wc.  20-30 minutes
>>> to do a merge operation really messed with our plans.
>>
>> I agree, and it seems like a pretty basic bug.  Can you file an issue
>> about it, linking to this thread (and the one you mention above)?
>
> How could merge tracking be merge tracking if it does not do this?  It
> has to scan the working copy to find out if you have any subtree
> mergeinfo.
>
> I do not think it makes sense to try and change this.  If we really
> implement a new WC, this checking will become trivial as it will no
> longer be crawling the WC to find this information.  That seems like a
> better solution given that the code is working as intended.

We have a real problem: metadata *about* files is being treated (for
authz purposes) as data *in* files, simply because we implemented the
mergeinfo as a property.

Thus a merge which does not actually change authz-protected files cannot
be committed, because we have no way of committing statements about
those files without committing changes to the files themselves.

The new WC will make things easier; it may open the door to totally
changing how we record mergeinfo (since, among other things, it will
make property inheritance much easier).

Unrelatedly, the slowness is still a bug, of course.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Useless explicit mergeinfo records

Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Sep 2, 2008 at 10:51 AM, Karl Fogel <kf...@red-bean.com> wrote:
> "Peter Wemm" <pe...@wemm.org> writes:
>> On Fri, Aug 29, 2008 at 2:22 AM, Vincent Lefevre <vi...@vinc17.org> wrote:
>>> On 2008-08-27 17:00:26 +0400, Danil Shopyrin wrote:
>>>> 1) Merge touches all files with explicit mergeinfo even if content of
>>>> these files isn't touched by merged revisions. This can result in
>>>> inability to commit changes after merge.
>>>
>>> Yes, I noted this problem in the users ML:
>>>
>>>  "svn merge" scans the whole wc and deletes svn:mergeinfo properties
>>>
>>> This is really annoying. Even if there are technical reasons, this
>>> should be regarded as a bug IMHO.
>>
>> Not only that, but it is horribly slow due to the wc.  20-30 minutes
>> to do a merge operation really messed with our plans.
>
> I agree, and it seems like a pretty basic bug.  Can you file an issue
> about it, linking to this thread (and the one you mention above)?

How could merge tracking be merge tracking if it does not do this?  It
has to scan the working copy to find out if you have any subtree
mergeinfo.

I do not think it makes sense to try and change this.  If we really
implement a new WC, this checking will become trivial as it will no
longer be crawling the WC to find this information.  That seems like a
better solution given that the code is working as intended.


-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org