Issue 29321352: Issue 2744 - Install correct version of JS shell for 64bit Linux (Closed)
|
Created:
July 2, 2015, 4:57 p.m. by kzar Modified:
July 6, 2015, 11:05 a.m. CC:
Thomas Greiner Visibility:
Public. |
DescriptionIssue 2744 - Install correct version of JS shell for 64bit Linux
Patch Set 1 #
Total comments: 4
Patch Set 2 : Addressed feedback #
Total comments: 3
MessagesTotal messages: 9
Patch Set 1
I'm pretty sure that this isn't solving the right issue - 32 bit JS shell was definitely working correctly on 64 bit Linux. Maybe some 32 bit library is missing? That also matches the error message you are getting.
For reference, the system libraries used by the JS shell are: libpthread.so.0 libdl.so.2 libstdc++.so.6 libm.so.6 libc.so.6 ld-linux.so.2 These are pretty certain to exist on any system having at least one x86 application installed. However, I guess that running the x64 build is indeed guaranteed to succeed - whereas the x86 build might fail under some rare conditions. https://codereview.adblockplus.org/29321352/diff/29321353/utils.py File utils.py (right): https://codereview.adblockplus.org/29321352/diff/29321353/utils.py#newcode7 utils.py:7: import os, platform, sys, urllib, zipfile Feel free to convert this to one per line while at it. https://codereview.adblockplus.org/29321352/diff/29321353/utils.py#newcode33 utils.py:33: build = build[platform.architecture()[0]] This will give you the architecture of the Python binary, not necessarily the platform architecture. I guess that you need to use platform.machine().
I remember that until a few years ago you couldn't actually run 32bit executable on 64bit Debian, at least not any code that relied on any library. They first had to add multiarch support, to manage 32bit and 64bit versions of libraries simultaneously. Other distributions were or might still be affected as well.
Patch Set 2 : Addressed feedback https://codereview.adblockplus.org/29321352/diff/29321353/utils.py File utils.py (right): https://codereview.adblockplus.org/29321352/diff/29321353/utils.py#newcode7 utils.py:7: import os, platform, sys, urllib, zipfile On 2015/07/02 17:48:31, Wladimir Palant wrote: > Feel free to convert this to one per line while at it. Done. https://codereview.adblockplus.org/29321352/diff/29321353/utils.py#newcode33 utils.py:33: build = build[platform.architecture()[0]] On 2015/07/02 17:48:31, Wladimir Palant wrote: > This will give you the architecture of the Python binary, not necessarily the > platform architecture. I guess that you need to use platform.machine(). Done.
LGTM
https://codereview.adblockplus.org/29321352/diff/29321402/utils.py File utils.py (right): https://codereview.adblockplus.org/29321352/diff/29321402/utils.py#newcode28 utils.py:28: 'linux2': { How about following? supported_platforms = { ... 'linux2': 'linux-%(arch)s' ... } ... build = supported_platforms[sys.platform] % {arch: platform.machine()}
https://codereview.adblockplus.org/29321352/diff/29321402/utils.py File utils.py (right): https://codereview.adblockplus.org/29321352/diff/29321402/utils.py#newcode28 utils.py:28: 'linux2': { On 2015/07/03 14:31:44, Sebastian Noack wrote: > How about following? > > supported_platforms = { > ... > 'linux2': 'linux-%(arch)s' > ... > } > ... > > build = supported_platforms[sys.platform] % {arch: platform.machine()} I did consider doing something like that but I think it's better to explicitly define the supported architectures. There aren't any other Linux builds of JS shell available.
https://codereview.adblockplus.org/29321352/diff/29321402/utils.py File utils.py (right): https://codereview.adblockplus.org/29321352/diff/29321402/utils.py#newcode28 utils.py:28: 'linux2': { On 2015/07/03 14:37:22, kzar wrote: > On 2015/07/03 14:31:44, Sebastian Noack wrote: > > How about following? > > > > supported_platforms = { > > ... > > 'linux2': 'linux-%(arch)s' > > ... > > } > > ... > > > > build = supported_platforms[sys.platform] % {arch: platform.machine()} > > I did consider doing something like that but I think it's better to explicitly > define the supported architectures. There aren't any other Linux builds of JS > shell available. This is a good point. However, I don't like introducing a special case here, but don't have a better idea right now. So LGTM. 
|
