Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api to get arm architecture of the binary as in the download page... #7803

Closed
cswl opened this issue Jul 20, 2016 · 16 comments
Closed

api to get arm architecture of the binary as in the download page... #7803

cswl opened this issue Jul 20, 2016 · 16 comments
Labels
feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. process Issues and PRs related to the process subsystem.

Comments

@cswl
Copy link

cswl commented Jul 20, 2016

  • Version: v6.3.0
  • Platform: linux-arm*
  • Subsystem: API?

I couldn't find any api to get the architecture of the binary itself,

process.arch just gives arm
uname -a would give for the host.. not the binary
node --v8-options gives target arm v7.. , and a lot of output..

I want it for the the binary as it is on the download page.. arm64 armv6l armv7l

I dumped and greped the process object and found.. process.config.variables.arm_version among others...
Checking the api doc for process.config says it is not read-only and gives some warning about some packages changing it..

Could and api be added for this, or should we just parse this form process.config.variables.arm_* or --v8-options or any other suggestions..

@mscdex
Copy link
Contributor

mscdex commented Jul 20, 2016

If you don't mind using child processes, you could always use file or readelf -h (if you know it's ELF) to get the same information.

@mscdex mscdex added the question Issues that look for answers. label Jul 20, 2016
@Fishrock123
Copy link
Contributor

process.config.variables.arm_version is correct.

Also, none of process is read-only, so yeah.

@Fishrock123 Fishrock123 added the process Issues and PRs related to the process subsystem. label Jul 20, 2016
@cswl
Copy link
Author

cswl commented Jul 20, 2016

test/common.js seems to use process.config.variables.arm_version

node/test/common.js

Lines 266 to 277 in 5aac4c4

if (process.arch !== 'arm')
return ms;
const armv = process.config.variables.arm_version;
if (armv === '6')
return 7 * ms; // ARMv6
if (armv === '7')
return 2 * ms; // ARMv7
return ms; // ARMv8+

But there is something strange, node.js armv7l binaries from https://nodejs.org/download gives 6 for the process.config.variables.arm_version.

/tmp/node-v6.3.0-linux-armv7l > ./bin/node -p process.config.variables.arm_version
6

/tmp/node-v7.0.0-nightly20160719f56cd32c70-linux-armv7l > ./bin/node -p process.config.variables.arm_version
6

The output from --v8-options | head are similar..

target arm v6 vfp2 hard
ARMv8=0 ARMv7=1 VFP3=1 VFP32DREGS=1 NEON=1 SUDIV=0 MLS=1UNALIGNED_ACCESSES=1 MOVW_MOVT_IMMEDIATE_LOADS=0 COHERENT_CACHE=0 USE_EABI_HARDFLOAT=1

But on distribution packaged one....

node -p process.config.variables.arm_version 
7
target arm v7 vfp3-d16 hard
ARMv8=0 ARMv7=1 VFP3=1 VFP32DREGS=1 NEON=1 SUDIV=0 MLS=1UNALIGNED_ACCESSES=1 MOVW_MOVT_IMMEDIATE_LOADS=0 COHERENT_CACHE=0 USE_EABI_HARDFLOAT=1

@Fishrock123
Copy link
Contributor

cc @nodejs/build maybe?

@bnoordhuis
Copy link
Member

I believe we do that to maximize compatibility with boards that are only nominally ARMv7-compatible, like the rpi1.

Code quality-wise it shouldn't matter too much because V8 detects the architecture at run-time. OpenSSL may be negatively affected but I'm not 100% sure about that, I think it does run-time feature detection as well.

@xbolshe
Copy link

xbolshe commented Jul 27, 2016

FYI:

we get #define __ARM_ARCH 6 from the compiler...

#4531 (comment)

PS: my compilation shows correct values:
arm7_rp2

@Trott
Copy link
Member

Trott commented Jul 15, 2017

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.

@Trott Trott closed this as completed Jul 15, 2017
@piranna
Copy link
Contributor

piranna commented Jul 15, 2017

I'm still interested on this feature. In fact, at prebuild/prebuild#174 we were discussing about using arm as an alias the same way x86 is an alias for i686, but we would need to know what aliased. Please reopen this for discussion.

@Trott Trott reopened this Jul 15, 2017
@Trott
Copy link
Member

Trott commented Jul 15, 2017

Re-opened as requested.

@piranna
Copy link
Contributor

piranna commented Jul 15, 2017

Thank you.

@Trott
Copy link
Member

Trott commented Aug 10, 2017

Same question I just posted to #4531: Is anyone in a good position to move this issue forward? Or should we add a stalled and/or help wanted label? Or something else?

@Trott Trott added feature request Issues that request new features to be added to Node.js. stalled Issues and PRs that are stalled. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. and removed question Issues that look for answers. stalled Issues and PRs that are stalled. labels Nov 8, 2018
@Trott
Copy link
Member

Trott commented Nov 8, 2018

I wonder if there would be resistance to adding more stuff to process in general? (I assume that's where this requested property would live.)

@rvagg
Copy link
Member

rvagg commented Nov 9, 2018

FYI the arm version should be fixed for Node 10+ since we're properly cross compiling with our own custom toolchain now and it's working really well. That's not the focus of this issue but it's relevant to some of the discussion.

I think I'd be OK with a process.arch_variant or something similar, that would be null on most platforms. I'm not sure an arm-specific property is a great idea. I still advise people to use process.config.variables.arm_version and don't really think there's a problem continuing to suggest that. 🤷‍♂️

@BridgeAR
Copy link
Member

BridgeAR commented Jan 2, 2020

Wouldn't an easy solution be to make process.config.variables.arm_version read only?

@jasnell
Copy link
Member

jasnell commented Jan 2, 2020

@BridgeAR ... there are modules in the ecosystem that completely override the value of process.config under the assumption that it's world writable. Fun fun fun.

@BridgeAR
Copy link
Member

BridgeAR commented Jan 2, 2020

Do you know the reason they override process.config? We could allow extending process.config but disallow overriding it and prohibit changing existing properties.

jasnell added a commit to jasnell/node that referenced this issue Jan 12, 2021
The fact that `process.config` is mutable has long made it
unreliable when it really should just work. Start the process
of deprecating the ability to change it.

Fixes: nodejs#7803
Signed-off-by: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants