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

zos: rebase and apply custom compile flags to support Node v12 and v14 #1

Merged
merged 12 commits into from
Jul 10, 2020

Conversation

gabylb
Copy link
Member

@gabylb gabylb commented Jun 26, 2020

This PR merges current latest joeferner@bc557ad with 11d557d and updates binding.gyp to enable node-java to be build with v12 and v14 without explicitly settings compiler flags, as follows:

  • v8 (ships with D190508), requires -U_VARARG_EXT_ and no-opt
  • v12 (ships with D191122), requires no-opt
  • v14 (ships with D191122), no workaround needed (note npm test actually fails with no-opt)

The latest version from joeferne is required to resolve v8 API incompatibility issues with v8 (8.1) from Node v14.

It also converts GetContents() to GetBackingStore() for Node versions 8 and up.

The package.json from node-jdbc (https://github.com/ibmruntimes/node-jdbc/tree/zos) still needs to be updated to replace dependency:
"java": "git+https://github.com/ibmruntimes/node-java.git#node12-zos",
by:
"java": "git+https://github.com/ibmruntimes/node-java.git#node14-zos",

joeferner and others added 10 commits November 17, 2019 22:40
…ter_work

callback for uv_queue_work(), since uv_queue_work() is not supposed to be called
outside of main thread.  Fixes queue corruption problem when multiple
simultaneous proxy calls come in from separate threads.
This is to workaround previous (to v14) versions of node
and a compiler bug affected by those versions, as follows:

- v8 (ships with D190508), requires -U_VARARG_EXT_ and no-opt
- v12 (ships with D191122), requires no-opt
- v14 (ships with D191122), no workaround needed.
@gabylb gabylb changed the title rebase and apply custom compile flags to support Node v8, v12 and v14 zos: rebase and apply custom compile flags to support Node v8, v12 and v14 Jun 26, 2020
@gabylb gabylb requested review from dinogun and joransiu June 26, 2020 22:54
gabylb added a commit to ibmruntimes/citgm that referenced this pull request Jul 8, 2020
This is to enable node-jdbc to use the latest, unmerged update that
resolves dependant node-java build failure with Node v14.

That update is ibmruntimes/node-java#1
and is pending approval/merge.
Copy link
Member

@joransiu joransiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gabylb, there seems to be other commits from upstream in this PR, so I only reviewed on the ones you commited. Your changes look goods to me, but I'm wondering if you also need to add a special case for zOS to include the additional library options -- i.e. similar to the ones documented here: https://www.ibm.com/support/knowledgecenter/en/SSYKE2_8.0.0/com.ibm.java.vm.80.doc/docs/jni_runtime_linking.html#jni_libs_zos

Find Java's lib_dir to link to libjvm.x, instead of assuming
users have their LDFLAGS env variable set to that path.
@gabylb
Copy link
Member Author

gabylb commented Jul 9, 2020

Thanks Joran @joransiu for your useful feedback. Please see new commit. btw on z/OS Java there's no libjvm.a, so -ljvm on that URL won't work, and also there's no jre under <java_install_dir>. I checked this on tlba98me (v2r2), zoscan2b (v2r3), torolabd (v2r4-pre).

@gabylb gabylb requested a review from joransiu July 9, 2020 17:14
Copy link
Member

@joransiu joransiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gabylb. LGTM

@gabylb gabylb merged commit b5d8614 into node14-zos Jul 10, 2020
gabylb added a commit to gabylb/node-jdbc that referenced this pull request Jul 10, 2020
This PR updates package.json to get a node-java version
that resolves the subject issue in new branch node14-zos:
https://github.com/ibmruntimes/node-java.git

It also removes the requirement of setting LDFLAGS env var
prior to building node-java or node-jdbc.

Ref: ibmruntimes/node-java#1
@gabylb
Copy link
Member Author

gabylb commented Jul 10, 2020

Just found that the last commit d6ec935 breaks build of node-java in Node v8, because Node v8 emits its output from $(node findJavaHome.js) (in find_java_libdir.sh) in EBCDIC, so the path to libjvm.x used in binding.gyp contains garbage.

Users of Node v8 who want to newly download https://github.com/ibmruntimes/node-jdbc should now get branch node8-zos instead of branch zos - as branch zos has now been updated to use the new node-java version from branch node14-zos that was updated in this PR.

@gabylb gabylb changed the title zos: rebase and apply custom compile flags to support Node v8, v12 and v14 zos: rebase and apply custom compile flags to support Node v12 and v14 Jul 10, 2020
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.

5 participants