Julian Foad wrote on Thu, Dec 10, 2015 at 14:27:44 +0000:
> Philip Martin wrote:
> > This discussion seems to have died out. Are we going to leave the BDB
> > code as is, in which case we should mark the failing test XFAIL, or make
> > a change? I still prefer the option that makes the root path mutable on
> > commit, primarily because for the vast majority of commits there is no
> > change at all.
>
> Stepping back a little, I want to pose the rhetorical question: Who is
> to say which FS layer implementation is the wrong one? BDB is the
> earlier implementation. If we define correctness by precedent, then
> it's the FSFS behaviour that we should consider to be wrong. On the
> other hand, if we define correctness by specification, then we need to
> specify this behaviour somewhere, not just "randomly" change it.
>
> So let's try to enumerate the issues.
>
> (1) In FS-BDB, a no-op commit may or may not produce a new root
> node-rev (depending on the specific form of the commit), whereas in
> FSFS, every commit produces a new root node-rev.
>
> (2) FS-BDB reports a different result from svn_fs_txn_base_revision()
> before and after reloading by name, when the no-new-node-rev situation
> in (1) occurs.
>
> (3) A recently added check can reject valid delete operations when (1)
> and (2) occur.
>
> Which of those are bugs, which are acceptable, which need to stay as
> they for backward compatibility even if they are bugs, and so on?
>
> It seems to me that (2) is definitely a bug, but I'm not sure about
> (1) and therefore not sure about (3).
>
About (2): I think svn_fs_txn_base_revision() should return the value of
the REV argument of the svn_fs_begin_txn() call that created the txn.
Therefore, (2) is a bug.
About (1): I think that, unless we have made specific API promises about
when noderevs are or aren't shared, the decision of whether to share
noderevs is an implementation detail of the backend: the backend may
choose to share or not to share, but neither choice is more "right" than
the other.
Therefore: if making BDB never share the root path's noderevs fixes the
bug, then I think it's a fine way to proceed. I just don't think it's
the *only* correct way to proceed. For example, I think the FSFS f7
logical addressing file format supports noderev sharing of the root path
of revisions within a single pack file, so FSFS 1.10 could conceivably
start sharing the root path's noderevs in some circumstances.
So... yes, we can go ahead and make libsvn_fs_base never share the root
path's noderevs. No problem at all with that. But higher-level code
shouldn't rely on that: if it hasn't been documented as an API promise,
it's an implementation detail and subject to change.
I hope this clarifies my viewpoint...
Cheers,
Daniel
Received on 2015-12-12 09:20:44 CET