Patchwork [4,of,7,v2] setup: short circuit copy_file for inplace

login
register
mail settings
Submitter timeless@mozdev.org
Date Jan. 12, 2016, 5:23 a.m.
Message ID <cffb330960bad95524d0.1452576186@waste.org>
Download mbox | patch
Permalink /patch/12674/
State Changes Requested
Headers show

Comments

timeless@mozdev.org - Jan. 12, 2016, 5:23 a.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1452572921 0
#      Tue Jan 12 04:28:41 2016 +0000
# Node ID cffb330960bad95524d0d170fdc7aea009e5b01d
# Parent  475a0e5bf82fc9c943323fa7045a133dfbeb38e2
setup: short circuit copy_file for inplace

copy_file doesn't do any work if setup is called for `local`,
this saves a lot of thought for that noop.
Yuya Nishihara - Jan. 15, 2016, 12:57 p.m.
On Mon, 11 Jan 2016 23:23:06 -0600, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1452572921 0
> #      Tue Jan 12 04:28:41 2016 +0000
> # Node ID cffb330960bad95524d0d170fdc7aea009e5b01d
> # Parent  475a0e5bf82fc9c943323fa7045a133dfbeb38e2
> setup: short circuit copy_file for inplace
> 
> copy_file doesn't do any work if setup is called for `local`,
> this saves a lot of thought for that noop.
> 
> diff --git a/setup.py b/setup.py
> --- a/setup.py
> +++ b/setup.py
> @@ -344,18 +344,22 @@
>                                   'Mercurial but weren\'t found in %s' % h)
>  
>      def copy_file(self, *args, **kwargs):
> -        dst, copied = build_py.copy_file(self, *args, **kwargs)
> +        src, dst = args[0:2]
> +        copied = False
> +        if './' + src != dst:
> +            dst, copied = build_py.copy_file(self, *args, **kwargs)
>  
> -        if copied and dst.endswith('mercurial/__init__.py'):
> +        if dst.endswith('mercurial/__init__.py'):
>              if self.distribution.pure:
>                  modulepolicy = 'py'
>              else:
>                  modulepolicy = 'c'
> -            content = open(dst, 'rb').read()
> -            content = content.replace(b'@MODULELOADPOLICY@',
> -                                      modulepolicy.encode(libdir_escape))
> -            with open(dst, 'wb') as fh:
> -                fh.write(content)
> +            if copied:
> +                content = open(dst, 'rb').read()
> +                content = content.replace(b'@MODULELOADPOLICY@',
> +                                          modulepolicy.encode(libdir_escape))
> +                with open(dst, 'wb') as fh:
> +                    fh.write(content)

I don't think it's good idea to duplicate things that are done right by
distutils. Instead, I would rather add a comment.

Patch

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -344,18 +344,22 @@ 
                                  'Mercurial but weren\'t found in %s' % h)
 
     def copy_file(self, *args, **kwargs):
-        dst, copied = build_py.copy_file(self, *args, **kwargs)
+        src, dst = args[0:2]
+        copied = False
+        if './' + src != dst:
+            dst, copied = build_py.copy_file(self, *args, **kwargs)
 
-        if copied and dst.endswith('mercurial/__init__.py'):
+        if dst.endswith('mercurial/__init__.py'):
             if self.distribution.pure:
                 modulepolicy = 'py'
             else:
                 modulepolicy = 'c'
-            content = open(dst, 'rb').read()
-            content = content.replace(b'@MODULELOADPOLICY@',
-                                      modulepolicy.encode(libdir_escape))
-            with open(dst, 'wb') as fh:
-                fh.write(content)
+            if copied:
+                content = open(dst, 'rb').read()
+                content = content.replace(b'@MODULELOADPOLICY@',
+                                          modulepolicy.encode(libdir_escape))
+                with open(dst, 'wb') as fh:
+                    fh.write(content)
 
         return dst, copied