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

svn_ra_get_file_revs2 vs. blame vs. FS API

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Sat, 15 Feb 2014 21:14:25 +0100

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
sides,
they will still be considered "changed". That is a violation of the API
definition
basically asking "is the delta empty?". Even the FSX code is not fully
consistent
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
"mainline"
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
side.

* 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
  mainline.

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

-- Stefan^2.
Received on 2014-02-15 21:14:57 CET

This is an archived mail posted to the Subversion Dev mailing list.