Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: 
 <CAEcU=1YBSf5_jw5mCdUANF+WgfZGEYSrdOLtNdxwSwukmVPn-w@mail.gmail.com>
References: 
 <CAB84uBVe8QnEpbPVAb__yQjiDDoYjFn2+M9mPcdBXZCwMCpOLw@mail.gmail.com>
 <20150921210118.GF1955@tarsus.local2>
 <CAEcU=1as1LjchTtLbaeK2W4tCZrjg59a91V6oDg79MeL7NKu=w@mail.gmail.com>
 <CAB84uBU99Sh9fUUNFCTHcmiOcskQwJudpMrKUqXHOtV061+v3A@mail.gmail.com>
 <5602746B.9050800@apache.org>
 <CAB84uBXx5f1-+9GtRNEOQRdtHbt5VpScMVtvM0fAD+kj=RqR+w@mail.gmail.com>
 <56027A52.1090806@apache.org>
 <CAEcU=1ZLjbRVMre4SdX+-uhjku0xEJ-zb-iBiujxwMybhTJZ7Q@mail.gmail.com>
 <CAEcU=1YBSf5_jw5mCdUANF+WgfZGEYSrdOLtNdxwSwukmVPn-w@mail.gmail.com>
From: Julian Foad <julianfoad@btopenworld.com>
Date: Fri, 25 Sep 2015 13:45:26 +0100
Message-ID: 
 <CAEcU=1ZQzccLfYWyTwFhyv2O+mpqgA=2kGY83dTA2pLwFbwV2A@mail.gmail.com>
Subject: Re: No-op changes no longer dumped by 'svnadmin dump' in 1.9
To: dev <dev@subversion.apache.org>
Content-Type: text/plain; charset=UTF-8

I (Julian Foad) wrote:
> Issue #4598 "No-op changes no longer dumped by 'svnadmin dump' in
> 1.9", http://subversion.tigris.org/issues/show_bug.cgi?id=4598
>
> Added to the release notes in http://svn.apache.org/r1704822 :
> file:///home/julianfoad/src/svn-site/publish/docs/release-notes/1.9.html#no-op-changes

For one line of attack, I started constructing some test cases to
explore the dump behaviour. This is quite a hard direction to start
from, because I don't know what internal details may affect the
output.

For another line of attack, I looked into the source code. The logic
in 1.8.x for 'svnadmin dump' is:

Write a node record when the appropriate 'revision-replay' function
calls the dump editor's open_file() or add_file() method for a file,
or its change_dir_prop() method for a dir. The 'revision-replay'
function is svn_repos_replay2() in most cases, or
svn_repos_dir_delta2() when dumping an initial non-incremental rev.

svn_repos_replay2() calls those methods in a manner close to the
following pseudo-Python:

def svn_repos_replay2():
  for (path, change) in svn_fs_paths_changed2():  # via svn_delta_path_driver2()
    if change.node_kind is 'file':
      add_file() or open_file()
    if change.prop_mod:
      if path is being reported as copied:
        for pc in svn_prop_diffs(new_props, old_props):
          change_*_prop(pc.name, pc.value)
      else:
        change_*_prop(dummy parameters)  # call just once

Back to the dumper. Within a node record, output a properties section
iff svn_fs_fs__noderev_same_rep_key() returns true for the properties
representations, and output a text section iff
svn_fs_fs__noderev_same_rep_key() returns true for the text
representations.

svn_boolean_t
svn_fs_fs__noderev_same_rep_key(representation_t *a,
                                representation_t *b)
{
  if (a == b)
    return TRUE;

  if (a == NULL || b == NULL)
    return FALSE;

  if (a->offset != b->offset)
    return FALSE;

  if (a->revision != b->revision)
    return FALSE;

  if (a->uniquifier == b->uniquifier)
    return TRUE;

  if (a->uniquifier == NULL || b->uniquifier == NULL)
    return FALSE;

  return strcmp(a->uniquifier, b->uniquifier) == 0;
}

(For BDB, svn_fs_base__things_different() calls
svn_fs_base__same_keys() to compare the props rep key, text rep key
text rep uniquifier.)

The function svn_fs_fs__noderev_same_rep_key() does not exist in
1.9.{0...2}, but we could recreate a suitable version of it. (The
.uniquifier member has changed form since 1.8.)

The FS layer APIs are:
  svn_fs_contents_changed()
  svn_fs_props_changed()

In 1.9 the implementations of these no longer compare the rep-keys but
use a different strategy, implemented in
svn_fs_fs__dag_things_different() which returns 'false positive'
outputs in fewer and different cases.

If our aim is to restore exact compatibility with 1.8 we'll have to be
careful to put the exact same logic back.

I'll continue examining the code paths.


One more issue I notice in svn_fs_fs__dag_things_different() is even
in 'strict' mode it reports that two strings are equal if their MD5
checksums are equal. Is this intentional, and is it consistent with
the rest of the Subversion system? The test 'compare_contents()' in
fs-test.c expects this behaviour, but I wonder if that is an
oversight?

- Julian