[PATCH] ui: support declaring repositories in [remotes] section

[Sean Farley]

Yuya Nishihara <yuya at tcha.org> writes:

> On Sat, 14 Nov 2015 19:38:55 -0800, Sean Farley wrote:
>> 
>> Gregory Szorc <gregory.szorc at gmail.com> writes:
>> 
>> >> On Nov 14, 2015, at 19:08, Sean Farley <sean at farley.io> wrote:
>> >> 
>> >> 
>> >> Gregory Szorc <gregory.szorc at gmail.com> writes:
>> >> 
>> >>> # HG changeset patch
>> >>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> >>> # Date 1447395178 28800
>> >>> #      Thu Nov 12 22:12:58 2015 -0800
>> >>> # Node ID eeed46507676c60df3febc358b5d301ed4470855
>> >>> # Parent  3309714ded262f1bb3e32dd552b01d6926de06d5
>> >>> ui: support declaring repositories in [remotes] section
>> >>> 
>> >>> Power users often want to apply per-path configuration options. For
>> >>> example, they may want to declare an alternate URL for push operations
>> >>> or declare a revset of revisions to push when `hg push` is used
>> >>> (as opposed to attempting to push all revisions by default).
>> >>> 
>> >>> This patch establishes the [remotes] section (bikeshedding invited).
>> >>> Unlike the [paths] sections, the [remotes] section reserves options
>> >>> with "." in them for declaring per-path/remote attributes. (The
>> >>> [paths] section interprets options with "." as a valid alias and
>> >>> attempting to shoehorn "." attribute interpretation into [paths]
>> >>> runs the risk of ambiguous behavior when a user defined an alias
>> >>> with "." that happens to conflict with an attribute defining
>> >>> special behavior. So, a dedicated section with different parsing
>> >>> semantics is necessary to avoid ambiguity.)
>> >>> 
>> >>> Initially, we only support the "pushurl" per-path attribute. However,
>> >>> additional attributes can be added relatively easily.
>> >>> 
>> >>> This patch requires that location values are URLs and that the URLs
>> >>> don't contain #fragment parts. The URL requirement might be
>> >>> controversial. I'm implementing it to see what people say. If people
>> >>> don't like it, I can remove it with little objection. However, I would
>> >>> encourage people to read the location interpretation logic in
>> >>> ui.path.__init__ and commands.push before rushing to judgement. The
>> >>> code and behavior is a bit complicated and requiring URLs does reduce
>> >>> ambiguity over how values should be interpretted. The banning of
>> >>> #fragment from locations should hopefully be less controversial. As
>> >>> a refresher, these declare the default branch to push to. Since
>> >>> branches aren't as popular as they once were, I'm guessing few will
>> >>> miss this feature. Plus, I intend to introduce a "pushrev" or
>> >>> "defaultpushrev" per-path attribute that declares a revset to be used
>> >>> to select what will be pushed when no -r argument is given to
>> >>> `hg push`. This feature will restore the capabilities of #fragment
>> >>> in the URL but will be more powerful and more useful.
>> >>> 
>> >>> In the spirit of full disclosure, I believe mpm had signed off on
>> >>> [urls] for the new section name. However, there has since been some
>> >>> discussion on introducing "remote" "refs" into core. I'm not sure
>> >>> what the user-facing nomenclature for that is going to be. But if it
>> >>> is "remotes", then IMO the section for declaring repos should be
>> >>> [remotes], as implemented in this patch.
>> >>> 
>> >>> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
>> >>> --- a/mercurial/help/config.txt
>> >>> +++ b/mercurial/help/config.txt
>> >>> @@ -1149,8 +1149,9 @@ used from the command line. Example::
>> >>> To push to the path defined in ``my_path`` run the command::
>> >>> 
>> >>>     hg push my_path
>> >>> 
>> >>> +(See ``[remotes]`` for a more advanced method of declaring repositories.)
>> >>> 
>> >>> ``phases``
>> >>> ----------
>> >>> 
>> >>> @@ -1283,8 +1284,30 @@ have a definite end point.
>> >>> 
>> >>> ``assume-tty``
>> >>>     If true, ALWAYS show a progress bar, unless disable is given.
>> >>> 
>> >>> +``remotes``
>> >>> +-----------
>> >>> +
>> >>> +Declare symbolic names to remote repositories along with additional attributes
>> >>> +defining behavior.
>> >>> +
>> >>> +Options without "." declare the symbolic name of a repository and its location.
>> >>> +Options with "." declare attributes for a repository. For example::
>> >>> +
>> >>> +    [remotes]
>> >>> +    my_remote = https://example.com/repo
>> >>> +    my_remote.pushurl = ssh://example.com/repo
>> >>> +
>> >>> +The followeing per-entry attributes are recognized:
>> >>> +
>> >>> +``pushurl``
>> >>> +    The URL to use for push operations. If not defined, the location defined
>> >>> +    by the attribute-less config option is used.
>> >>> +
>> >>> +(See ``[paths]`` for a simpler but less powerful method to declare symbolic
>> >>> +names to repositories.)
>> >> 
>> >> I really hate another config section for something that is still
>> >> [paths]. The BC complaint seems to be splitting on '.' but we aren't
>> >> going to use any TLDs for our actions (are we?). So, can't we just split
>> >> on '.pushurl'?
>> >> 
>> >> For example,
>> >> 
>> >> [paths]
>> >> smf.io = URL1
>> >> smf.io.pushurl = URL2
>> >> smf.io.pushrev = '.'
>> >> 
>> >> In this case, we can easily split on '.pushurl' and 'pushrev' without
>> >> any ambiguity.
>> >
>> > I can't predict the future and can't guarantee we won't introduce per-path
>> > attributes that have sub-attributes. E.g. smf.bookmarkmap.@ = upstream
>> >
>> > (Probably not a good example because of remotenames. But hopefully it gets
>> > my point across that splitting on "." can be ambiguous.)
>> >
>> > Also, this isn't about domains /TLDs having the naming conflict: it's about
>> > the aliases people choose.
>> 
>> I still don't see how splitting from the left wouldn't be safe enough. I
>> guess you're saying that someone could have '.pushurl' alias. I don't
>> agree that that's enough to warrant a new section and would rather
>> improve what we have.
>
> I didn't read everything, but there were long discussion. It isn't safe if
> you have "smf" and "smf.io" in paths, and if we introduce ".io" attribute
> in future version. Every single new attribute could be a BC.
>
> http://thread.gmane.org/gmane.comp.version-control.mercurial.devel/76859
>
> Anyway, if we want to put everything in "[paths]", can we use a different
> separator such as ":" ? It didn't work until 3.6, so nobody would use "foo:bar"
> aliases.
>
>   [paths]
>   smf.io = URL1
>   smf.io:pushurl = URL2

Awesome, that's perfectly fine with me.