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

iojs+release doesn't call configure with --verbose, so compiler versions not reported #2281

Closed
sam-github opened this issue Apr 9, 2020 · 9 comments

Comments

@sam-github
Copy link
Contributor

The job does call gcc --version, but that's not guaranteed to be the compiler that is configured, AFAICT.

--verbose should be added to the configure flags.

I'll look at this. It quite possibly affects other jobs that are calling configure, so I'll do a dredge through the job XML to find the locations that need changing.

See: nodejs/node#32715

@richardlau
Copy link
Member

I don’t have visibility of the jobs but it’s possible they do not call configure directly. The Makefile target that builds the tarballs calls configure: https://github.com/nodejs/node/blob/dccdc51788bd5337f9fd80441ef52932383a2441/Makefile#L1163-L1172

@rvagg
Copy link
Member

rvagg commented Apr 10, 2020

They all do make binary-upload (or tar-upload, or headers-upload, or doc-upload), but they also have CONFIG_FLAGS which is currently --download-path=${HOME}/node-icu/ for all of them I think. We can just add --verbose in there I suppose? Would that be safe all the way back to Node 10?

@richardlau
Copy link
Member

They all do make binary-upload (or tar-upload, or headers-upload, or doc-upload), but they also have CONFIG_FLAGS which is currently --download-path=${HOME}/node-icu/ for all of them I think. We can just add --verbose in there I suppose? Would that be safe all the way back to Node 10?

Yes, 10.x supports the --verbose flag: https://github.com/nodejs/node/blob/670f3dbecdfa289942a02543c81c223a76b5af01/configure.py#L575

@richardlau
Copy link
Member

--download-path=${HOME}/node-icu/ could probably go as all supported releases should be using the in-tree ICU's (either small for Node.js 10/12 or full for 13+). (On the otherhand it's benign if we keep it.)

@sam-github
Copy link
Contributor Author

OK, CONFIG_FLAGS is defined repeatedly, and was missing on AIX (I added it). It now has --verbose appended, build: https://ci-release.nodejs.org/job/iojs+release/5897/ (10.x)

I noticed this while looking at an AIX build to confirm that CONFIG_FLAGS was empty:

01:00:39 + '[' Xnightly == Xrc ']'
01:00:39 + '[X' == Xdocker ']'
01:00:39 /tmp/jenkins2601478034381801002.sh: line 29: [X: command not found
01:00:39 + echo ostype=AIX72
01:00:39 ostype=AIX72

I think its because the AIX jobs have if ["X${OSVARIANT}" == "Xdocker" ]; then when they should have if [[ "X${OSVARIANT}" == "Xdocker" ]]; then. AIX builds never happen on Docker, it doesn't matter, but I could fix the syntax. Or, should I just delete it?

@sam-github
Copy link
Contributor Author

It looks like adding --verbose didn't break anything, but it isn't as useful on 10.x, I backported the compiler detection PR.

@sam-github
Copy link
Contributor Author

ci-release build was green, but will leave open as a reminder of #2281 (comment)

@richardlau
Copy link
Member

OK, CONFIG_FLAGS is defined repeatedly, and was missing on AIX (I added it). It now has --verbose appended, build: https://ci-release.nodejs.org/job/iojs+release/5897/ (10.x)

I noticed this while looking at an AIX build to confirm that CONFIG_FLAGS was empty:

01:00:39 + '[' Xnightly == Xrc ']'
01:00:39 + '[X' == Xdocker ']'
01:00:39 /tmp/jenkins2601478034381801002.sh: line 29: [X: command not found
01:00:39 + echo ostype=AIX72
01:00:39 ostype=AIX72

I think its because the AIX jobs have if ["X${OSVARIANT}" == "Xdocker" ]; then when they should have if [[ "X${OSVARIANT}" == "Xdocker" ]]; then. AIX builds never happen on Docker, it doesn't matter, but I could fix the syntax. Or, should I just delete it?

Single square bracket should be okay but needs a space between the opening square bracket and the double quote.

@sam-github
Copy link
Contributor Author

OK, added single space.

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

No branches or pull requests

3 participants