[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: CVS update: subversion/subversion/tests-common svn_test_editor.c

From: Greg Hudson <ghudson_at_MIT.EDU>
Date: 2001-01-27 18:35:13 CET

> It was a compromise: given that we wanted to have a nested tree
> delta, and yet defer the sending of txdeltas until the end, then
> "closing" a directory doesn't really mean that everything in that
> directory is done yet.

I can certainly see needing to ref-count directories when you want to
know that a particular directory is done, but it's unpleasant to have
to do that for the edit as a whole.

> By the way, doing things this way *was* actually part of the design
> assumptions of the editor, but this particular assumption was never
> very well publicized. The following comment, which has been in
> svn_delta.h for some time, pretty much sums it up:

I wrote that comment. (Not the original version, but the stuff you
quoted, certainly.)

> ...I also think that not having close_edit() makes more sense. I
> mean, what if one *hadn't* yet balanced opens with closes, but
> called close_edit() anyway? What would it mean?

It wouldn't mean anything, just like a hundred other possible invalid
call sequences to the editor structure (such as closing a file while
you have a directory open other than the directory the file lives in).
If we want the editor interface to enforce its semantic constraints,
then we have to start over from scratch. I don't see why Greg Stein's
concern that "the caller could erroneously invoke replace_root twice"
means the editor structure has to be changed, but my earlier concern
that "the caller could do any number of things out of order" didn't
mean the editor structure had to be changed. See
http://subversion.tigris.org/subversion-dev/8/msg00177.html for when I
brought this up back in October.

I didn't notice that double standard when change #1 was proposed. Now
that I see the collateral damage change #1 did (by making it
impossible for an editor implementation to clean up resources after
the edit without extra bookkeeping), I see the double standard and I
question the wisdom of change #1.

I grant that if almost every editor needs to track when a directory is
finished, then tracking when the root directory is finished isn't
extra bookkeeping. But I tend to think that the wc update editor is
fairly unique in this regard; and in fact, I don't think there's even
an instrinsic need in the wc update editor. All that stuff could be
done in one fell swoop at the end of the edit without consuming any
additional resources, if postfix text deltas are being used.

I'll restate my opinion on the editor interface:

        * I still think we should get rid of directory and file
          batons, just as I did back in October. Relative to a single
          edit baton (plus explicit one-time-use batons for deferred
          text deltas), they invite more type-safety issues, create
          all sorts of invalid uses of the editor, and add complexity.
          All they give you in return is the ability (which we don't
          actually take advantage of) to notice quickly when a caller
          forgets to close a directory. Plus a hedge in case we ever
          decide to allow non-hierarchial editing, but I don't think
          that kind of hedge is valuable.

        * In the presence of the above change, I think the editor
          interface should look more like the byte stream interface,
          with the editor baton stored in the structure and callers
          using wrapper functions; for example:

                err = svn_edit_replace_directory(editor, "foodir");
                ...
                err = svn_edit_close_directory(editor);

          instead of edit_fns->replace_directory(baton, "foodir") like
          we have now.

        * In the absence of the above changes, I think there needs to
          be an explicit call when the edit is over. If that means
          restoring the editor baton and making get-editor return
          three pieces of information (edit_fns, editor baton, root
          directory baton), so be it.
Received on Sat Oct 21 14:36:20 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.