Patchwork config: add extension point for extracting all included files

login
register
mail settings
Submitter Mathias De Maré
Date Feb. 3, 2017, 3:56 p.m.
Message ID <ea2e95febfeff39377c8.1486137391@ubuntu-512mb-ams3-01>
Download mbox | patch
Permalink /patch/18321/
State Changes Requested
Headers show

Comments

Mathias De Maré - Feb. 3, 2017, 3:56 p.m.
# HG changeset patch
# User Mathias De Mare <mathias.de_mare@nokia.com>
# Date 1486132049 -3600
#      Fri Feb 03 15:27:29 2017 +0100
# Node ID ea2e95febfeff39377c878fa05cc853832bc3b2a
# Parent  eb78ec9e97b70310e2944f72c29463bedfc21442
config: add extension point for extracting all included files
Sean Farley - Feb. 3, 2017, 11:24 p.m.
Mathias De Maré <mathias.demare@gmail.com> writes:

> # HG changeset patch
> # User Mathias De Mare <mathias.de_mare@nokia.com>
> # Date 1486132049 -3600
> #      Fri Feb 03 15:27:29 2017 +0100
> # Node ID ea2e95febfeff39377c878fa05cc853832bc3b2a
> # Parent  eb78ec9e97b70310e2944f72c29463bedfc21442
> config: add extension point for extracting all included files

I think I see where this is going based on IRC conversation. I guess
it's ok for now.
Mathias De Maré - Feb. 7, 2017, 3:31 p.m.
> -----Original Message-----

> From: Mercurial-devel [mailto:mercurial-devel-bounces@mercurial-scm.org]

> On Behalf Of Kevin Bullock

> Sent: vrijdag 3 februari 2017 23:54

> To: Mathias De Maré <mathias.demare@gmail.com>

> Cc: mercurial-devel@mercurial-scm.org

> Subject: Re: [PATCH] config: add extension point for extracting all included

> files

> 

> > On Feb 3, 2017, at 09:56, Mathias De Maré <mathias.demare@gmail.com>

> wrote:

> >

> > # HG changeset patch

> > # User Mathias De Mare <mathias.de_mare@nokia.com> # Date

> 1486132049

> > -3600

> > #      Fri Feb 03 15:27:29 2017 +0100

> > # Node ID ea2e95febfeff39377c878fa05cc853832bc3b2a

> > # Parent  eb78ec9e97b70310e2944f72c29463bedfc21442

> > config: add extension point for extracting all included files

> 

> Hmm... for those of us who haven't been paying close attention, can you say

> more about why this is desirable? Generally we don't add an extension point

> unless at least an in-tree extension uses it. (It's possible there are historical

> exceptions to this.)


Pierre-Yves and myself spent some time working on an extension that would allow users to share their configuration with a server and allow a server to send configuration suggestions to a client. The work so far is available at https://bitbucket.org/Mathiasdm/hg-configexpress/overview
One of the possible suggestions to clients would be to add specific includes, and this extension point allows checking if these includes are present.

At some point, we'd like to get this extension added to the official repository, but that's a long-term plan. I'm not sure if that matches the requirements.

Greetings,
Mathias
Yuya Nishihara - Feb. 11, 2017, 9:52 a.m.
On Tue, 7 Feb 2017 15:31:02 +0000, De Mare, Mathias (Nokia - BE) wrote:
> > > On Feb 3, 2017, at 09:56, Mathias De Maré <mathias.demare@gmail.com>
> > wrote:
> > >
> > > # HG changeset patch
> > > # User Mathias De Mare <mathias.de_mare@nokia.com> # Date
> > 1486132049
> > > -3600
> > > #      Fri Feb 03 15:27:29 2017 +0100
> > > # Node ID ea2e95febfeff39377c878fa05cc853832bc3b2a
> > > # Parent  eb78ec9e97b70310e2944f72c29463bedfc21442
> > > config: add extension point for extracting all included files
> > 
> > Hmm... for those of us who haven't been paying close attention, can you say
> > more about why this is desirable? Generally we don't add an extension point
> > unless at least an in-tree extension uses it. (It's possible there are historical
> > exceptions to this.)
> 
> Pierre-Yves and myself spent some time working on an extension that would allow users to share their configuration with a server and allow a server to send configuration suggestions to a client. The work so far is available at https://bitbucket.org/Mathiasdm/hg-configexpress/overview
> One of the possible suggestions to clients would be to add specific includes, and this extension point allows checking if these includes are present.

Can it be done by using/extending config._source dict?

