On Tue 2003-03-25 at 12:34:53 +0100, Wolf Josef wrote:
> Benjamin Pflugmann wrote:
>
> > Ben, what are your thoughts on a changed plan? Has something like this
> > a better (well, realistic) chance to be accepted:
> >
> > A. (Might exist already) Store in the .svn admin area checksums of the
> > base copies.
> >
> > B. Assure that all places using the prestine base copy do something
> > sensible when it is missing (probably give a reasonable error) and
> > make a list of them for later.
> >
> > C1. Add a "--no-base-copy" option to "svn checkout".
> > C2. Add a "prepare-edit" command to "svn" that will copy the
> > working copy into the base copy (if the checksums match).
> > C3. Add some fetch-base command/option (could be combined with C2).
> > C4. Add a cleanup-base-copies command/option.
>
> Hmm, C4 would be nice but not a strict requirement.
Yes, never said so. But since C4 doesn't touch existing code, helps
debugging the stuff and migrating existing WCs, it is a nice-to-have
feature.
> > All that is needed to have a basic version running wouldn't require
> > big changes to existing code, as far as I can see. Mainly some
> > if-condition to "svn checkout" and some updates to the error messages
> > (to be sure they make sense in the new context).
> >
> > I hope others agree that this should be possible for 1.0. If the
> > fetch-bases command becomes "intelligent", it would be quite usable,
> > too. At least better than nothing. :-)
>
> How "intelligent" would fetch-bases need to be? Would it be enough to
> simply fetch the bases of the files that fail the checksum-check?
No, not really. But it would be good enough for a start.
> > After that works one could determine when to change the other points:
> >
> > D. Tweak libsvn_wc/diff.c:file_diff to fetch a pristine copy from the
> > server if a cached base version is not available. The downloaded
> > pristine copy should be cached.
> > E. Change the commit/merge to send the whole working copy instead
> > of fetching a base copy from the server and send back the diff,
> > if the .svn does not contain the base copy.
> > F. Whatever else needs tweaking for automagically working.
>
> If "fetch-bases" is intelligent (as you suggested above), then
> there is no more reason to tweak the diff or the commit/merge
> procedures.
There is. See below.
> AFAICS the diff on the text-base will be done only
> when svn_wc_text_modified_p() says to do so.
That is not really important for this discussion, but on
schedule == svn_wc_schedule_delete
the text base will be accessed without calling svn_wc_text_modified_p
before.
> Just let diff, commit, merge and all the other commands which could
> potentially need the text-bases call fetch-bases before calling
> file_diff.
Well, that is a tweak to all those commands, isn't it? And there are
cases, where your local copy is not modified, but you need a base. So
you either have to use the C2 or C3 method to generate it. And those
changes would go to existing code.
Basically you are back to the original proposal of implementing all at
once, only that you suggested that common code should go to a common
function, fetch-bases - something which goes without question.
> It would not even do a harm if every command (except fetch-bases, of
> course) would implicitly call fetch-bases. Umm, this way the
> user-visible fetch-bases command could die.
That is the long-term plan, and according to Ben opinion not suitable
for pre-1.0. And though mine has no effect, I agree.
> And very little existing code would be modified. Therefore the main
> reason for not putting such a change into 1.0 would disappear.
I think the conclusion is wrong. From what I have seen of the code,
this would be a much more than a very little change to libsvn_wc.
IMHO, the problem is that the WC library has to cover a lot of edge
cases and you would have to handle all those edge cases for the new
feature, too. I.e. a lot of small code changes with a lot of potential
to introduce new bugs.
The issue with svn_wc_schedule_delete above is just an example of such
a case.
> Hmm... If _every_ command would implicitly call fetch-bases then
> your step "B." would not be necessary anymore since the required
> base would already exist at the time the actual command would be
> run.
That would only be a variant of B: "Assure that all places using the
prestine base copy do something sensible when it is missing [...] and
make a list of them for later." The thing about error messages was
only a side-point.
I am not sure why you want to change step "B", what you want to
optimize. This is not about the amount of work, but about risk
management.
Assuring that nothing crashes when the text base is missing (it
already shouldn't, but if you remove text-bases by intent, you have to
be sure) and making a list of affected places that you can present to
peer review is certainly a step to reduce risk. I do not see what is
gained if that step is left out.
> So the six-step-plan might look like this:
>
> A. Store checksums (might exist already) (_does_ this already exist?
> I don't think so)
> B. modify svn_wc_text_modified_p() to use checksums if the text_base
> is missing.
> C. Add an "intelligent" fetch_bases() library call that fetches all
> the bases of the files that fail the checksum test. Fetch_bases()
> could ask svn_wc_text_modified_p() to determinde which files to
> fetch.
> C(2). Let every command that potentially could need a text-base call
> fetch_bases() before doing the real work.
> D. Add --no-base-copy
> F. (optionally) add prepare-edit and/or cleanup-base-copy
>
> Doing the change this way, very little modifications to existing code
> would be necessary.
C(2) is the problem. AFAICT, it needs a lot of little changes to
existing code. As I said, you are basically back to the original
suggestion. You only made clear that the fetch-the-base stuff should
go into an own function.
Besides, I do not see the advantage of your solution against mine
regarding risk. If you go with mine, i.e. make the fetch_bases
function and the command for it, you can easily have your C(2) at a
later point (that was what my suggestion was all about, even if it did
not come accross as that). Implement the low-risk stuff and when it's
done start thinking whether C(2) can go pre-1.0 or not.
Regards,
Benjamin.
- application/pgp-signature attachment: stored
Received on Tue Mar 25 19:15:45 2003