aspect-build / aspect-build/rules_js
[FR]: Allow using node headers for node-gyp from repo rule
- Dominant language
- Starlark
- Stars
- 378
- Forks
- 183
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 32
Description
### What is the current behavior?
## Background
At the moment, when running life cycle via `node-gyp`, it need to download node headers from https://nodejs.org/download/release/v20.12.2/node-v20.12.2-headers.tar.gz, which
1. Does not use bazel downloader
2. The url is external
## Use local tarball
`node-gyp` does provide some args that allows us to override the node header in many ways, and I think the most appropriate one is `tarball` https://github.com/nodejs/node-gyp?tab=readme-ov-file#command-options. It's configurable via `npm_config_tarball` env which can be achieved in `lifecycle_hooks_envs`.
What's the alternative?
1. `--dist-url`. It solves the external url issue, but it's still better to use the bazel downloader I think
2. `--devdir` and `--ensure`: `node-gyp` will check a file called `installVersion` under `devdir` and it's coupled with `node-gyp` version
## Blockers so far
In order to use `npm_config_tarball`, an intended workflow would be adding a repo rule, such as
```
http_archive(
name = "node_header",
strip_prefix = "node-v20.12.2",
urls = [
"https://nodejs.org/download/release/v20.12.2/node-v20.12.2-headers.tar.gz",
],
)
```
However, there's no way to pass the path to the lifecycle action at the moment if I get it correct?
### Describe the feature
I'm happy to create a PR for that, but I'm not sure what's the best solution. From what I can see there are multiple ones
## Pass tarball to `npm_translate_lock` directly
### API:
It can be called something like `lifecycle_hooks_gyp_tarball`, e.g. `lifecycle_hooks_gyp_tarball = "@node_header//tarball"`.
### Implementation:
When generating the `defs.bzl` via template for packages that need `node-gyp`, like `/external/aspect_rules_js~~npm~pnpm_deps__node-pty__1.0.0__links/defs.bzl`, `@node_header//tarball` is added to its `srcs`, and a flag will also be passed to `lifecycle-hooks`, `--tarball=xxx`. `lifecycle-hooks.js` will then append it with `process.cwd()` and pass to `node-gyp`
### pros
API is easy to use
### cons
A bit too specific? Not sure whether we want to introduce such an option
## Allow adding dependencies to lifecycle hook
### API
Add a new option maybe called `lifecycle_hooks_additional_deps`, like
```
"lifecycle_hooks_additional_deps": {
"node-pty": ["@node_header//tarball"]
}
```
### Implementation
When generating the `defs.bzl` via template for packages that need `node-gyp`, like `/external/aspect_rules_js~~npm~pnpm_deps__node-pty__1.0.0__links/defs.bzl`, these additional deps are added to its `srcs`. However, since bazel doesn't allow finding the absolute path of the input, we still need to find a way to prepend cwd in `lifecycle-hooks.js`. I'm thinking maybe we can add a functionality in `lifecycle-hooks.js`, to replace `$CWD` with `process.cwd()`?
## cons
A bit hard to consumes it, but I think it's fine? It's advance usage to use your own node header?
## pros
It's much generous, people may want to pass other deps to lifecycle hooks?
## Use `@rules_nodejs//nodejs/headers:current_node_cc_headers`
I'm blocked on the generated `defs.bzl`
```
_js_run_binary(
name = "{}/lc".format(store_target_name),
srcs = [
...
# node header tarball for node_gyp
"@rules_nodejs//nodejs/headers:current_node_cc_headers", <--
],
# js_run_binary runs in the output dir; must add "../../../" because paths are relative to the exec root
args = [
...
--header_dir=??? <-- how can I pass the dir to @rules_nodejs//nodejs/headers:current_node_cc_headers?
] +
select({
Label("@aspect_rules_js//platforms/os:osx"): ["--platform=darwin"],
Label("@aspect_rules_js//platforms/os:linux"): ["--platform=linux"],
Label("@aspect_rules_js//platforms/os:windows"): ["--platform=win32"],
"//conditions:default": [],
}) +
....
)
```
Contributor guide
Research direction
Start by tracing npm_translate_lock and the generated defs.bzl for packages using node-gyp, then inspect lifecycle-hooks.js and the existing lifecycle hook environment handling. Compare the proposed tarball, additional-dependency, and current_node_cc_headers approaches. Done means a documented repo-rule workflow can provide local Node headers to node-gyp without its external download.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, nodejs
- Domain
- build-system, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100