On Tue, Jan 27, 2015 at 11:19 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name>
wrote:
> Good morning Stefan,
>
> stefan2_at_apache.org wrote on Tue, Jan 27, 2015 at 01:48:08 -0000:
> > + /* Redirect text rep to props rep. */
> > + rev_path = svn_fs_fs__path_rev_absolute(fs, rev, pool);
> > + SVN_ERR(svn_stringbuf_from_file2(&rev_contents, rev_path, pool));
> > +
> > + props_line = get_line(rev_contents, "props: ", pool);
> > + text = stringbuf_find(rev_contents, "text: ");
> > +
> > + if (text)
> > + {
> > + /* Explicitly set the last number before the MD5 to 0 */
> > + strstr(props_line->data, " 2d29")[-1] = '0';
> > +
> > + /* Add a padding space - in case we shorten the number in TEXT */
> > + svn_stringbuf_appendbyte(props_line, ' ');
> > +
> > + /* Make text point to the PLAIN data rep with no expanded size
> info. */
> > + memcpy(text + 6, props_line->data + 7, props_line->len - 7);
> > + }
>
> The above bit appears brittle: it heavily depends on the exact current
> layout of rev files, down to the number of whitespace bytes between the
> <expanded_size> and <md5> fields in a noderev header line and the length
> of the uniquifier field.
>
> I realize that it works now, and that if the original bytes ever are
> different the test may corrupt the filesystem and thereby at least fail
> noisily, but perhaps there is a more robust way to achieve the declared
> end of redirecting the text rep to the props rep?
>
You are right. After thinking about it, I remembered that /trunk
has nice parsers and serializers for such tasks, see r1655008.
The downside is that only the original implementation can be
backported.
It is still somewhat brittle as the numbers must not expand,
which they won't. If that was to happen one day, we would
have to add quite a bit more code.
> For example, perhaps enable rep-cache.db (after creating the first revision
> with it disabled) and insert a false record for the text rep, or use
> svn_cstring_split() to get at the individual tokens more robustly.
>
Rep caching is only available for format 4+ while older formats
are particularly likely to use "PLAIN" representations.
> Cheers,
>
> Daniel
>
> P.S. The test changes the number of bytes in the noderev header, without
> changing any of of the physical offsets in the rev file. Why does that
> work with the formats ≤ 6? I expected it to invalidate the
> final changed-paths-offset, but the test runs and passes and the offset is
> correct when it finishes.
>
The length of rev_contents did not change. New data got
memcpy'ed over whatever older text was there.
-- Stefan^2.
Received on 2015-01-27 12:32:13 CET