[Greg, resending a reply, since I forgot to CC the list last time.]
Greg Stein <gstein@lyra.org> writes:
> There is still an outstanding issue on how to fetch local properties during
> a commit crawl.
>
> The heart of the problem is coupling the RA commit editor with the WC. If
> the RA "knows" the commit editor is driven by the WC, and uses code based on
> that fact, then we cannot run the RA editor from other drivers.
>
> The problem and my proposed solution is at:
> http://subversion.tigris.org/subversion-dev/current/msg00789.html
I remember that message, yeah (have reproduced it at the end of this
message, for others' convenience). My earlier response may have been
off the mark; let me ask what I should have asked before...
What do you need these properties for? The reason I ask is that it
seems a slippery slope for the network layer to be storing
implementation-specific properties in the WC. Technically, all the
information you need should be there already -- that is, information
that is "theoretically" necessary, not just needed by one particular
implementation of a networking layer. (I'm not saying anyone will
ever implement any other network protocol, just that these separations
are worth maintaining simply for our own sanity.)
The reasons you gave were:
> Oh, and some background. WHY does the commit editor need to fetch these
> props? Well, there are two properties of interest:
>
> 1) where (on the server) to create a DeltaV activity. the activity is the
> visible manifestation of the FS transaction.
>
> 2) for each resource on the client, what is the corresponding version
> resource on the server. i.e. the path/revision pair that was checked out.
>
> The first is needed at the beginning of the commit to establish an FS
> transaction for the change. The second is needed for each resource to be
> committed -- we need to do a CHECKOUT of the version resource. This roughly
> corresponds to cloning the path/revision in the FS, which then allows us to
> begin modifying it [within a transaction context].
Why should the working copy know or care about (1)? It's a DAV/DeltaV
thing -- the network layer can choose any activity name it wants,
since only the network layer should ever see it, right?
Similarly for (2). The WC should only store Subversion's idea of
ancestry information; from that, shouldn't it be possible to convert
to or deduce anything else?
We'll have an infinity of other such questions if the network layer's
fingers start reaching down into the WC. (I'm sorry, my alarm bells
should have gone off earlier about this one.)
OR: possibly I'm misunderstanding the question here, and the real
issue is the one stated at the beginning of your mail, about getting a
root_path(), but I think Ben and I both followed-up to that, answering
that you only call replace_root() once per commit, and have to
compute the deepest common ancestor dir of the entities involved in
the commit before you call get_commit_editor().
Let me know if I'm missing something here (or call, Greg, if that's
easier -- sometimes I can blunder about in emails, missing the point,
until someone says it to me verbally :-) ).
Best,
-K
> During the walk for the commit, I need the PATH for the "current" node so
> that I can pass it to svn_wc_prop_*(). But the best that I can come up with
> is to use "." at the start of the walk, and then to append paths on the way
> up/down.
>
> The problem with this, though is when somebody does:
>
> $ svn commit some/dir/way/down
>
> The current directory will not match the root of the commit walk, so the "."
> will not work properly. I think the proper answer is to pass a path into
> the replace_root() editor function.
>
> We can't really do it as part of the get_commit_editor() because a single
> editor may be used multiple times (e.g. replace_root called many times) with
> different roots.
>
> Another acceptable option would be to pass a "handle" into the replace_root,
> add/replace_dir, and add/replace_file callbacks. The editor could then pass
> this handle back to the WC to fetch information. For example:
>
> replace_root(edit_baton, root_handle, &root_baton)
>
> add_directory(name, parent_baton, parent_handle, ancestor_path,
> ancestor_revision, &child_baton)
> etc.
>
> svn_wc_prop_get(&value, propname, file_handle)
>
> [ toss the pool cuz it can live inside file_handle ]
>
> Hrm. I'm not sure that I like this because it means that the commit editor
> would "know" that the handle belongs to the WC. i.e. it loses its
> interchangeable nature. To some extent, passing a raw path is the same thing
> -- how does the commit editor "know" that the passed-in path is intended to
> be passed to WC functions? If it assumes that, then the commit editor is now
> tied to being called by the WC. [not a bad assumption, but it increases the
> coupling, which the editor interface was intended to kill]
>
> The "real" solution would be:
>
> svn_resource_type_t
> {
> svn_string_t * get_prop(svn_resource_t *res,
> svn_string_t *name);
>
> void set_prop(svn_resource_t *res,
> svn_string_t *name, svn_string_t *value);
>
> /* ### anything else? */
> }
>
> svn_resource_t
> {
> const svn_resource_type_t *type;
> void *resource_baton;
> }
>
> A pointer to an svn_resource_t would be passed to the relevant editor
> functions (replace_root, add/replace file/dir). The editor can then call
> back to the driver to query information about the resource.
>
> [ I put set_prop in there, but I only need get_prop; we could possibly limit
> the callbacks to readonly (e.g. get_prop) type functions. note that I
> separated out the vtable from the resource itself. look at the "type" as
> the class, and the resource as the specific instance. ]
>
> It should also be legal for editor drivers to pass NULL, meaning they don't
> support callbacks for extended information. Yes, this means the commit
> editor wouldn't work with editors that don't supply this, but that's okay...
> we've removed the hard WC dependence and provided a way for other editors to
> be able to satisfy the commit editor's needs.
>
>
> Oh, and some background. WHY does the commit editor need to fetch these
> props? Well, there are two properties of interest:
>
> 1) where (on the server) to create a DeltaV activity. the activity is the
> visible manifestation of the FS transaction.
>
> 2) for each resource on the client, what is the corresponding version
> resource on the server. i.e. the path/revision pair that was checked out.
>
> The first is needed at the beginning of the commit to establish an FS
> transaction for the change. The second is needed for each resource to be
> committed -- we need to do a CHECKOUT of the version resource. This roughly
> corresponds to cloning the path/revision in the FS, which then allows us to
> begin modifying it [within a transaction context].
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
Received on Sat Oct 21 14:36:18 2006