`max_diff_files` and `max_diff_bytes` do not handle modified moved/copied files

[Matt Shin]

When we tried to view a very large Subversion changeset where almost all files were renamed and then modified, we discovered that the max_diff_files and max_diff_bytes configurations had no effect what-so-ever.

It appears that the code that counts the number of diff files in trunk/trac/versioncontrol/web_ui/changeset.py@4124#L444​ relies on the the logic change == Changeset.EDIT. This means that moved or copied files are automatically discarded - and so later on in the code, it will try to display the diff any way.

To fix it, I guess the code will have to compare any file nodes with change == Changeset.MOVE or change == Changeset.COPY while it is counting the number of diff files, and only discarding those without modifications.

[Christian Boos]

Fixed in r4131.

---

Some additional explanations about the code, in case you're wondering why things are done the way they are:

We can't distinguish precisely which of the COPY or MOVE are done with modifications from those that aren't, without actually doing the comparison.

So what we're actually trying to estimate is the global quantity of data that will eventually be processed when we try to produce the diffs. For example, you copied a 1Mb file without modifying it. If that's below the threshold, then we'll compare the old and new files, trying to produce the diff. So 2Mb of data will be processed and that's why we have to consider them in the estimate.

It could be interesting to detect and record Copy+Modification and Rename+Modification as such in the future, as Subversion (at least) can make the difference. For backends that can't, then always using Copy+Mod and Rename+Mod would be equivalent to the current behavior.

[Matt Shin]

Thanks for fixing the problem so quickly.

Yes, I can understand the difficulty in trying to distinguish between copy and copy+modify, and between move and move+modify. Although your fix may overkill a situation, I agree it is at least the safer and more efficient behaviour.

[Matt Shin]

Any chance of this going into 0.10.1 as well?

[Christian Boos]

Ok, done in r4144. Thanks to svnmerge, this sort of things is getting cheaper ;)

[Matt Shin]

Sorry for being a pain, but I have had a go testing your change, and it appears to have broken somewhat. When I am viewing a changeset with lots of move+edit files, it no longer displays links to "view diffs" for these files.

I do not understand the logic well enough to send you a proper patch, but it appears that the problem can be fixed if you also modify the line trunk/trac/versioncontrol/web_ui/changeset.py@4143#L484​ to:

                if change in Changeset.DIFF_CHANGES and not show_diffs:

However, I am not sure whether it will break other situations or whether you need to modify trunk/trac/versioncontrol/web_ui/changeset.py@4143#L465​ as well.

[Christian Boos]

You're right, I'll fix that.

Thinking about the restriction mechanism a bit further, it would be nice if it selectively enabled the "small" files to have their diff displayed in line, and only add links to restricted changesets for the "big" files.

[Christian Boos]

Ok, did the fix for 0.10-stable in r4149.

For trunk, I have a more complete fix pending, implementing the mechanism described above.