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

WIP: feat: add compatibility with bun #1523

Closed
wants to merge 2 commits into from

Conversation

nya1
Copy link

@nya1 nya1 commented Oct 2, 2022

Closes #1519

Changes

  1. The preferred way to identify the node version is via process.versions.node (same logic is used also here), there is a fallback to the old process.version
  2. Binary path location is determined using path.resolve in current (node_modules) directory

Bun issues

…e binary using path resolve

In Bun the `process.version` returns the Bun's version, instead to get the Node version we need to
use `process.versions.node`, the same logic is used also here: https://github.com/oracle/node-oracledb/blob/8388891eb59a13403d607543b578f6e7aa876da8/package/install.js#L79

Signed-off-by: Nicola Rossi <nya1git@imap.cc>
Signed-off-by: Nicola Rossi <nya1git@imap.cc>
@oracle-contributor-agreement
Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Oct 2, 2022
@cjbj
Copy link
Member

cjbj commented Oct 19, 2022

@nya1 this is still marked a WIP. Shall we merge it into our internal code repo and close this PR (which is our normal release process)? Or are we waiting for more updates from you?

@nya1
Copy link
Author

nya1 commented Oct 20, 2022

@nya1 this is still marked a WIP. Shall we merge it into our internal code repo and close this PR (which is our normal release process)? Or are we waiting for more updates from you?

Hello, I've added the draft status on purpose, while working on this issue I discovered some missing napi implementation and bugs in Bun which I reported to the Bun's repository, at the moment there are three major napi implementation missing, I've created a checklist in the PR description (under "Bun issues") with links to Bun's issues to track what we need to have a working/minimum Bun compatibility and go on with the testing and changes.

@cjbj
Copy link
Member

cjbj commented Oct 21, 2022

@nya1 we will wait for you to let us know when you are finished with this PR.

@Jarred-Sumner
Copy link

napi_create_external and napi_get_value_external will be available in Bun v0.2.3

@stale
Copy link

stale bot commented Dec 31, 2022

This issue has been automatically marked as inactive because it has not been updated recently. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactive This was marked by stalebot as inactive label Dec 31, 2022
@stale
Copy link

stale bot commented Jan 8, 2023

This issue has been automatically closed because it has not been updated for a month.

@stale stale bot closed this Jan 8, 2023
@cjbj cjbj removed the inactive This was marked by stalebot as inactive label Jan 8, 2023
@cjbj cjbj reopened this Jan 8, 2023
@Jarred-Sumner
Copy link

napi_make_callback is implemented in the canary build and napi_create_external + napi_get_value_external are in Bun v0.3.0 (there was no v0.2.3)

Assuming there are no bugs (big assumption at the present state), good shot of this working @nya1

to upgrade:

bun upgrade --canary

@nya1
Copy link
Author

nya1 commented Jan 18, 2023

napi_make_callback is implemented in the canary build and napi_create_external + napi_get_value_external are in Bun v0.3.0 (there was no v0.2.3)

Assuming there are no bugs (big assumption at the present state), good shot of this working @nya1

to upgrade:

bun upgrade --canary

Thank you for the update, I've encountered another possible bug while testing, I've created a new issue here: oven-sh/bun#1831

@stale
Copy link

stale bot commented Mar 25, 2023

This issue has been automatically marked as inactive because it has not been updated recently. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactive This was marked by stalebot as inactive label Mar 25, 2023
@stale
Copy link

stale bot commented Apr 2, 2023

This issue has been automatically closed because it has not been updated for a month.

@stale stale bot closed this Apr 2, 2023
@cjbj cjbj removed the inactive This was marked by stalebot as inactive label Apr 2, 2023
@cjbj cjbj reopened this Apr 2, 2023
@stale
Copy link

stale bot commented May 22, 2023

This issue has been automatically marked as inactive because it has not been updated recently. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactive This was marked by stalebot as inactive label May 22, 2023
@cjbj cjbj removed the inactive This was marked by stalebot as inactive label May 22, 2023
@cjbj
Copy link
Member

cjbj commented May 23, 2023

I'm keeping this open so it can be assessed by @nya1 when node-oracledb 6.0 has been released.

@cjbj
Copy link
Member

cjbj commented May 24, 2023

@nya1 can you take a look at this PR with node-oracledb 6 and the new pure JS mode?

@stale
Copy link

stale bot commented Aug 12, 2023

This issue has been automatically marked as inactive because it has not been updated recently. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the inactive This was marked by stalebot as inactive label Aug 12, 2023
@stale
Copy link

stale bot commented Sep 17, 2023

This issue has been automatically closed because it has not been updated for a month.

@stale stale bot closed this Sep 17, 2023
@sharadraju sharadraju removed the inactive This was marked by stalebot as inactive label Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please make node-oracledb compatible with bun
4 participants