RE: svn_ra_get_file_revs2 vs. blame vs. FS API

                Hi,

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).

                Bert

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
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-16 23:09:55 CET