[PATCH 1 of 2] manifest: allow specifying the revlog filename

Martin von Zweigbergk martinvonz at google.com
Tue Feb 28 12:53:33 EST 2017


On Tue, Feb 28, 2017 at 9:49 AM, Durham Goode <durham at fb.com> wrote:
>
>
> On 2/27/17 5:10 PM, Martin von Zweigbergk wrote:
>>
>> +mercurial-devel again (I had initially thought my comment was not
>> about the patch, but it turns out it was)
>>
>> On Sat, Feb 25, 2017 at 3:23 PM, Durham Goode <durham at fb.com> wrote:
>>>
>>> On 2/24/17 10:00 PM, Martin von Zweigbergk wrote:
>>>>
>>>>
>>>> -list
>>>>
>>>> On Fri, Feb 24, 2017, 14:44 Durham Goode <durham at fb.com
>>>> <mailto:durham at fb.com>> wrote:
>>>>
>>>>     # HG changeset patch
>>>>     # User Durham Goode <durham at fb.com <mailto:durham at fb.com>>
>>>>     # Date 1487973639 28800
>>>>     #      Fri Feb 24 14:00:39 2017 -0800
>>>>     # Node ID ed987b24e755a4c61c006f7d98a4ff370229faad
>>>>     # Parent  f01df5d2fe493376a67569756a9bc2ddffa5cd81
>>>>     manifest: allow specifying the revlog filename
>>>>
>>>>     Previously we had hardcoded the manifest filename to be
>>>>     00manifest.i. In our
>>>>     external treemanifest extension, we want to allow writing a
>>>>     treemanifest side by
>>>>     side with a flat manifest, so we need to be able to store the root
>>>>     revisions at
>>>>     a different location (in our extension we use 00manifesttree.i).
>>>>
>>>>
>>>> Why not meta/00manifest.i? That would be more consistent. I let them
>>>> (flat and tree) share a revlog only to allow for transition between
>>>> them. But if you don't care about that, meta/00manifest.i sounds like
>>>> the obvious choice.
>>>
>>>
>>>
>>> I'd be fine with that too, but I'm not sure how we'd go about that. I
>>> guess
>>> I could put an option on the opener that the manifest constructor checks
>>> for, like 'separatetreeroot=True' and then the constructor would know to
>>> put
>>> it in meta/.  Seem reasonable?
>>
>>
>> Or you could keep your new indexfile argument to the constructor and
>> use that if it was set and otherwise use the current way of
>> calculating it. Your extension could then pass 'meta/00manifest.i' for
>> dir=='' and None for all others. That gives more flexibility if you
>> want to use some other scheme too. Also, I'd like a comment explaining
>> that the parameter is for extensions, so the next person doesn't
>> remove it.
>
>
> I can add a comment.  I think putting directories in the indexfile string
> might be a bit funky since I'm not sure if other parts of the code rely on
> self.indexfile to be just a filename.

I don't follow. You patch touches a line that says

  indexfile = "meta/" + dir + "00manifest.i"

so it already (potentially) has a directory in it. Or do you mean
something else?

>  I guess I can play with that after it
> lands.


More information about the Mercurial-devel mailing list