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

adding support for linux ppc64le #862

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adilhusain-s
Copy link

Description:
*Adding change to support ppc64le architecture

  • since actions/node-versions doesn't have binaries for ppc64le architecture, falling back to official node binaries.

Please review and merge.

Related issue:
Add link to the related issue.

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@adilhusain-s adilhusain-s requested a review from a team as a code owner October 3, 2023 07:12
@adilhusain-s
Copy link
Author

testing performed

  • created a sample workflow and ran it on ppc64le architecture
  • workflow

root@adilhusain-centos-test-1:/actions-runner# uname -a
Linux adilhusain-centos-test-1.power-iaas.cloud.ibm.com 4.18.0-408.el8.ppc64le #1 SMP Mon Jul 18 16:06:55 UTC 2022 ppc64le ppc64le ppc64le GNU/Linux
root@adilhusain-centos-test-1:
/actions-runner# node
Welcome to Node.js v20.6.1.
Type ".help" for more information.

os.arch()
'ppc64'
os.endianness()
'LE'

@dmitry-shibanov
Copy link
Contributor

Hello @adilhusain-s. Thank you for your pull request. Could you please take a look at this comment about our concerns for these changes?

@adilhusain-s
Copy link
Author

@dmitry-shibanov
Thanks for the comment,
I did see those comment which you provided on old PR.
I'm not sure if we should actually add support for iax-ppc64 for following reasons:

  • self-hosted github runner isn't supported on iax-pp64 or ported for this platform (as I know)
  • not sure iax-ppc64 is still widely in use. the official docker node or busybox image doesn't have support for iax-ppc6.

if someone can give me the value of os.arch and os.endianess on iax-ppc64, I'll be more than happy to update my PR with the changes to support iax-ppc64.
currently, I don't have access to iax-ppc64 VM nor I'm able to emulate iax-ppc64 platform architecture using qemu.
please let me know if you have any way to get the values of os.arch and os.endianess for iax-ppc64.

@adilhusain-s
Copy link
Author

@dmitry-shibanov

I've updated this PR to address the requested changes.

  • Added aix in supported OS.
    • returning ppc64le when os.arch () ppc64 and os.endianness () is le
  • returning ppc64 when os.arch () ppc64 and os.endianess() is be

please look at the following output:

bash-5.1# cat os.js     
const os = require('os');           
            
// Get the operating system name    
const osName = os.type();           
console.log(`Operating System: ${osName}`);     
console.log ("os platform: ", os.platform());   
// Get the processor architecture   
const processorArchitecture = os.arch();        
console.log(`Architecture: ${processorArchitecture}`);      
            
// Get the endianness (byte order)  
const endianness = os.endianness();             
console.log(`Endianness: ${endianness}`);       
bash-5.1# ./node os.js  
Operating System: AIX   
os platform:  aix       
Architecture: ppc64     
Endianness: BE          

root@adilhusain-centos-test-1:~# node os.js     
Operating System: Linux             
os platform:  linux     
Architecture: ppc64     
Endianness: LE          

@dmitry-shibanov
Copy link
Contributor

Hello @adilhusain-s. Sorry for misinformation. Initially setup-node does not support aix so your previous changes should be good. Could you please revert changes to the previous ones?

@adilhusain-s
Copy link
Author

@dmitry-shibanov
revert the changes.
please review and merge.

@nikolai-laevskii
Copy link
Contributor

Quick update:

There seems to be problem with failing tests. It should be resolved as soon as https://github.com/actions/setup-node/pull/882 is merged.

For now everything looks good.

@janani66
Copy link

Quick update:

There seems to be problem with failing tests. It should be resolved as soon as https://github.com/actions/setup-node/pull/882 is merged.

For now everything looks good.

Looks like #882 is merged. Hoping that this PR can also be merged soon.

@adilhusain-s
Copy link
Author

Hi
@dmitry-shibanov
@nikolai-laevskii

just rebased my PR.
could you approve the waiting workflows?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants