aspect-build / rules_js

High-performance Bazel rules for running Node.js tools and building JavaScript projects
https://docs.aspect.build/rules/aspect_rules_js
Apache License 2.0
310 stars 107 forks source link

[Bug]: 'node_modules/.bin' directory not created in sandbox #630

Open gregjacobs opened 2 years ago

gregjacobs commented 2 years ago

What happened?

Hey guys, so I came across an interesting scenario: I'm using icon-font-buildr to generate a font file, and the utility tries to spawn a process as part of its execution: npx svg2ttf

Unfortunately, because node_modules/.bin/svg2ttf is not found, npx instead tries to download the package and store it in /users/<user>/.npm/_npx/61742, resulting in the following error:

Error: EPERM: operation not permitted, mkdir '/Users/<user>/.npm/_npx/61742'

Would it be possible to create the node_modules/.bin directory for the Bazel sandbox? Seems like this would get this particular package to work, along with any other packages that rely on calling the npx command internally.

The npx call can be seen in the source code here btw: https://github.com/fabiospampinato/icon-font-buildr/blob/master/src/index.ts#L387

Version

Development (host) and target OS/architectures: MacOS Monterey

Output of bazel --version: 6.0.0-pre.20221020.1

Version of the Aspect rules, or other relevant rules from your WORKSPACE or MODULE.bazel file: rules_js 1.6.8

Language(s) and/or frameworks involved: JS

How to reproduce

BUILD:

load("@aspect_rules_js//js:defs.bzl", "js_binary", "js_run_binary")

js_run_binary(
    name = "test_icon_font_buildr",
    tool = ":test_icon_font_buildr_bin",
    outs = ["builder-fonts/MaterialDesign.woff2"]
)

js_binary(
    name = "test_icon_font_buildr_bin",
    entry_point = "test-icon-font-buildr.js",
    data = [
        "//:node_modules/icon-font-buildr",
        "//:node_modules/svg2ttf",
    ]
)

test-icon-font-buildr.js:

const path = require('path');
const IconFontBuildr = require('icon-font-buildr');

async function build() {
    const builder = new IconFontBuildr({
        sources: [
            // Where to get the icons
            'https://fonts.gstatic.com/s/i/materialicons/[icon]/v5/24px.svg',
        ],
        icons: [
            // Icons to download/use
            'error',
        ],
        output: {
            fonts: path.join(__dirname, 'builder-fonts'), // Where to save the fonts
            fontName: 'MaterialDesign', // The name of the font to generate
            formats: [
                'woff2',
            ],
        },
    });

    await builder.build();
}

build().catch(err => {
    console.log(err);
    process.exit(1);
});

Command:

bazel build //:test_icon_font_buildr

Any other information?

No response

Fund our work

gregmagolan commented 2 years ago

Thanks for the issue. It is possible to configure the bins for any package manually using the bins attribute in npm_translate_lock. See the example here https://github.com/aspect-build/rules_js/blob/dc854eced0e43ae04096cb0730e94dd511a6cb2a/WORKSPACE#L65

For rules_js to do this automatically we will need an upstream change in pnpm so that the lock file has the listing of bins for each package instead of just the current hasBins field that indicates there are some bins. This is necessary since rules_js would need to know about the bins of a package without having to download the package itself from the registry to keep the current property of lazy fetching & linking of npm packages.

We have a feature request up to pnpm but so far no activity. The change upstream in pnpm will be relatively small so we if we don't see activity on the request we'll make a PR at some point.

gregjacobs commented 2 years ago

Hey Greg, thanks so much. I totally missed the bins feature of npm_translate_lock!

That would be amazing if pnpm can add the bins information to its lock file, and would make sense. I think it's great that you guys took the lazy-fetching approach that you did with regards to downloading only the npm packages that are needed for any given build, so I'm definitely more than willing to specify the binaries myself inside npm_translate_lock in the meantime!

Thanks again for the info. Do you want me to keep this issue open for tracking or did you have a different one?

pcj commented 1 year ago

What does the bins attribute do? Does it somehow generate a js_binary rule? I don't see how to use it.

gregmagolan commented 1 year ago

What does the bins attribute do? Does it somehow generate a js_binary rule? I don't see how to use it.

It generates the .bin/foo executable bash scripts that can be used by runs such as js_run_devserver: https://github.com/aspect-build/bazel-examples/blob/6687141ce635ddd868f0dcadfe04725980faa1f3/react-cra/BUILD.bazel#L39

gregmagolan commented 1 year ago

Let's keep the issue open. Marked it as blocked by https://github.com/pnpm/pnpm/issues/5131.