| Submitter | David Soria Parra |
|---|---|
| Date | March 14, 2017, 2:47 a.m. |
| Message ID | <7e2222599b4f534e27a0.1489459667@davidsp-mbp-2.local> |
| Download | mbox | patch |
| Permalink | /patch/19306/ |
| State | Changes Requested |
| Headers | show |
Comments
This is a BC because loading order matters: # a.rc %include b.rc [ui] editor = foo %include b.rc # b.rc [ui] editor = bar Since people should be able to arrange config files in a way that no files are included twice. I prefer not being too smart in hg. Excerpts from David Soria Parra's message of 2017-03-13 19:47:47 -0700: > # HG changeset patch > # User David Soria Parra <davidsp@fb.com> > # Date 1489459557 25200 > # Mon Mar 13 19:45:57 2017 -0700 > # Node ID 7e2222599b4f534e27a0462bb1263208cc1c8d71 > # Parent 3d3109339b57341b333c1112beb41dd281fa944a > ui: don't read the same config file twice > > During dispatch and localrepo setup we might read the same file twice if > the repository we are loading is the currnet working directory/.hg which is the > default case. We now keep a set of filenames we read around and abort > if we read the config already. It would be nice to do this in a decorator, > but I have not found a reliable, simple way to hash arguments. > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -160,6 +160,7 @@ > self._colormode = None > self._terminfoparams = {} > self._styles = {} > + self._readfilenames = set() > > if src: > self.fout = src.fout > @@ -258,6 +259,9 @@ > > def readconfig(self, filename, root=None, trust=False, > sections=None, remap=None): > + if filename in self._readfilenames: > + return > + > try: > fp = open(filename, u'rb') > except IOError: > @@ -304,6 +308,7 @@ > if root is None: > root = os.path.expanduser('~') > self.fixconfig(root=root) > + self._readfilenames.add(filename) > > def fixconfig(self, root=None, section=None): > if section in (None, 'paths'):
On 3/13/17 7:56 PM, Jun Wu wrote: > This is a BC because loading order matters: > > # a.rc > %include b.rc > [ui] > editor = foo > %include b.rc > > # b.rc > [ui] > editor = bar > > Since people should be able to arrange config files in a way that no files > are included twice. I prefer not being too smart in hg. I agree with Jun that we need preserve last-loaded-wins semantics. We could still build single-loading if we read config files once, kept them in separate dicts in an ordered set, and just reordered to the end of a list when re-read, and once all configs are read, then combine the dicts in list order, but that sounds a bit complex. I'd still be supportive of that direction if there's a perf win though. > > Excerpts from David Soria Parra's message of 2017-03-13 19:47:47 -0700: >> # HG changeset patch >> # User David Soria Parra <davidsp@fb.com> >> # Date 1489459557 25200 >> # Mon Mar 13 19:45:57 2017 -0700 >> # Node ID 7e2222599b4f534e27a0462bb1263208cc1c8d71 >> # Parent 3d3109339b57341b333c1112beb41dd281fa944a >> ui: don't read the same config file twice >> >> During dispatch and localrepo setup we might read the same file twice if >> the repository we are loading is the currnet working directory/.hg which is the Typo: current is misspelled >> default case. We now keep a set of filenames we read around and abort We don't abort, we return early. Abort would imply raising an error.Abort (the way I inrepretted it anyway). >> if we read the config already. It would be nice to do this in a decorator, >> but I have not found a reliable, simple way to hash arguments. >> >> diff --git a/mercurial/ui.py b/mercurial/ui.py >> --- a/mercurial/ui.py >> +++ b/mercurial/ui.py >> @@ -160,6 +160,7 @@ >> self._colormode = None >> self._terminfoparams = {} >> self._styles = {} >> + self._readfilenames = set() >> >> if src: >> self.fout = src.fout >> @@ -258,6 +259,9 @@ >> >> def readconfig(self, filename, root=None, trust=False, >> sections=None, remap=None): >> + if filename in self._readfilenames: >> + return >> + >> try: >> fp = open(filename, u'rb') >> except IOError: >> @@ -304,6 +308,7 @@ >> if root is None: >> root = os.path.expanduser('~') >> self.fixconfig(root=root) >> + self._readfilenames.add(filename) >> >> def fixconfig(self, root=None, section=None): >> if section in (None, 'paths'):
On Mon, Mar 13, 2017 at 07:56:09PM -0700, Jun Wu wrote:
> This is a BC because loading order matters:
good catch. let's drop it until i find a better solution.
Patch
diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -160,6 +160,7 @@ self._colormode = None self._terminfoparams = {} self._styles = {} + self._readfilenames = set() if src: self.fout = src.fout @@ -258,6 +259,9 @@ def readconfig(self, filename, root=None, trust=False, sections=None, remap=None): + if filename in self._readfilenames: + return + try: fp = open(filename, u'rb') except IOError: @@ -304,6 +308,7 @@ if root is None: root = os.path.expanduser('~') self.fixconfig(root=root) + self._readfilenames.add(filename) def fixconfig(self, root=None, section=None): if section in (None, 'paths'):