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

Select compiler broken on AIX #1992

Closed
mhdawson opened this issue Oct 23, 2019 · 13 comments · Fixed by #1994
Closed

Select compiler broken on AIX #1992

mhdawson opened this issue Oct 23, 2019 · 13 comments · Fixed by #1994

Comments

@mhdawson
Copy link
Member

@rvagg this change seems to have broken select compiler on AIX.
#1985

Failure is:

+ rm -rf build
+ git clone https://github.com/nodejs/build.git
Cloning into 'build'...
+ . ./build/jenkins/scripts/select-compiler.sh
/tmp/jenkins4646137741077972430.sh[50]: 0403-057 Syntax error at line 125 : `=~' is not expected.
Build step 'Conditional steps (multiple)' marked build as failure

https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/nodes=aix61-ppc64/811/console

@sam-github
Copy link
Contributor

The #!/bin/bash on the top of select-compiler.sh is wrong, its not bash, and its not a #! executable, its expected to be sourced. I'll change that to a comment that is correct.

Jenkins on AIX uses /bin/sh explicitly to call select-compiler:

[aix61-ppc64] $ /bin/sh -xe /tmp/jenkins1203817308243684471.sh

So basically select-compiler needs conversion to be /bin/sh syntax.

@mhdawson
Copy link
Member Author

@sam-github unless we can fix this quickly can you back out the previous change? It will block all releases.

@sam-github
Copy link
Contributor

Leaving open til a build passes.

Kicked off: https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/820/

(I've never run this job before, I limited it to aix61, perhaps someone cat take a look that I did it correctly).

@richardlau
Copy link
Member

Leaving open til a build passes.

Kicked off: https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api-new/820/

(I've never run this job before, I limited it to aix61, perhaps someone cat take a look that I did it correctly).

Unfortunately that build failed while downloading the Node.js binary... probably being affected by #1993.

@sam-github
Copy link
Contributor

Ouch. Well, I'll wait a bit before trying again.

@rvagg
Copy link
Member

rvagg commented Oct 23, 2019

do you not have Bash on AIX? if you do, then just make sure #!/bin/bash is at the top of the script blocks in Jenkins and it'll all run in Bash and this will be resolved. I already sorted out one instance of bad scripting in node-test-commit-v8-linux yesterday that had #/bin/bash at the top and was running Dash instead, the ! got this sorted. Can we just do the same on AIX?

@richardlau
Copy link
Member

Bash does exist on AIX.

@rvagg
Copy link
Member

rvagg commented Oct 23, 2019

Screen Shot 2019-10-24 at 9 46 17 am

This is node-test-commit-aix, is it in /usr/bin/bash? If so, why was this broken then? That should all have been executed in Bash.

@richardlau
Copy link
Member

@rvagg I don't think node-test-commit-aix was broken. This issue references node-test-node-addon-api-new.

@sam-github
Copy link
Contributor

I don't have strong feelings about this, though having been burned in the past porting scripts that assume that all the world is GNU to OSs that are not, I happily stick /bin/sh for scripts, and keep bashisms to interactive use.

The existing script was /bin/sh compatible and worked, now it is /bin/sh compatible again, I don't see the ifelfs as any clearer than the case statements that were used throughout the rest of the file. I'm also not keen to have scripts that depend on magic config typed into jenkins job configs, which are unversioned.

But that's just me, if you really like bash and want to use it everywhere, you're doing the work, its up to you, I've no objections to working code!

And I just saw your post you added.... recall that select-compiler.sh is called from a number of jobs, and they all need the #! added.

@rvagg
Copy link
Member

rvagg commented Oct 23, 2019

I'm also not keen to have scripts that depend on magic config typed into jenkins job configs, which are unversioned.

That's the world we live in here, we can't escape some dependency on Jenkins, this script depends on being called by Jenkins. It's even invoked in two different ways currently - one by curl and one by git clone, this is "magic" by the same definition. We can't escape this reality unless you want to ditch Jenkins for <some unicorn>. Documentation helps though.

@sam-github
Copy link
Contributor

OK, so would you like me to revert the use of case back to bashisms, or add a commit clarifying that bash syntax is permitted in the future, or shall we just close this as "working for now"?

@rvagg
Copy link
Member

rvagg commented Oct 27, 2019

Sorry, it's fine as is, my bad for pushing it forward without understanding the implications. I'd prefer us to agree on Bash availability and compatibility for most purposes but it's not a big deal.

@rvagg rvagg closed this as completed Oct 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants