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

build,aix: skip ld missing symbols warning #27773

Merged
merged 1 commit into from
Jun 1, 2019

Conversation

refack
Copy link
Contributor

@refack refack commented May 19, 2019

On a typical node CI job we get ~68.5K of these, e.g for https://ci.nodejs.org/job/node-test-commit-aix/23246/nodes=aix61-ppc64/consoleText:

...
ld: 0711-319 WARNING: Exported symbol not defined: v8::internal::BaseBuiltinsFromDSLAssembler::Cast16FixedDoubleArray(v8::internal::compiler::TNode<v8::internal::HeapObject>, v8::internal::compiler::CodeAssemblerLabel*)
ld: 0711-319 WARNING: Exported symbol not defined: v8::internal::BaseBuiltinsFromDSLAssembler::Convert5ATSmi8ATintptr(v8::internal::compiler::TNode<v8::internal::IntPtrT>)
ld: 0711-319 WARNING: Exported symbol not defined: v8::internal::BaseBuiltinsFromDSLAssembler::Convert5ATSmi8ATuint32(v8::internal::compiler::TNode<v8::internal::Uint32T>)
...
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label May 19, 2019
@refack refack added aix Issues and PRs related to the AIX platform. wip Issues and PRs that are still a work in progress. labels May 19, 2019
@sam-github
Copy link
Contributor

/cc @miladfarca

They overwhelm the logs, for sure, would be nice to get rid of them. Will this get rid of all warnings? I'm not necessarily opposed to that, just curious.

@refack refack removed the wip Issues and PRs that are still a work in progress. label May 19, 2019
@refack
Copy link
Contributor Author

refack commented May 19, 2019

They overwhelm the logs, for sure, would be nice to get rid of them. Will this get rid of all warnings? I'm not necessarily opposed to that, just curious.

I'm just key-mashing I have no idea what I'm doing ¯\(ツ)/¯.
Seems like -bnoerrmsg gets the job done, but I assume it will probably also suppress "interesting" error messages.
https://ci.nodejs.org/job/node-test-commit-aix/23251/nodes=aix61-ppc64/consoleText

@refack refack self-assigned this May 19, 2019
@miladfarca
Copy link
Contributor

miladfarca commented May 19, 2019

Not really sure if it's fully doable since gcc calls the AIX linker behind the scenes, here is a list of available flags: https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.aix.cmds3/ld.htm
noerrmsg is explained in the above page and may hide errors as-well.

@Trott
Copy link
Member

Trott commented May 30, 2019

@refack Do you still want this to land? Or does this suppress warnings too broadly?

@sam-github
Copy link
Contributor

@miladfarca So, you suggest not merging this?

The linked docs are:

noerrmsg | Does not write error messages to standard error. Use this option if you specify thenoquiet option and you pipe standard output to a command such as tee or pg.

That's not so illuminating, it makes it sound like the message will go to standard out.

@miladfarca
Copy link
Contributor

miladfarca commented May 31, 2019

So I just ran a test with the flag added, all the WARNING: Exported symbol not defined messages are gone, and warning message such as this are still displayed (which is good):

In file included from ../deps/openssl/openssl/crypto/objects/obj_dat.c:22:0:
../deps/openssl/openssl/crypto/objects/obj_dat.h:1117:5: warning: missing initializer for field 'flags' of ...

Also checked for error messages and they display just fine so seems like the change might do the trick.

@sam-github
Copy link
Contributor

LGTM since @miladfarca is good with it

@refack
Copy link
Contributor Author

refack commented May 31, 2019

LGTM since @miladfarca is good with it

Great. So this needs a formal ✔️

@nodejs-github-bot
Copy link
Collaborator

PR-URL: nodejs#27773
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@refack refack merged commit 74b4986 into nodejs:master Jun 1, 2019
@refack refack deleted the aix-ignore-ld-warnings branch June 1, 2019 12:52
targos pushed a commit that referenced this pull request Jun 1, 2019
PR-URL: #27773
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@targos targos mentioned this pull request Jun 3, 2019
@refack refack removed their assignment Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aix Issues and PRs related to the AIX platform. build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants