Patchwork largefiles: improve repo wrapping detection

login
register
mail settings
Submitter Katsunori FUJIWARA
Date April 9, 2013, 5:36 p.m.
Message ID <89f37fa1de2fe25f6359.1365528987@juju>
Download mbox | patch
Permalink /patch/1266/
State Accepted
Commit 257afe5489d4a6ebca6a9e13bba1b69a9289b89d
Headers show

Comments

Katsunori FUJIWARA - April 9, 2013, 5:36 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1365528455 -32400
# Node ID 89f37fa1de2fe25f6359043e89adbffbd58274e4
# Parent  bd6aed2ad5eed666319bc3bdad1c1e37582cf4b1
largefiles: improve repo wrapping detection

Before this patch, repo wrapping detection in "reposetup()" of
largefiles can detect only limited repo wrapping: replacing target
functions by another one named as "wrap".

So, it can't detect repo wrapping even in recommended style: replacing
"__class__" of repo by derived class.

This patch can detect repo wrapping in both styles below:

  - replacing "__class__" of repo by derived class (recommended style):

        class derived(repo.__class__):
            def push(self, *args, **kwargs):
                return super(derived, self).push(*args, **kwargs)
        repo.__class__ = derived

  - replacing function of repo by another one (not recommended style):

        orgpush = repo.push
        def push(*args, **kwargs):
            return orgpush(*args, **kwargs)
        repo.push = push
Matt Mackall - April 9, 2013, 10:40 p.m.
On Wed, 2013-04-10 at 02:36 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1365528455 -32400
> # Node ID 89f37fa1de2fe25f6359043e89adbffbd58274e4
> # Parent  bd6aed2ad5eed666319bc3bdad1c1e37582cf4b1
> largefiles: improve repo wrapping detection

Is this http://bz.selenic.com/show_bug.cgi?id=3861 ?

> Before this patch, repo wrapping detection in "reposetup()" of
> largefiles can detect only limited repo wrapping: replacing target
> functions by another one named as "wrap".
> 
> So, it can't detect repo wrapping even in recommended style: replacing
> "__class__" of repo by derived class.
> 
> This patch can detect repo wrapping in both styles below:
> 
>   - replacing "__class__" of repo by derived class (recommended style):
> 
>         class derived(repo.__class__):
>             def push(self, *args, **kwargs):
>                 return super(derived, self).push(*args, **kwargs)
>         repo.__class__ = derived
> 
>   - replacing function of repo by another one (not recommended style):
> 
>         orgpush = repo.push
>         def push(*args, **kwargs):
>             return orgpush(*args, **kwargs)
>         repo.push = push
> 
> diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
> --- a/hgext/largefiles/reposetup.py
> +++ b/hgext/largefiles/reposetup.py
> @@ -27,10 +27,11 @@
>      if not repo.local():
>          return proto.wirereposetup(ui, repo)
>  
> +    origclass = localrepo.localrepository
> +    repoclass = repo.__class__
>      for name in ('status', 'commitctx', 'commit', 'push'):
> -        method = getattr(repo, name)
> -        if (isinstance(method, types.FunctionType) and
> -            method.func_name == 'wrap'):
> +        if (getattr(origclass, name) != getattr(repoclass, name) or
> +            isinstance(getattr(repo, name), types.FunctionType)):
>              ui.warn(_('largefiles: repo method %r appears to have already been'
>                      ' wrapped by another extension: '
>                      'largefiles may behave incorrectly\n')
> diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
> --- a/tests/test-largefiles.t
> +++ b/tests/test-largefiles.t
> @@ -180,6 +180,34 @@
>    $ cat sub/large4
>    large22
>  
> +Test repo method wrapping detection
> +
> +  $ cat > $TESTTMP/wrapping1.py <<EOF
> +  > from hgext import largefiles
> +  > def reposetup(ui, repo):
> +  >     class derived(repo.__class__):
> +  >         def push(self, *args, **kwargs):
> +  >             return super(derived, self).push(*args, **kwargs)
> +  >     repo.__class__ = derived
> +  >     largefiles.reposetup(ui, repo)
> +  > uisetup = largefiles.uisetup
> +  > EOF
> +  $ hg --config extensions.largefiles=$TESTTMP/wrapping1.py status
> +  largefiles: repo method 'push' appears to have already been wrapped by another extension: largefiles may behave incorrectly
> +
> +  $ cat > $TESTTMP/wrapping2.py <<EOF
> +  > from hgext import largefiles
> +  > def reposetup(ui, repo):
> +  >     orgpush = repo.push
> +  >     def push(*args, **kwargs):
> +  >         return orgpush(*args, **kwargs)
> +  >     repo.push = push
> +  >     largefiles.reposetup(ui, repo)
> +  > uisetup = largefiles.uisetup
> +  > EOF
> +  $ hg --config extensions.largefiles=$TESTTMP/wrapping2.py status
> +  largefiles: repo method 'push' appears to have already been wrapped by another extension: largefiles may behave incorrectly
> +
>  Test copies and moves from a directory other than root (issue3516)
>  
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Katsunori FUJIWARA - April 10, 2013, 11:26 a.m.
At Tue, 09 Apr 2013 17:40:30 -0500,
Matt Mackall wrote:
> 
> On Wed, 2013-04-10 at 02:36 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1365528455 -32400
> > # Node ID 89f37fa1de2fe25f6359043e89adbffbd58274e4
> > # Parent  bd6aed2ad5eed666319bc3bdad1c1e37582cf4b1
> > largefiles: improve repo wrapping detection
> 
> Is this http://bz.selenic.com/show_bug.cgi?id=3861 ?

No.

According to current rebase implementation, it doesn't wrap any repo
method but "hg pull" command.

This patch just detects repo wrapping, so this can't show any useful
hint in this case.

BTW, I found the cause of issue 3861 while confirming it, so I'll post
it to bugzilla.


> > Before this patch, repo wrapping detection in "reposetup()" of
> > largefiles can detect only limited repo wrapping: replacing target
> > functions by another one named as "wrap".
> > 
> > So, it can't detect repo wrapping even in recommended style: replacing
> > "__class__" of repo by derived class.
> > 
> > This patch can detect repo wrapping in both styles below:
> > 
> >   - replacing "__class__" of repo by derived class (recommended style):
> > 
> >         class derived(repo.__class__):
> >             def push(self, *args, **kwargs):
> >                 return super(derived, self).push(*args, **kwargs)
> >         repo.__class__ = derived
> > 
> >   - replacing function of repo by another one (not recommended style):
> > 
> >         orgpush = repo.push
> >         def push(*args, **kwargs):
> >             return orgpush(*args, **kwargs)
> >         repo.push = push
> > 
> > diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
> > --- a/hgext/largefiles/reposetup.py
> > +++ b/hgext/largefiles/reposetup.py
> > @@ -27,10 +27,11 @@
> >      if not repo.local():
> >          return proto.wirereposetup(ui, repo)
> >  
> > +    origclass = localrepo.localrepository
> > +    repoclass = repo.__class__
> >      for name in ('status', 'commitctx', 'commit', 'push'):
> > -        method = getattr(repo, name)
> > -        if (isinstance(method, types.FunctionType) and
> > -            method.func_name == 'wrap'):
> > +        if (getattr(origclass, name) != getattr(repoclass, name) or
> > +            isinstance(getattr(repo, name), types.FunctionType)):
> >              ui.warn(_('largefiles: repo method %r appears to have already been'
> >                      ' wrapped by another extension: '
> >                      'largefiles may behave incorrectly\n')
> > diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
> > --- a/tests/test-largefiles.t
> > +++ b/tests/test-largefiles.t
> > @@ -180,6 +180,34 @@
> >    $ cat sub/large4
> >    large22
> >  
> > +Test repo method wrapping detection
> > +
> > +  $ cat > $TESTTMP/wrapping1.py <<EOF
> > +  > from hgext import largefiles
> > +  > def reposetup(ui, repo):
> > +  >     class derived(repo.__class__):
> > +  >         def push(self, *args, **kwargs):
> > +  >             return super(derived, self).push(*args, **kwargs)
> > +  >     repo.__class__ = derived
> > +  >     largefiles.reposetup(ui, repo)
> > +  > uisetup = largefiles.uisetup
> > +  > EOF
> > +  $ hg --config extensions.largefiles=$TESTTMP/wrapping1.py status
> > +  largefiles: repo method 'push' appears to have already been wrapped by another extension: largefiles may behave incorrectly
> > +
> > +  $ cat > $TESTTMP/wrapping2.py <<EOF
> > +  > from hgext import largefiles
> > +  > def reposetup(ui, repo):
> > +  >     orgpush = repo.push
> > +  >     def push(*args, **kwargs):
> > +  >         return orgpush(*args, **kwargs)
> > +  >     repo.push = push
> > +  >     largefiles.reposetup(ui, repo)
> > +  > uisetup = largefiles.uisetup
> > +  > EOF
> > +  $ hg --config extensions.largefiles=$TESTTMP/wrapping2.py status
> > +  largefiles: repo method 'push' appears to have already been wrapped by another extension: largefiles may behave incorrectly
> > +
> >  Test copies and moves from a directory other than root (issue3516)
> >  
> >    $ cd ..
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
> 
> 
> -- 
> Mathematics is the supreme nostalgia of our time.
> 
> 
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
--- a/hgext/largefiles/reposetup.py
+++ b/hgext/largefiles/reposetup.py
@@ -27,10 +27,11 @@ 
     if not repo.local():
         return proto.wirereposetup(ui, repo)
 
+    origclass = localrepo.localrepository
+    repoclass = repo.__class__
     for name in ('status', 'commitctx', 'commit', 'push'):
-        method = getattr(repo, name)
-        if (isinstance(method, types.FunctionType) and
-            method.func_name == 'wrap'):
+        if (getattr(origclass, name) != getattr(repoclass, name) or
+            isinstance(getattr(repo, name), types.FunctionType)):
             ui.warn(_('largefiles: repo method %r appears to have already been'
                     ' wrapped by another extension: '
                     'largefiles may behave incorrectly\n')
diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
--- a/tests/test-largefiles.t
+++ b/tests/test-largefiles.t
@@ -180,6 +180,34 @@ 
   $ cat sub/large4
   large22
 
+Test repo method wrapping detection
+
+  $ cat > $TESTTMP/wrapping1.py <<EOF
+  > from hgext import largefiles
+  > def reposetup(ui, repo):
+  >     class derived(repo.__class__):
+  >         def push(self, *args, **kwargs):
+  >             return super(derived, self).push(*args, **kwargs)
+  >     repo.__class__ = derived
+  >     largefiles.reposetup(ui, repo)
+  > uisetup = largefiles.uisetup
+  > EOF
+  $ hg --config extensions.largefiles=$TESTTMP/wrapping1.py status
+  largefiles: repo method 'push' appears to have already been wrapped by another extension: largefiles may behave incorrectly
+
+  $ cat > $TESTTMP/wrapping2.py <<EOF
+  > from hgext import largefiles
+  > def reposetup(ui, repo):
+  >     orgpush = repo.push
+  >     def push(*args, **kwargs):
+  >         return orgpush(*args, **kwargs)
+  >     repo.push = push
+  >     largefiles.reposetup(ui, repo)
+  > uisetup = largefiles.uisetup
+  > EOF
+  $ hg --config extensions.largefiles=$TESTTMP/wrapping2.py status
+  largefiles: repo method 'push' appears to have already been wrapped by another extension: largefiles may behave incorrectly
+
 Test copies and moves from a directory other than root (issue3516)
 
   $ cd ..