mapbox / mapbox/node-pre-gyp

Compatible with Windows system path

Open
#670 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
1.2k
Forks
271
Avg merge
23h 40m
Merged PRs (30d)
2

Description

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used [patch-package](https://github.com/ds300/patch-package) to patch `@mapbox/node-pre-gyp@1.0.9` for the project I'm working on.

Here is the diff that solved my problem:

```diff
diff --git a/node_modules/@mapbox/node-pre-gyp/lib/install.js b/node_modules/@mapbox/node-pre-gyp/lib/install.js
index 617dd86..fbf6ac7 100644
--- a/node_modules/@mapbox/node-pre-gyp/lib/install.js
+++ b/node_modules/@mapbox/node-pre-gyp/lib/install.js
@@ -209,9 +209,8 @@ function install(gyp, argv, callback) {
}

makeDir(to).then(() => {
- const fileName = from.startsWith('file://') && from.slice('file://'.length);
- if (fileName) {
- extract_from_local(fileName, to, after_place);
+ if (opts.use_local_mirror) {
+ extract_from_local(from, to, after_place);
} else {
place_binary(from, to, opts, after_place);
}
diff --git a/node_modules/@mapbox/node-pre-gyp/lib/util/versioning.js b/node_modules/@mapbox/node-pre-gyp/lib/util/versioning.js
index 825cfa1..c53813e 100644
--- a/node_modules/@mapbox/node-pre-gyp/lib/util/versioning.js
+++ b/node_modules/@mapbox/node-pre-gyp/lib/util/versioning.js
@@ -274,7 +274,7 @@ module.exports.get_process_runtime = get_process_runtime;
const default_package_name = '{module_name}-v{version}-{node_abi}-{platform}-{arch}.tar.gz';
const default_remote_path = '';

-module.exports.evaluate = function(package_json, options, napi_build_version) {
+module.exports.evaluate = function (package_json, options, napi_build_version) {
options = options || {};
validate_config(package_json, options); // options is a suitable substitute for opts in this case
const v = package_json.version;
@@ -307,14 +307,19 @@ module.exports.evaluate = function(package_json, options, napi_build_version) {
toolset: options.toolset || '', // address https://github.com/mapbox/node-pre-gyp/issues/119
bucket: package_json.binary.bucket,
region: package_json.binary.region,
- s3ForcePathStyle: package_json.binary.s3ForcePathStyle || false
+ s3ForcePathStyle: package_json.binary.s3ForcePathStyle || false,
};
- // support host mirror with npm config `--{module_name}_binary_host_mirror`
- // e.g.: https://github.com/node-inspector/v8-profiler/blob/master/package.json#L25
- // > npm install v8-profiler --profiler_binary_host_mirror=https://npm.taobao.org/mirrors/node-inspector/
+ // support host mirror with npm config `--{module_name}_binary_host_mirror`
+ // e.g.: https://github.com/node-inspector/v8-profiler/blob/master/package.json#L25
+ // > npm install v8-profiler --profiler_binary_host_mirror=https://npm.taobao.org/mirrors/node-inspector/
const validModuleName = opts.module_name.replace('-', '_');
const host = process.env['npm_config_' + validModuleName + '_binary_host_mirror'] || package_json.binary.host;
opts.host = fix_slashes(eval_template(host, opts));
+ opts.use_local_mirror = opts.host.startsWith('file://');
+ if (opts.use_local_mirror) {
+ opts.host = opts.host.slice('file://'.length);
+ }
+ // console.log(opts)
opts.module_path = eval_template(package_json.binary.module_path, opts);
// now we resolve the module_path to ensure it is absolute so that binding.gyp variables work predictably
if (options.module_root) {
@@ -329,7 +334,7 @@ module.exports.evaluate = function(package_json, options, napi_build_version) {
const package_name = package_json.binary.package_name ? package_json.binary.package_name : default_package_name;
opts.package_name = eval_template(package_name, opts);
opts.staged_tarball = path.join('build/stage', opts.remote_path, opts.package_name);
- opts.hosted_path = url.resolve(opts.host, opts.remote_path);
- opts.hosted_tarball = url.resolve(opts.hosted_path, opts.package_name);
+ opts.hosted_path = opts.use_local_mirror ? path.resolve(opts.host, opts.remote_path) : url.resolve(opts.host, opts.remote_path);
+ opts.hosted_tarball = opts.use_local_mirror ? path.resolve(opts.hosted_path, opts.package_name) : url.resolve(opts.hosted_path, opts.package_name);
return opts;
};
```

This issue body was [partially generated by patch-package](https://github.com/ds300/patch-package/issues/296).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with lib/install.js and lib/util/versioning.js, focusing on how a file:// local mirror is detected and how its path is resolved. Compare the proposed changes with the existing Windows path handling and verify that local mirrors and remote URLs both continue to produce usable binary paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.