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: do not hard code module and napi versions #49277

Closed
wants to merge 1 commit into from

Conversation

zcbenz
Copy link
Contributor

@zcbenz zcbenz commented Aug 22, 2023

Node encourages embedders to define their own node_module_version, but currently the only way to modify the number is to edit the src/node_version.h file, which means anyone who wants to embed Node has also to fork Node, sometime merely to edit the src/node_version.h file.

What this PR does

This PR makes node_module_version and napi_build_version numbers configurable so embedders do not have to fork Node to set the numbers. It does follow things:

  1. Add options in configure.py to override node_module_version and napi_build_version, there is also a node_api_versions.json file added which records current versions.
  2. Rename src/node_version.h to src/node_version.h.in, and replace the values of NODE_MODULE_VERSION macros with placeholders.
  3. The build system will read src/node_version.h.in, replace the placeholders with actual numbers, and then write a node_version.h to output directory.
  4. Node's source code will be built with the generated node_version.h file.

Changes to Node's release workflows

This unfortunately affects Node's release workflows, namely 2 things:

  1. When updating the node_module_version and napi_build_version numbers, developers will edit the node_api_versions.json file instead of src/node_version.h.
  2. When updating Node versions, developers will edit the renamed src/node_version.h.in file.

Benefits

  1. Embedders have one less reason to fork Node.
  2. The configure.py script no longer needs to parse src/node_version.h to get the node_module_version and napi_build_version numbers, and the tools/getmoduleversion.py and tools/getnapibuildversion.py scripts can be removed.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/tsc
  • @nodejs/website

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 22, 2023
@zcbenz
Copy link
Contributor Author

zcbenz commented Aug 22, 2023

Unfortunately this is going to break a lot of Node.js tooling which often needs to work across Node.js versions

How do you think if I put only node_module_version and napi_build_version to a separate node_api_versions.h file, and then include it in node_version.h? The affected toolings will be far less and I can create PRs for them to help the migration.

@zcbenz
Copy link
Contributor Author

zcbenz commented Aug 22, 2023

I find it quite hard to make test addons work with generated node_version.h, I'll go with another approach.

@zcbenz zcbenz closed this Aug 22, 2023
@zcbenz
Copy link
Contributor Author

zcbenz commented Aug 22, 2023

Replaced with #49279.

Technically this PR is much more decent, but there are too many things depending on this one single file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants