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

RE: svn_ra_get_file_revs2 vs. blame vs. FS API

From: Bert Huijben <bert_at_qqmail.nl>
Date: Sun, 16 Feb 2014 23:09:17 +0100



I don't believe the -g implementation really works. It is a nice proof of
concept, but it doesn't handle many merge scenarios. (It only works if you
assume branches that are kept closely in sync).


But the changes you suggest look much broader than the -g mode.


I don't see a problem with changing the requirements for the use merge
tracking mode as that already breaks most rules about a direct version path,
but I would be afraid that we do break backward compatibility if this also
changes the behavior for not-g mode.



The comments on 'svn_ra_get_file_revs2 always accept a delta handler' don't
make sense for me. That function has a delta handler as argument and I don't
see scenarios where it doesn't accept that argument?


Perhaps you should explain what the real problem is what you try to solve,
instead of assuming that everybody has a deep knowledge about both the fs
internals and this very specific api that drives blame (and several third
party tools that want to obtain all versions of a file in the most efficient
way possible).




From: Stefan Fuhrmann [mailto:stefan.fuhrmann_at_wandisco.com]
Sent: zaterdag 15 februari 2014 21:14
To: Subversion Development
Subject: svn_ra_get_file_revs2 vs. blame vs. FS API


Hi there,

r1568600 uncovered an inconsistency in our API usage / interpretation

making blame -g tests fail for FSX.

The starting point is svn_fs_contents_changed and svn_fs_props_changed.

FSFS and probably BDB implement those as "has there been an intermittent
change?" E.g. if the result of some merge yields equal content on both

they will still be considered "changed". That is a violation of the API

basically asking "is the delta empty?". Even the FSX code is not fully

with that, yet.

svn_ra_get_file_revs2 uses svn_fs_contents_changed to decide whether

a delta should be sent to the given handler. The false positives returned by

FSFS and BDB are benign since we simply end up sending an empty delta

(a mere waste of CPU cycles).

However, our blame implementation relies on those false positives because

it tracks contents from revisions to be merged later separately from

changes. When the merge finally happens but results in the same contents
as the previous branch revision, the delta handler must be called to pull in
the blame info from the branch. The latter no longer happens with the strict

interpretation of svn_fs_contents_changed.

I see the following options:

* Redefine svn_fs_contents_changed and svn_fs_props_changed to match

  the current implementation. I'd rather not but that just a -0 from my

* Make svn_ra_get_file_revs2 always accept a delta handler unless the

  previous change was on the same branch and the delta is empty (e.g.

  just a prop change). Sounds arbitrary, so -0 on that too.

* Make svn_ra_get_file_revs2 accept delta handlers (roughly) as today

  but use the log information for that, i.e. whenever the content modified

  flag is set for the respective file path. That would be sound approach

  since we are in rev_hunt.c anyway. The info may already be available

  (haven't checked yet).

* Make the blame implementation update the mainline blame info even

  if no new delta info came in but the previous callback wasn't for the


I suggest to do any of the last two or even both.

-- Stefan^2.

Received on 2014-02-16 23:09:55 CET

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.