<div dir="ltr"><br><br><div class="gmail_quote">On Fri, May 8, 2015 at 2:13 PM Adrian Buehlmann <<a href="mailto:adrian@cadifra.com">adrian@cadifra.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 2015-05-08 21:59, Martin von Zweigbergk wrote:<br>
> # HG changeset patch<br>
> # User Martin von Zweigbergk <<a href="mailto:martinvonz@google.com" target="_blank">martinvonz@google.com</a>><br>
> # Date 1430953094 25200<br>
> #      Wed May 06 15:58:14 2015 -0700<br>
> # Node ID baa1fb9f49617b0d554c6c47af9838922baf9a35<br>
> # Parent  8179af513aebf96c4902ba3e5e3cf710d49501e4<br>
> pathencode: for long paths, strip first 5 chars, not first dir<br>
><br>
> When encoding long paths, the pure Python code strips the first<br>
> directory from the path, while the native code currently strips the<br>
> first 5 characters. This discrepancy has not been a problem so far,<br>
> since we have not stored anything in directories other than<br>
> data/. However, we will soon be storing submanifest revlogs in<br>
> metadata/, so the discrepancy will have to go [1]. Since file<br>
> collisions are avoided by the hashing alone (which is done on the full<br>
> unencoded path), it doesn't really matter whether we drop the first<br>
> dir, the first 5 characters, or special-case non-data/. To avoid<br>
> touching the C code, let's always strip the first 5 characters.<br>
><br>
>  [1] Or maybe elsewhere, but the discrepancy is ugly either way.<br>
><br>
> diff -r 8179af513aeb -r baa1fb9f4961 mercurial/store.py<br>
> --- a/mercurial/store.py      Thu May 07 16:43:58 2015 -0700<br>
> +++ b/mercurial/store.py      Wed May 06 15:58:14 2015 -0700<br>
> @@ -187,7 +187,7 @@<br>
><br>
>  def _hashencode(path, dotencode):<br>
>      digest = _sha(path).hexdigest()<br>
> -    le = lowerencode(path).split('/')[1:]<br>
> +    le = lowerencode(path[5:]).split('/')<br>
>      parts = _auxencode(le, dotencode)<br>
>      basename = parts[-1]<br>
>      _root, ext = os.path.splitext(basename)<br>
> diff -r 8179af513aeb -r baa1fb9f4961 tests/test-hybridencode.py<br>
> --- a/tests/test-hybridencode.py      Thu May 07 16:43:58 2015 -0700<br>
> +++ b/tests/test-hybridencode.py      Wed May 06 15:58:14 2015 -0700<br>
> @@ -460,3 +460,9 @@<br>
>            'VWXYZ-1234567890-xxxxxxxxx-xxxxxxxxx-xxxxxxxx-xxxx'<br>
>            'xxxxx-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwww'<br>
>            'wwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww.i')<br>
> +<br>
> +print "paths outside data/ can be encoded"<br>
> +show('metadata/dir/00manifest.i')<br>
> +show('metadata/12345678/12345678/12345678/12345678/12345678/'<br>
> +          '12345678/12345678/12345678/12345678/12345678/12345678/'<br>
> +          '12345678/12345678/00manifest.i')<br>
> diff -r 8179af513aeb -r baa1fb9f4961 tests/test-hybridencode.py.out<br>
> --- a/tests/test-hybridencode.py.out  Thu May 07 16:43:58 2015 -0700<br>
> +++ b/tests/test-hybridencode.py.out  Wed May 06 15:58:14 2015 -0700<br>
> @@ -491,3 +491,10 @@<br>
>  A = 'data/12345678/12345678/12345678/12345678/12345678/12345678/12345678/12345/-xxxxxxxxx-xxxxxxxxx-xxxxxxxxx-123456789-12.3456789-12345-ABCDEFGHIJKLMNOPRSTUVWXYZ-abcdefghjiklmnopqrstuvwxyz-ABCDEFGHIJKLMNOPRSTUVWXYZ-1234567890-xxxxxxxxx-xxxxxxxxx-xxxxxxxx-xxxxxxxxx-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww-wwwwwwwww.i'<br>
>  B = 'dh/12345678/12345678/12345678/12345678/12345678/12345678/12345678/12345/-xxxxx93352aa50377751d9e5ebdf52da1e6e69a6887a6.i'<br>
><br>
> +paths outside data/ can be encoded<br>
> +A = 'metadata/dir/00manifest.i'<br>
> +B = 'metadata/dir/00manifest.i'<br>
> +<br>
> +A = 'metadata/12345678/12345678/12345678/12345678/12345678/12345678/12345678/12345678/12345678/12345678/12345678/12345678/12345678/00manifest.i'<br>
> +B = 'dh/ata/12345678/12345678/12345678/12345678/12345678/12345678/12345678/00manife0a4da1f89aa2aa9eb0896eb451288419049781b4.i'<br>
> +<br>
<br>
The current layout (using the current, 'fncache', repo format) for<br>
regular revlogs under .hg/store/ is:<br>
<br>
  data/      for non-path-hashed revlogs<br>
  dh/        for path-hashed-revlogs<br>
<br>
You - as I understand it - want to separately store submanifests under a<br>
new directory named 'metadata', under 'store' (leaving aside for now<br>
that I don't like the name 'metadata', because of its length).<br>
<br>
Don't you then also need a separate directory for paths of submanifests<br>
that trigger the path-hash-encoding?<br>
<br>
If you store path-hashed submanifest revlogs under 'dh', together with<br>
non-submanifest revlogs, won't that lead to potential path collisions<br>
inside dh?</blockquote><div><br></div><div>I think not. That's what I tried to explain with the "collisions are avoided by the hashing alone" bit.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The trailing 'ata' of 'metadata' doesn't look like a correct<br>
discriminator to me, as that string may derive from the working tree as<br>
well.<br>
</blockquote></div></div>