Comment 5 for bug 415508

Revision history for this message
Robert Collins (lifeless) wrote : Re: [Bug 415508] Re: Content filtering breaks commit w/ merge parents

On Tue, 2009-08-18 at 21:07 +0000, John A Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Robert Collins wrote:
> > On Tue, 2009-08-18 at 17:53 +0000, John A Meinel wrote:
> >> I expect performance will suck a little bit, as we'll be re-reading
> >> all
> >> the files that have a content filter. But at least it won't cause
> >> bogus
> >> data to be inserted into the repository on every merge commit.
> >
> > This would appear to still leave an api in place that isn't content
> > filter aware.
> >
> > I appreciate that we need to fix this promptly, but it doesn't seem
> > complete without fixing / deprecating the get_content_summary api; and
> > if we fix it commit doesn't need to change.
> >
> > -Rob
> >
>
> So the thing is we can't know the size of the file without either
> caching it into the dirstate or running the content filters.
>
> With this patch, I believe it will fall down to the next check, which is
> checking if the cached sha1 matches the sha1 of the only parent text,
> and thus not require us to read the file at all. (So my earlier
> statement about it re-reading all texts would be false.)
>
> If you want to just deprecate the non 'record-iter-changes' path, that
> is reasonable. I'm not sure the specific effect on commit, etc.

All I'm saying is that stopping-the-use-of-a-bad-API is not sufficient.
The /cause/ of the bug remains, and that API should be
deprecated/deleted/made to raise when content filters are in use, or
something along those lines.