I don't think hooking config method is a good idea because config object
may be used to parse template maps, .hgsub, etc.
Mathias De Maré - March 1, 2017, 2:51 p.m.
On 11-02-17 10:52, Yuya Nishihara wrote:
> On Tue, 7 Feb 2017 15:31:02 +0000, De Mare, Mathias (Nokia - BE) wrote:
>>>> On Feb 3, 2017, at 09:56, Mathias De Maré <mathias.demare@gmail.com>
>>> wrote:
>>>> # HG changeset patch
>>>> # User Mathias De Mare <mathias.de_mare@nokia.com> # Date
>>> 1486132049
>>>> -3600
>>>> #      Fri Feb 03 15:27:29 2017 +0100
>>>> # Node ID ea2e95febfeff39377c878fa05cc853832bc3b2a
>>>> # Parent  eb78ec9e97b70310e2944f72c29463bedfc21442
>>>> config: add extension point for extracting all included files
>>> Hmm... for those of us who haven't been paying close attention, can you say
>>> more about why this is desirable? Generally we don't add an extension point
>>> unless at least an in-tree extension uses it. (It's possible there are historical
>>> exceptions to this.)
>> Pierre-Yves and myself spent some time working on an extension that would allow users to share their configuration with a server and allow a server to send configuration suggestions to a client. The work so far is available at https://bitbucket.org/Mathiasdm/hg-configexpress/overview
>> One of the possible suggestions to clients would be to add specific includes, and this extension point allows checking if these includes are present.
> Can it be done by using/extending config._source dict?
>
> I don't think hooking config method is a good idea because config object
> may be used to parse template maps, .hgsub, etc.
config._source can't be used as it is (as it stores the absolute paths, 
while config includes could be include with a relative path).

I guess one possible alternative would be to create a config._relsource 
containing the relative paths, but that would require signature changes 
to some of the config methods.
Another option would be for hg-configexpress to implement parsing of 
configfiles itself (only the 'includes' part), but that seems a bit 
redundant.
Finally, it would be possible to add 2 additional keyword arguments to 
the 'include' function that's called inside config.config.parse, so it 
would be something like: include(inc, remap=remap, sections=sections, 
base=base, expanded=expanded)

I quite like the last option, but it would require changing the subrepo 
code as well (like you mentioned).

(Sorry, I appear to have missed this mail due to my holidays.)

Greetings,
Mathias
Yuya Nishihara - March 2, 2017, 3:56 p.m.
On Wed, 1 Mar 2017 15:51:44 +0100, Mathias De Maré wrote:
> On 11-02-17 10:52, Yuya Nishihara wrote:
> > On Tue, 7 Feb 2017 15:31:02 +0000, De Mare, Mathias (Nokia - BE) wrote:
> >>>> On Feb 3, 2017, at 09:56, Mathias De Maré <mathias.demare@gmail.com>
> >>> wrote:
> >>>> # HG changeset patch
> >>>> # User Mathias De Mare <mathias.de_mare@nokia.com> # Date
> >>> 1486132049
> >>>> -3600
> >>>> #      Fri Feb 03 15:27:29 2017 +0100
> >>>> # Node ID ea2e95febfeff39377c878fa05cc853832bc3b2a
> >>>> # Parent  eb78ec9e97b70310e2944f72c29463bedfc21442
> >>>> config: add extension point for extracting all included files
> >>> Hmm... for those of us who haven't been paying close attention, can you say
> >>> more about why this is desirable? Generally we don't add an extension point
> >>> unless at least an in-tree extension uses it. (It's possible there are historical
> >>> exceptions to this.)
> >> Pierre-Yves and myself spent some time working on an extension that would allow users to share their configuration with a server and allow a server to send configuration suggestions to a client. The work so far is available at https://bitbucket.org/Mathiasdm/hg-configexpress/overview
> >> One of the possible suggestions to clients would be to add specific includes, and this extension point allows checking if these includes are present.
> > Can it be done by using/extending config._source dict?
> >
> > I don't think hooking config method is a good idea because config object
> > may be used to parse template maps, .hgsub, etc.
> config._source can't be used as it is (as it stores the absolute paths, 
> while config includes could be include with a relative path).
> 
> I guess one possible alternative would be to create a config._relsource 
> containing the relative paths, but that would require signature changes 
> to some of the config methods.
> Another option would be for hg-configexpress to implement parsing of 
> configfiles itself (only the 'includes' part), but that seems a bit 
> redundant.
> Finally, it would be possible to add 2 additional keyword arguments to 
> the 'include' function that's called inside config.config.parse, so it 
> would be something like: include(inc, remap=remap, sections=sections, 
> base=base, expanded=expanded)

Internal API changes should be okay. I don't know which would be better,
but both config._relsource and include() sound good to me.

Patch

diff -r eb78ec9e97b7 -r ea2e95febfef mercurial/config.py
--- a/mercurial/config.py	Mon Jan 16 21:17:39 2017 -0800
+++ b/mercurial/config.py	Fri Feb 03 15:27:29 2017 +0100
@@ -89,6 +89,11 @@ 
                 self._data[section].pop(item, None)
             self._source.pop((section, item), None)
 
+    def checkincludedfile(self, base, expanded):
+        """make it possible for extensions (configexpress)
+           to extract the included files"""
+        pass
+
     def parse(self, src, data, sections=None, remap=None, include=None):
         sectionre = util.re.compile(br'\[([^\[]+)\]')
         itemre = util.re.compile(br'([^=\s][^=]*?)\s*=\s*(.*\S|)')
@@ -130,6 +135,7 @@ 
 
                     try:
                         include(inc, remap=remap, sections=sections)
+                        self.checkincludedfile(base, expanded)
                         break
                     except IOError as inst:
                         if inst.errno != errno.ENOENT: