Patchwork [1,of,6,RFC] localrepo: remove all internal uses of localrepo.opener

login
register
mail settings
Submitter Angel Ezquerra
Date Jan. 11, 2015, 1:20 a.m.
Message ID <459444107fc272ee7ca1.1420939250@Angels-MacBook-Pro.local>
Download mbox | patch
Permalink /patch/7428/
State Accepted
Commit 99e125626352c08d7e88c11f76ed37cc3a7dadd9
Headers show

Comments

Angel Ezquerra - Jan. 11, 2015, 1:20 a.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1420927372 -3600
#      Sat Jan 10 23:02:52 2015 +0100
# Node ID 459444107fc272ee7ca15cc9ac72602d60ed364f
# Parent  678f53865c6860a950392691814766957ee89316
localrepo: remove all internal uses of localrepo.opener

It has been replaced with localrepo.vfs. In the future we may split the vfs into
different vfs objects to access different elements of the repository.
Matt Harbison - Jan. 11, 2015, 2:58 a.m.
On Sat, 10 Jan 2015 20:20:50 -0500, Angel Ezquerra  
<angel.ezquerra@gmail.com> wrote:

> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra@gmail.com>
> # Date 1420927372 -3600
> #      Sat Jan 10 23:02:52 2015 +0100
> # Node ID 459444107fc272ee7ca15cc9ac72602d60ed364f
> # Parent  678f53865c6860a950392691814766957ee89316
> localrepo: remove all internal uses of localrepo.opener

The series LGTM.  The test suite on my Linux VM is happy with all 6  
applied, with the following skips:

Skipped test-convert-p4-filetypes.t: skipped
Skipped test-casecollision-merge.t: skipped
Skipped test-convert-baz.t: skipped
Skipped test-convert-cvs-synthetic.t: skipped
Skipped test-convert-p4.t: skipped
Skipped test-convert-tla.t: skipped
Skipped test-casefolding.t: skipped
Skipped test-no-symlinks.t: skipped
Skipped test-gpg.t: skipped

--Matt
Pierre-Yves David - Jan. 11, 2015, 6:12 a.m.
On 01/10/2015 06:58 PM, Matt Harbison wrote:
> On Sat, 10 Jan 2015 20:20:50 -0500, Angel Ezquerra
> <angel.ezquerra@gmail.com> wrote:
>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra@gmail.com>
>> # Date 1420927372 -3600
>> #      Sat Jan 10 23:02:52 2015 +0100
>> # Node ID 459444107fc272ee7ca15cc9ac72602d60ed364f
>> # Parent  678f53865c6860a950392691814766957ee89316
>> localrepo: remove all internal uses of localrepo.opener
>
> The series LGTM.  The test suite on my Linux VM is happy with all 6
> applied, with the following skips:

I'm not super eager to drop the opener version yet as this would break 
unsuspicious extension. I would go with a (new-feature) developper 
warning for a few version.
Angel Ezquerra - Jan. 11, 2015, 8:59 a.m.
On Sun, Jan 11, 2015 at 7:12 AM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 01/10/2015 06:58 PM, Matt Harbison wrote:
>>
>> On Sat, 10 Jan 2015 20:20:50 -0500, Angel Ezquerra
>> <angel.ezquerra@gmail.com> wrote:
>>
>>> # HG changeset patch
>>> # User Angel Ezquerra <angel.ezquerra@gmail.com>
>>> # Date 1420927372 -3600
>>> #      Sat Jan 10 23:02:52 2015 +0100
>>> # Node ID 459444107fc272ee7ca15cc9ac72602d60ed364f
>>> # Parent  678f53865c6860a950392691814766957ee89316
>>> localrepo: remove all internal uses of localrepo.opener
>>
>>
>> The series LGTM.  The test suite on my Linux VM is happy with all 6
>> applied, with the following skips:
>
>
> I'm not super eager to drop the opener version yet as this would break
> unsuspicious extension. I would go with a (new-feature) developper warning
> for a few version.
>
> --
> Pierre-Yves David

OK. Do we have a standard way of doing that?

