Re: [Monotone-devel] heads up: file system changes

[Stephen Leake]

Zack Weinberg <address@hidden> writes:

> On Thu, Sep 24, 2009 at 1:12 AM, Stephen Leake
> <address@hidden> wrote:
>> Stephen Leake <address@hidden> writes:
>>> Zack Weinberg <address@hidden> writes:
>>>> Closely related question: what the hell is going on with
>>>> new_optimal_path?
>>>
>>> system_path constructors assume the input path is relative to the
>>> original process directory. But I needed a path that is relative to
>>> the workspace root, when reading from a stored conflicts file.
>
> Okay, but you could have added a system_path constructor that operated
> relative to the workspace root,  couldn't you have?

Yes, but that would not have solved the other problem.

>> I remembered the rest of the reason. If the user provides a relative
>> path in a conflict file, then I want to write out that same relative
>> path when I update the conflict file.
>
> Why?  

To be nice to the user; they wrote that part of the file, so don't
mess with it. Relative paths are easier to edit and read than
absolute.

> What's wrong with writing it back out as an absolute path? But you
> could, again, add an as_external_relative_to_ws_root() method to
> system_path.

Yes, but then I'd have to figure out when to call that, as opposed to
as_external(). So I'd have to store some metadata (bool relative) in
the conflict, and write code to check it everywhere.

>> So I need as_external() to dispatch on the path type when writing a
>> path to a conflict file.
>
> The thing is, the paths classes are really, really not supposed to be
> used with dynamic typing.  

Hmm. Then don't declare them as inherited classes? 

The dynamic typing solves my particular problem quite nicely, and in
my opinion is a perfect example of what dynamic typing is for.

> new_optimal_path introduces a hole through which horrible bugs could
> crawl (like, the conflicts file somehow getting added to a roster).

I don't follow this. Adding a file to a roster takes a deliberate
action; calling roster_t::attach_node (after creating a node_id for
it). How would merely using a dynamically typed path object cause
that?

I guess someone writing new mtn code could use new_optimal_path to
create a path object that was intended to be added to a roster, when
they should use a file_path constructor. Then at some point they call
roster_t::attach_node (nid, dynamic_cast<file_path> (path)) (note that
having to write that dynamic_cast should have been a red flag). And
then some user gives the new code a bookkeeping path, so
new_optimal_path silently creates a bookkeeping_path object. Then the
problem would be caught later by the dynamic_cast<file_path>.

The problem would be caught later than if the coder had used the right
constructor, but it would be caught. Reasonable testing would find the
problem, and the programmer would hopefully be inspired to use the
right constructor.

-- 
-- Stephe