Stefan Sperling wrote on Sat, 28 Mar 2020 11:32 +0100: > On Fri, Mar 27, 2020 at 10:58:48PM +0000, Daniel Shahaf wrote: > > Stefan Sperling wrote on Fri, 27 Mar 2020 14:41 +0100: > > > On Thu, Mar 26, 2020 at 08:32:38PM +0000, Daniel Shahaf wrote: > > > > What I'm worried about is that we'll apply the "backwards compatibility > > > > until 2.0" promise to something we shouldn't. As I said, issues of > > > > this nature can't generally be found by tests; they can only be found > > > > by code review. > > > > > > I'm not worried about that. After reading through the patch I don't see > > > any reason why we would not want to commit to supporting this feature. > > > > > > > I wasn't discussing the build-repcache patch specifically; I was > > commenting in general terms about the risks of rushing patch review > > processes lest the patches in question miss a minor release train. > > "We'll just fix it in a patch release" is the right attitude, but > > doesn't always apply. > > Ah, I see. That wasn't clear to me.
Well, I'm glad there is still one mailing list where people can sort out disagreements without resorting to pyromania. :-) > > Well, I guess the documentation, once written, should point out that if > > any single revision build-repcache operates on adds a large number of > > reps, commits made in parallel to the build-repcache run will be starved > > out of adding their own rep-cache entries, which will manifest as > > "post-commit FS processing failed". (We have a bug filed for this, but > > I can't find its number right now, sorry.) > > Good point. > > Maybe the command's help output should mention that the repository will > be blocked for commits while this command is running? Even if that isn't > entirely true at the implementation level it would be good user-facing > advice. I don't think inaccuracies in the documentation are a good idea. The proposal might discourage people from running this when they could; might lead people to run this in order to trigger the side effect of blocking commits for a short while, only to find commits did happen successfully; and might get people to distrust our documentation in general. Therefore, I'd document the semantics accurately. If need be, we can refer to fuller documentation in the release notes and/or the book. > > > I know of at least two cases where rep-sharing was temporarily or perhaps > > > even permanently disabled after SHA1 collisions were committed (one case > > > is webkit, I learned about another case via elego's SVN support). > > > These repositories are now growing faster than they would if rep-caching > > > could be re-enabled easily. > > > > I don't follow. There's nothing stopping the admins of those > > repositories from reënabling rep-cache as soon as they upgrade to > > a Subversion release that has the sha1 collision fixes. That will solve > > the growth problem going forward. Compared to this, the benefits of > > running build-repcache on those repositories are limited. > > Ah, yes. You're right about that. > > I suspect some might have left the rep-cache disabled anyway cause > of (unwarranted) trust issues. > > Apart from the above, there's also a small possibility that a rep-cache DB > becomes corrupt or somehow unusable by sqlite. In such cases this command > would be very useful. +1 Come to think of it, mightn't it also be useful in plain «svnadmin load» use-cases? Disable rep-cache, run «svnadmin load», then after it's done go back and populate the rep-cache. Or even run «svnadmin load --use-post-commit-hook» with a post-commit hook that forks off an «svnadmin build-rep -r $REV» process, to have rep-cache population happen in a separate thread.