Angel
Angel Ezquerra - Jan. 11, 2015, 9:02 a.m.
On Sun, Jan 11, 2015 at 9:59 AM, Angel Ezquerra
<angel.ezquerra@gmail.com> wrote:
> On Sun, Jan 11, 2015 at 7:12 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>>
>>
>> On 01/10/2015 06:58 PM, Matt Harbison wrote:
>>>
>>> On Sat, 10 Jan 2015 20:20:50 -0500, Angel Ezquerra
>>> <angel.ezquerra@gmail.com> wrote:
>>>
>>>> # HG changeset patch
>>>> # User Angel Ezquerra <angel.ezquerra@gmail.com>
>>>> # Date 1420927372 -3600
>>>> #      Sat Jan 10 23:02:52 2015 +0100
>>>> # Node ID 459444107fc272ee7ca15cc9ac72602d60ed364f
>>>> # Parent  678f53865c6860a950392691814766957ee89316
>>>> localrepo: remove all internal uses of localrepo.opener
>>>
>>>
>>> The series LGTM.  The test suite on my Linux VM is happy with all 6
>>> applied, with the following skips:
>>
>>
>> I'm not super eager to drop the opener version yet as this would break
>> unsuspicious extension. I would go with a (new-feature) developper warning
>> for a few version.
>>
>> --
>> Pierre-Yves David
>
> OK. Do we have a standard way of doing that?
>
> Angel

Also, would it be ok if I just resend the series without the removal
of the localrepository opener, sopener and wopener properties (i.e.
still keeping the aliases, but removing all our usages)?

Cheers,

Angel
Augie Fackler - Jan. 12, 2015, 10:14 p.m.
On Sat, Jan 10, 2015 at 10:12:52PM -0800, Pierre-Yves David wrote:
>
>
> On 01/10/2015 06:58 PM, Matt Harbison wrote:
> >On Sat, 10 Jan 2015 20:20:50 -0500, Angel Ezquerra
> ><angel.ezquerra@gmail.com> wrote:
> >
> >># HG changeset patch
> >># User Angel Ezquerra <angel.ezquerra@gmail.com>
> >># Date 1420927372 -3600
> >>#      Sat Jan 10 23:02:52 2015 +0100
> >># Node ID 459444107fc272ee7ca15cc9ac72602d60ed364f
> >># Parent  678f53865c6860a950392691814766957ee89316
> >>localrepo: remove all internal uses of localrepo.opener
> >
> >The series LGTM.  The test suite on my Linux VM is happy with all 6
> >applied, with the following skips:
>
> I'm not super eager to drop the opener version yet as this would break
> unsuspicious extension. I would go with a (new-feature) developper warning
> for a few version.

On the other hand, as the likely author of such extensions, I'd be a
huge fan of just ripping the band-aid off now.

>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - Jan. 14, 2015, 10:28 p.m.
On Sun, 2015-01-11 at 09:59 +0100, Angel Ezquerra wrote:
> On Sun, Jan 11, 2015 at 7:12 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
> >
> >
> > On 01/10/2015 06:58 PM, Matt Harbison wrote:
> >>
> >> On Sat, 10 Jan 2015 20:20:50 -0500, Angel Ezquerra
> >> <angel.ezquerra@gmail.com> wrote:
> >>
> >>> # HG changeset patch
> >>> # User Angel Ezquerra <angel.ezquerra@gmail.com>
> >>> # Date 1420927372 -3600
> >>> #      Sat Jan 10 23:02:52 2015 +0100
> >>> # Node ID 459444107fc272ee7ca15cc9ac72602d60ed364f
> >>> # Parent  678f53865c6860a950392691814766957ee89316
> >>> localrepo: remove all internal uses of localrepo.opener
> >>
> >>
> >> The series LGTM.  The test suite on my Linux VM is happy with all 6
> >> applied, with the following skips:
> >
> >
> > I'm not super eager to drop the opener version yet as this would break
> > unsuspicious extension. I would go with a (new-feature) developper warning
> > for a few version.
> >
> > --
> > Pierre-Yves David
> 
> OK. Do we have a standard way of doing that?

Yep: mark the patch that removes the feature with (API) and then chuckle
quietly to yourself.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -325,7 +325,7 @@ 
             self.sopener.options['maxchainlen'] = maxchainlen
 
     def _writerequirements(self):
