Patchwork [2,of,2,V2] setup.py: don't rewrite @LIBDIR@ when creating wheels

login
register
mail settings
Submitter Gregory Szorc
Date Dec. 6, 2015, 10:18 p.m.
Message ID <43a7531c00902c4be3ac.1449440310@ubuntu-main>
Download mbox | patch
Permalink /patch/11875/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Gregory Szorc - Dec. 6, 2015, 10:18 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1449366770 28800
#      Sat Dec 05 17:52:50 2015 -0800
# Node ID 43a7531c00902c4be3acab314d1fbb24136a047e
# Parent  b47041dfe62151571ed6a9d8e9271b3924a2d80f
setup.py: don't rewrite @LIBDIR@ when creating wheels

This is necessary to produce wheels that install properly. More
details are captured in an in-line comment.

After this patch, produced wheels can be installed via `pip install`
and appear to "just work," including on Windows.
Yuya Nishihara - Dec. 7, 2015, 1:38 p.m.
On Sun, 06 Dec 2015 14:18:30 -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1449366770 28800
> #      Sat Dec 05 17:52:50 2015 -0800
> # Node ID 43a7531c00902c4be3acab314d1fbb24136a047e
> # Parent  b47041dfe62151571ed6a9d8e9271b3924a2d80f

Pushed to the clowncopter, thanks.

>              if b('\0') in data:
>                  continue
>  
> +            # During local installs, the shebang will be rewritten to the final
> +            # install path. During wheel packaging, the shebang has a special
> +            # value.
> +            if data.startswith(b'#!python'):

I noticed we use b(...) in setup.py, but it won't be necessary since we've
dropped support for Python < 2.6.
Gregory Szorc - Dec. 7, 2015, 10:39 p.m.
> On Dec 7, 2015, at 08:38, Yuya Nishihara <yuya@tcha.org> wrote:
> 
>> On Sun, 06 Dec 2015 14:18:30 -0800, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1449366770 28800
>> #      Sat Dec 05 17:52:50 2015 -0800
>> # Node ID 43a7531c00902c4be3acab314d1fbb24136a047e
>> # Parent  b47041dfe62151571ed6a9d8e9271b3924a2d80f
> 
> Pushed to the clowncopter, thanks.
> 
>>             if b('\0') in data:
>>                 continue
>> 
>> +            # During local installs, the shebang will be rewritten to the final
>> +            # install path. During wheel packaging, the shebang has a special
>> +            # value.
>> +            if data.startswith(b'#!python'):
> 
> I noticed we use b(...) in setup.py, but it won't be necessary since we've
> dropped support for Python < 2.6.

I think this is for Python 3 compatibility. Without it, we'd be comparing a bytes wth a str, which isn't allowed.
Yuya Nishihara - Dec. 8, 2015, 12:40 p.m.
On Mon, 7 Dec 2015 17:39:58 -0500, Gregory Szorc wrote:
> > On Dec 7, 2015, at 08:38, Yuya Nishihara <yuya@tcha.org> wrote:
> > 
> >> On Sun, 06 Dec 2015 14:18:30 -0800, Gregory Szorc wrote:
> >> # HG changeset patch
> >> # User Gregory Szorc <gregory.szorc@gmail.com>
> >> # Date 1449366770 28800
> >> #      Sat Dec 05 17:52:50 2015 -0800
> >> # Node ID 43a7531c00902c4be3acab314d1fbb24136a047e
> >> # Parent  b47041dfe62151571ed6a9d8e9271b3924a2d80f
> > 
> > Pushed to the clowncopter, thanks.
> > 
> >>             if b('\0') in data:
> >>                 continue
> >> 
> >> +            # During local installs, the shebang will be rewritten to the final
> >> +            # install path. During wheel packaging, the shebang has a special
> >> +            # value.
> >> +            if data.startswith(b'#!python'):
> > 
> > I noticed we use b(...) in setup.py, but it won't be necessary since we've
> > dropped support for Python < 2.6.
> 
> I think this is for Python 3 compatibility. Without it, we'd be comparing
> a bytes wth a str, which isn't allowed.

Yes, and we can use b'...' instead of b(...) now.

Patch

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -476,16 +476,35 @@  class hginstallscripts(install_scripts):
     def finalize_options(self):
         install_scripts.finalize_options(self)
         self.set_undefined_options('install',
                                    ('install_lib', 'install_lib'))
 
     def run(self):
         install_scripts.run(self)
 
+        # It only makes sense to replace @LIBDIR@ with the install path if
+        # the install path is known. For wheels, the logic below calculates
+        # the libdir to be "../..". This is because the internal layout of a
+        # wheel archive looks like:
+        #
+        #   mercurial-3.6.1.data/scripts/hg
+        #   mercurial/__init__.py
+        #
+        # When installing wheels, the subdirectories of the "<pkg>.data"
+        # directory are translated to system local paths and files therein
+        # are copied in place. The mercurial/* files are installed into the
+        # site-packages directory. However, the site-packages directory
+        # isn't known until wheel install time. This means we have no clue
+        # at wheel generation time what the installed site-packages directory
+        # will be. And, wheels don't appear to provide the ability to register
+        # custom code to run during wheel installation. This all means that
+        # we can't reliably set the libdir in wheels: the default behavior
+        # of looking in sys.path must do.
+
         if (os.path.splitdrive(self.install_dir)[0] !=
             os.path.splitdrive(self.install_lib)[0]):
             # can't make relative paths from one drive to another, so use an
             # absolute path instead
             libdir = self.install_lib
         else:
             common = os.path.commonprefix((self.install_dir, self.install_lib))
             rest = self.install_dir[len(common):]
@@ -497,16 +516,24 @@  class hginstallscripts(install_scripts):
             fp = open(outfile, 'rb')
             data = fp.read()
             fp.close()
 
             # skip binary files
             if b('\0') in data:
                 continue
 
+            # During local installs, the shebang will be rewritten to the final
+            # install path. During wheel packaging, the shebang has a special
+            # value.
+            if data.startswith(b'#!python'):
+                log.info('not rewriting @LIBDIR@ in %s because install path '
+                         'not known' % outfile)
+                continue
+
             data = data.replace(b('@LIBDIR@'), libdir.encode(libdir_escape))
             fp = open(outfile, 'wb')
             fp.write(data)
             fp.close()
 
 cmdclass = {'build': hgbuild,
             'build_mo': hgbuildmo,
             'build_ext': hgbuildext,