Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

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.

Description

Issue 2744 - Install correct version of JS shell for 64bit Linux

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed feedback #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -7 lines) Patch
M utils.py View 1 2 chunks +20 lines, -7 lines 3 comments Download

Messages

Total messages: 9
kzar
Patch Set 1
July 2, 2015, 4:59 p.m. (2015-07-02 16:59:59 UTC) #1
Wladimir Palant
I'm pretty sure that this isn't solving the right issue - 32 bit JS shell ...
July 2, 2015, 5:34 p.m. (2015-07-02 17:34:14 UTC) #2
Wladimir Palant
For reference, the system libraries used by the JS shell are: libpthread.so.0 libdl.so.2 libstdc++.so.6 libm.so.6 ...
July 2, 2015, 5:48 p.m. (2015-07-02 17:48:30 UTC) #3
Sebastian Noack
I remember that until a few years ago you couldn't actually run 32bit executable on ...
July 2, 2015, 9:49 p.m. (2015-07-02 21:49:51 UTC) #4
kzar
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, ...
July 3, 2015, 1:08 p.m. (2015-07-03 13:08:51 UTC) #5
Wladimir Palant
LGTM
July 3, 2015, 1:33 p.m. (2015-07-03 13:33:25 UTC) #6
Sebastian Noack
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 = { ...
July 3, 2015, 2:31 p.m. (2015-07-03 14:31:45 UTC) #7
kzar
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: > ...
July 3, 2015, 2:37 p.m. (2015-07-03 14:37:22 UTC) #8
Sebastian Noack
July 3, 2015, 2:41 p.m. (2015-07-03 14:41:34 UTC) #9
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.

Powered by Google App Engine
This is Rietveld