[PATCH 1 of 6 path intent] ui: add an intent to expandpath

Gregory Szorc gregory.szorc at gmail.com
Mon Sep 29 17:53:06 CDT 2014


On 9/29/14 2:43 PM, Mads Kiilerich wrote:
> On 09/29/2014 07:30 AM, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1411967336 25200
>> #      Sun Sep 28 22:08:56 2014 -0700
>> # Node ID 2f06625fe4d502f43294e9087d9a83398a36fe9f
>> # Parent  4109cc16279ef0e04dc70e7f4c9ab7415e6a22d3
>> ui: add an intent to expandpath
>>
>> This patch adds an optional named argument to ui.expandpath() that
>> defines the intended use of a repository.
>>
>> The impetus behind this patch is to allow extensions to provide
>> more advanced and dynamic repository lookup functionality. For
>> example, there exists an extension at Mozilla which automagically
>> defines common repository URLs for the many Firefox repositories.
>> We want read operations hitting read-only slaves over HTTP(S) and
>> write operations hitting the master over SSH. Today, we have to
>> use separate names for separate URLs, which increases cognitive
>> burden and sometimes results in accidental use of the wrong repo.
>> We desire built-in hg operations to automagically use the URL for
>> the task at hand. This change and subsequent additions to define
>> the intent parameter will enable this through monkeypatching
>> ui.expandpath().
>
> Meh...
>
> First: It doesn't work to add "features" to Mercurial without any usage
> or test in Mercurial - especially not without any reference to exactly
> what it is that need this feature. That would be unmaintainable.
>
> I assume the intent parameter should be considered some enumeration of
> some well-known path types - that do not seem like a nice API, but we
> would at least need a definition of which values are valid and what they
> mean.
>
> Also, Incoming should be like a preview of pull and outgoing like a
> preview of push. They should thus be treated exactly the same and it
> seems wrong that they have different "intents".
>
> The existing handling of push/pull paths is also a bit not-so-elegant.
> The default and -push handling is duplicated in all the places it is used.
>
> Instead, I suggest giving the ui class a couple of extra methods:
>
>      def pushpath(self, loc):
>          """Return path (which might be a URL) for pushing to loc.
>          loc will be 'default' if not specified.
>          If paths.loc is configured, use that instead of loc.
>          However, if paths.loc-push is configured, prefer that.
>          """
>      def pullpath(loc):
>          """Return path (which might be a URL) for pulling from loc.
>          loc will be 'default' if not specified.
>          If paths.loc is configured, use that instead of loc.
>          """
>
> (This docstring also specify that the -push suffix should apply to other
> paths than default. I find that an obvious extension that it would be
> hard not to implement accidentally.)
>
> (I have always found the special -push suffix a bit odd. Having a
> special -pull suffix seemed more obvious and safe to me. For symmetry
> reasons I would like to have both so I could specify default-push and
> default-pull and know that they "always" only would be used for their
> intended purpose.)
>
> I assume your extensions could do their job by monkeypatching these
> methods? Or why not? What do these extensions do?

My primary case was documented in the commit message.

I think adding explicit APIs for pushpath() and pullpath() would be 
sufficient for my needs. Although, outgoing could fall into either camp 
and having an explicit intent for the operation at hand could be useful. 
Not sure I can justify it though.

I don't feel comfortable bloating scope to include adding magical 
"-push" for every path. For one, I believe there are proposals around a 
more robust [paths] definition (defining which branches/bookmarks to 
"track," etc) and I don't want to implement something that could 
interfere too much with that future. But if I have buy-in, I'll do what 
it takes to land this feature.


More information about the Mercurial-devel mailing list