-        reqfile = self.opener("requires", "w")
+        reqfile = self.vfs("requires", "w")
         for r in sorted(self.requirements):
             reqfile.write("%s\n" % r)
         reqfile.close()
@@ -449,7 +449,7 @@ 
                                    " working parent %s!\n") % short(node))
                 return nullid
 
-        return dirstate.dirstate(self.opener, self.ui, self.root, validate)
+        return dirstate.dirstate(self.vfs, self.ui, self.root, validate)
 
     def __getitem__(self, changeid):
         if changeid is None:
@@ -531,9 +531,9 @@ 
         prevtags = ''
         if local:
             try:
-                fp = self.opener('localtags', 'r+')
+                fp = self.vfs('localtags', 'r+')
             except IOError:
-                fp = self.opener('localtags', 'a')
+                fp = self.vfs('localtags', 'a')
             else:
                 prevtags = fp.read()
 
@@ -909,7 +909,7 @@ 
         self._writejournal(desc)
         renames = [(vfs, x, undoname(x)) for vfs, x in self._journalfiles()]
         rp = report and report or self.ui.warn
-        vfsmap = {'plain': self.opener} # root of .hg/
+        vfsmap = {'plain': self.vfs} # root of .hg/
         tr = transaction.transaction(rp, self.sopener, vfsmap,
                                      "journal",
                                      aftertrans(renames),
@@ -933,14 +933,14 @@ 
         return [(vfs, undoname(x)) for vfs, x in self._journalfiles()]
 
     def _writejournal(self, desc):
-        self.opener.write("journal.dirstate",
-                          self.opener.tryread("dirstate"))
-        self.opener.write("journal.branch",
+        self.vfs.write("journal.dirstate",
+                          self.vfs.tryread("dirstate"))
+        self.vfs.write("journal.branch",
                           encoding.fromlocal(self.dirstate.branch()))
-        self.opener.write("journal.desc",
+        self.vfs.write("journal.desc",
                           "%d\n%s\n" % (len(self), desc))
-        self.opener.write("journal.bookmarks",
-                          self.opener.tryread("bookmarks"))
+        self.vfs.write("journal.bookmarks",
+                          self.vfs.tryread("bookmarks"))
         self.sopener.write("journal.phaseroots",
                            self.sopener.tryread("phaseroots"))
 
@@ -950,7 +950,7 @@ 
             if self.svfs.exists("journal"):
                 self.ui.status(_("rolling back interrupted transaction\n"))
                 vfsmap = {'': self.sopener,
-                          'plain': self.opener,}
+                          'plain': self.vfs,}
                 transaction.rollback(self.sopener, vfsmap, "journal",
                                      self.ui.warn)
                 self.invalidate()
@@ -978,7 +978,7 @@ 
     def _rollback(self, dryrun, force):
         ui = self.ui
         try:
-            args = self.opener.read('undo.desc').splitlines()
+            args = self.vfs.read('undo.desc').splitlines()
             (oldlen, desc, detail) = (int(args[0]), args[1], None)
             if len(args) >= 3:
                 detail = args[2]
@@ -1007,7 +1007,7 @@ 
 
         parents = self.dirstate.parents()
         self.destroying()
-        vfsmap = {'plain': self.opener}
+        vfsmap = {'plain': self.vfs}
         transaction.rollback(self.sopener, vfsmap, 'undo', ui.warn)
         if self.vfs.exists('undo.bookmarks'):
             self.vfs.rename('undo.bookmarks', 'bookmarks')
@@ -1020,7 +1020,7 @@ 
         if parentgone:
             self.vfs.rename('undo.dirstate', 'dirstate')
             try:
-                branch = self.opener.read('undo.branch')
+                branch = self.vfs.read('undo.branch')
                 self.dirstate.setbranch(encoding.tolocal(branch))
             except IOError:
                 ui.warn(_('named branch could not be reset: '
@@ -1822,7 +1822,7 @@ 
         return "%s %s %s %s %s" % (one, two, three, four, five)
 
     def savecommitmessage(self, text):
-        fp = self.opener('last-message.txt', 'wb')
+        fp = self.vfs('last-message.txt', 'wb')
         try:
             fp.write(text)
         finally: