Patchwork [stable] largefiles: drop repo wrapping detection

login
register
mail settings
Submitter Mads Kiilerich
Date April 26, 2013, 5:04 p.m.
Message ID <c6e31b698ee21631c1ae.1366995891@xps>
Download mbox | patch
Permalink /patch/1487/
State Accepted
Commit ce4472b2edb2212f026b61ca829753347f03ab1b
Headers show

Comments

Mads Kiilerich - April 26, 2013, 5:04 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1366995841 -7200
#      Fri Apr 26 19:04:01 2013 +0200
# Branch stable
# Node ID c6e31b698ee21631c1aeb8d14e6f7c5836bbb3c3
# Parent  be207d9b7e4bc222f4ba72ad9a266df83d939ca4
largefiles: drop repo wrapping detection

After 257afe5489d4 I see:

  $ hg id -q
  largefiles: repo method 'commit' appears to have already been wrapped by another extension: largefiles may behave incorrectly
  largefiles: repo method 'push' appears to have already been wrapped by another extension: largefiles may behave incorrectly
  be207d9b7e4b

The warning is bad:

* The message gives no hint what the problem is and how it can be resolved.
  The message is useless.

* Largefiles do have its share of problems, but I don't think I ever have seen
  a problem where this warning would have helped. The 'may' in the warning
  seems like an exaggeration of the risk. Having largefiles enabled in
  combination with for instance mq, hggit and hgsubversion causes a warning
  (depending on the configuration order) but do not cause problems. Extensions
  might of course be incompatible, but they can be that in many other ways.
  The check and the message are incorrect.

It would thus be better to remove the check and the warning completely.

Before 257afe5489d4 the check always failed. That change made the check work
more like intended ... but the intention was wrong. This change will thus also
back that change out.
Matt Mackall - April 26, 2013, 5:22 p.m.
On Fri, 2013-04-26 at 19:04 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1366995841 -7200
> #      Fri Apr 26 19:04:01 2013 +0200
> # Branch stable
> # Node ID c6e31b698ee21631c1aeb8d14e6f7c5836bbb3c3
> # Parent  be207d9b7e4bc222f4ba72ad9a266df83d939ca4
> largefiles: drop repo wrapping detection

Good to cc: its author. We still have an outstanding issue here with
rebase vs largefiles, yes?

> After 257afe5489d4 I see:
> 
>   $ hg id -q
>   largefiles: repo method 'commit' appears to have already been wrapped by another extension: largefiles may behave incorrectly
>   largefiles: repo method 'push' appears to have already been wrapped by another extension: largefiles may behave incorrectly
>   be207d9b7e4b
> 
> The warning is bad:
> 
> * The message gives no hint what the problem is and how it can be resolved.
>   The message is useless.
> 
> * Largefiles do have its share of problems, but I don't think I ever have seen
>   a problem where this warning would have helped. The 'may' in the warning
>   seems like an exaggeration of the risk. Having largefiles enabled in
>   combination with for instance mq, hggit and hgsubversion causes a warning
>   (depending on the configuration order) but do not cause problems. Extensions
>   might of course be incompatible, but they can be that in many other ways.
>   The check and the message are incorrect.
> 
> It would thus be better to remove the check and the warning completely.
> 
> Before 257afe5489d4 the check always failed. That change made the check work
> more like intended ... but the intention was wrong. This change will thus also
> back that change out.
> 
> diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
> --- a/hgext/largefiles/reposetup.py
> +++ b/hgext/largefiles/reposetup.py
> @@ -8,7 +8,6 @@
>  
>  '''setup for largefiles repositories: reposetup'''
>  import copy
> -import types
>  import os
>  
>  from mercurial import context, error, manifest, match as match_, util, \
> @@ -27,16 +26,6 @@ def reposetup(ui, repo):
>      if not repo.local():
>          return proto.wirereposetup(ui, repo)
>  
> -    origclass = localrepo.localrepository
> -    repoclass = repo.__class__
> -    for name in ('status', 'commitctx', 'commit', 'push'):
> -        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')
> -                    % name)
> -
>      class lfilesrepo(repo.__class__):
>          lfstatus = False
>          def status_nolfiles(self, *args, **kwargs):
> diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
> --- a/tests/test-largefiles.t
> +++ b/tests/test-largefiles.t
> @@ -180,34 +180,6 @@ Test moving largefiles and verify that n
>    $ 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
Mads Kiilerich - April 26, 2013, 5:50 p.m.
On 04/26/2013 07:22 PM, Matt Mackall wrote:
> On Fri, 2013-04-26 at 19:04 +0200, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1366995841 -7200
>> #      Fri Apr 26 19:04:01 2013 +0200
>> # Branch stable
>> # Node ID c6e31b698ee21631c1aeb8d14e6f7c5836bbb3c3
>> # Parent  be207d9b7e4bc222f4ba72ad9a266df83d939ca4
>> largefiles: drop repo wrapping detection
> Good to cc: its author.

Just to clarify: The warning has "always" been there ... it just didn't 
work. foozy was not the author - he just fixed it.

> We still have an outstanding issue here with
> rebase vs largefiles, yes?

Yes, but that is unrelated. Combining largefiles with rebase did not 
trigger this warning anyway.

/Mads
Matt Mackall - April 27, 2013, 5:17 p.m.
On Fri, 2013-04-26 at 19:04 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1366995841 -7200
> #      Fri Apr 26 19:04:01 2013 +0200
> # Branch stable
> # Node ID c6e31b698ee21631c1aeb8d14e6f7c5836bbb3c3
> # Parent  be207d9b7e4bc222f4ba72ad9a266df83d939ca4
> largefiles: drop repo wrapping detection

Ok, queued for stable, thanks.

Patch

diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
--- a/hgext/largefiles/reposetup.py
+++ b/hgext/largefiles/reposetup.py
@@ -8,7 +8,6 @@ 
 
 '''setup for largefiles repositories: reposetup'''
 import copy
-import types
 import os
 
 from mercurial import context, error, manifest, match as match_, util, \
@@ -27,16 +26,6 @@  def reposetup(ui, repo):
     if not repo.local():
         return proto.wirereposetup(ui, repo)
 
-    origclass = localrepo.localrepository
-    repoclass = repo.__class__
-    for name in ('status', 'commitctx', 'commit', 'push'):
-        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')
-                    % name)
-
     class lfilesrepo(repo.__class__):
         lfstatus = False
         def status_nolfiles(self, *args, **kwargs):
diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
--- a/tests/test-largefiles.t
+++ b/tests/test-largefiles.t
@@ -180,34 +180,6 @@  Test moving largefiles and verify that n
   $ 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 ..