Patchwork chgserver: include [extdiff] in confighash

login
register
mail settings
Submitter Jun Wu
Date March 11, 2016, 1 p.m.
Message ID <fed30eaade617978d93c.1457701250@x1c>
Download mbox | patch
Permalink /patch/13769/
State Accepted
Commit e6e183687545f90b756ca523aea52c3f310b8f14
Headers show

Comments

Jun Wu - March 11, 2016, 1 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1457701220 0
#      Fri Mar 11 13:00:20 2016 +0000
# Node ID fed30eaade617978d93c5fe186e907691d4e3500
# Parent  5021398417ed44f7d589879ff74ce9e9eaf20b5b
chgserver: include [extdiff] in confighash

extdiff's uisetup will register new commands. If we do not include it in
confighash, changes to [extdiff] will not get new commands registered.
This patch adds extdiff to confighash and makes it possible for chg to pass
test-extdiff.t.
Pierre-Yves David - March 11, 2016, 1:04 p.m.
On 03/11/2016 01:00 PM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457701220 0
> #      Fri Mar 11 13:00:20 2016 +0000
> # Node ID fed30eaade617978d93c5fe186e907691d4e3500
> # Parent  5021398417ed44f7d589879ff74ce9e9eaf20b5b
> chgserver: include [extdiff] in confighash
>
> extdiff's uisetup will register new commands. If we do not include it in
> confighash, changes to [extdiff] will not get new commands registered.
> This patch adds extdiff to confighash and makes it possible for chg to pass
> test-extdiff.t.

What is the story for another extensions (3rd party) that need to 
register itself here?

> diff --git a/hgext/chgserver.py b/hgext/chgserver.py
> --- a/hgext/chgserver.py
> +++ b/hgext/chgserver.py
> @@ -77,7 +77,8 @@
>       return util.sha1(str(items)).hexdigest()
>
>   # sensitive config sections affecting confighash
> -_configsections = ['extensions']
> +# extdiff's uisetup will register new commands
> +_configsections = ['extensions', 'extdiff']
>
>   # sensitive environment variables affecting confighash
>   _envre = re.compile(r'''\A(?:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Jun Wu - March 11, 2016, 1:06 p.m.
On 03/11/2016 01:04 PM, Pierre-Yves David wrote:
> What is the story for another extensions (3rd party) that need to register
> itself here?

They can import chgserver and change the array, just like hg.wirepeersetupfuncs

I thought about doing this in extdiff.py but since we tends to handle internal
extensions (pager, progress etc.) directly in command server code so I follow
the pattern.
Pierre-Yves David - March 11, 2016, 1:41 p.m.
On 03/11/2016 01:06 PM, Jun Wu wrote:
> On 03/11/2016 01:04 PM, Pierre-Yves David wrote:
>> What is the story for another extensions (3rd party) that need to
>> register
>> itself here?
>
> They can import chgserver and change the array, just like
> hg.wirepeersetupfuncs

But are they loaded soon enough to allow the chg server to use them for 
its hashing?

> I thought about doing this in extdiff.py but since we tends to handle
> internal
> extensions (pager, progress etc.) directly in command server code so I
> follow
> the pattern.

progress is in core and pager is getting there. I think it would be 
better to do it in the extension to make sure 3rd party one can actually 
use this.

Cheers,
Jun Wu - March 11, 2016, 1:58 p.m.
On 03/11/2016 01:41 PM, Pierre-Yves David wrote:
> But are they loaded soon enough to allow the chg server to use them for its
> hashing?

Hashing happens after extensions.loadall() so it's okay.

> progress is in core and pager is getting there. I think it would be better to
> do it in the extension to make sure 3rd party one can actually use this.

I'm okay with either way. Let's wait for Yuya's comment.
Yuya Nishihara - March 11, 2016, 3:06 p.m.
On Fri, 11 Mar 2016 13:58:06 +0000, Jun Wu wrote:
> On 03/11/2016 01:41 PM, Pierre-Yves David wrote:
> > But are they loaded soon enough to allow the chg server to use them for its
> > hashing?
> 
> Hashing happens after extensions.loadall() so it's okay.
> 
> > progress is in core and pager is getting there. I think it would be better to
> > do it in the extension to make sure 3rd party one can actually use this.
> 
> I'm okay with either way. Let's wait for Yuya's comment.

I'm okay with either way, too. If we open up _configsections for 3rd party, I
think we'll need a way to specify "section.name".
Yuya Nishihara - March 11, 2016, 4:13 p.m.
On Fri, 11 Mar 2016 13:00:50 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457701220 0
> #      Fri Mar 11 13:00:20 2016 +0000
> # Node ID fed30eaade617978d93c5fe186e907691d4e3500
> # Parent  5021398417ed44f7d589879ff74ce9e9eaf20b5b
> chgserver: include [extdiff] in confighash
> 
> extdiff's uisetup will register new commands. If we do not include it in
> confighash, changes to [extdiff] will not get new commands registered.
> This patch adds extdiff to confighash and makes it possible for chg to pass
> test-extdiff.t.
> 
> diff --git a/hgext/chgserver.py b/hgext/chgserver.py
> --- a/hgext/chgserver.py
> +++ b/hgext/chgserver.py
> @@ -77,7 +77,8 @@
>      return util.sha1(str(items)).hexdigest()
>  
>  # sensitive config sections affecting confighash
> -_configsections = ['extensions']
> +# extdiff's uisetup will register new commands
> +_configsections = ['extensions', 'extdiff']

I've reformatted this to list an item per line, and will push to the
clowncopter, thanks.

Let's revisit the story about 3rd-party extensions after we can make it pass
tests with --chg.

Patch

diff --git a/hgext/chgserver.py b/hgext/chgserver.py
--- a/hgext/chgserver.py
+++ b/hgext/chgserver.py
@@ -77,7 +77,8 @@ 
     return util.sha1(str(items)).hexdigest()
 
 # sensitive config sections affecting confighash
-_configsections = ['extensions']
+# extdiff's uisetup will register new commands
+_configsections = ['extensions', 'extdiff']
 
 # sensitive environment variables affecting confighash
 _envre = re.compile(r'''\A(?: