microsoft / sarif-sdk

.NET code and supporting files for working with the 'Static Analysis Results Interchange Format' (SARIF, see https://github.com/oasis-tcs/sarif-spec)
Other
189 stars 88 forks source link

@microsoft/sarif-multitool installs platform-dependent package #1964

Open rsenden opened 4 years ago

rsenden commented 4 years ago

Instructions at https://www.npmjs.com/package/@microsoft/sarif-multitool illustrate how to use SARIF MultiTool in a TypeScript project.

In order to use the @microsoft/sarif-multitool module in another TypeScript module, developers will need to run a command like npm install @microsoft/sarif-multitool at development time. Under the hoods this command will also automatically install the platform-dependent module and native binary for the current development platform.

However, for self-contained projects that include the installed node modules in their distribution, this basically means that the resulting project will be platform-dependent as well.

For example, suppose that I'm developing a GitHub Action, using Windows as a development platform. When adding the @microsoft/sarif-multitool module to my project, the @microsoft/sarif-multitool-win32 module is also automatically added to the project's node_modules directory. Now when I publish this GitHub Action, it will only run on Windows-based runners, because the project doesn't include the Linux or Darwin modules in its node_modules directory. As a developer, I cannot even install the Linux or Darwin modules manually, as the platform checks disallow installing these modules when running on Windows.

Is it possible to add functionality for dynamically downloading the appropriate native binary at runtime, rather than development time?

jeffersonking commented 3 years ago

@rsenden We have a similar scenario with the sarif-vscode-extension. Our solution was to use:

npm install --force --E @microsoft/sarif-multitool-darwin @microsoft/sarif-multitool-win32

We're open to considering other solutions if you have any.

rsenden commented 3 years ago

@jeffersonking Current TypeScript usage instructions just show the import ... from @microsoft/sarif-multitool statement, so I guess most users would simply try npm install @microsoft/sarif-multitool. As we know, this will fail at runtime whenever the platform used to download/install this module is different from the target deployment platform.

I think it would be helpful if the TypeScript usage instructions would state that npm install @microsoft/sarif-multitool installs a platform-specific module, together with potential pitfalls (examples mentioned in this thread) and work-arounds (npm install --force ...).

Maybe even better would be to have two separate top-level modules; one for console usage (behaving the same as the current sarif-multitool package), and another one for TypeScript usage, with functionality like the following:

jeffersonking commented 3 years ago

@rsenden Agreed on updating the usage instructions. I will do that. We had a console-focused customer when this was initially built, but now we need to refocus on your scenario.

The truth is this module is a thin wrapper around a series of platform-specific executables. Ironically, the dynamic download you are asking for is the reason we have leveraged Node in the first place: to not have maintain the detection, download, and hosting ourselves. Ideally (and cost-prohibitively for us), we would translate this from .NET to an actual node module with all the true import characteristics you describe.

Would it be easier if you just called npx @microsoft/sarif-multitool inside your GitHub Action rather than installing the module?

rsenden commented 3 years ago

@jeffersonking, thanks for looking into updating the usage instructions.

Actually running npm install -g @microsoft/sarif-multitool and npx @microsoft/sarif-multitool inside my GitHub Action is exactly what I'm doing now, thereby basically following the Console usage instructions; see https://github.com/fortify/gha-fpr-to-sarif/blob/master/src/main.ts.

It seems a bit strange though to invoke npm/npx from within a Node module; I think having a native Node API to access the actual platform-specific binaries would be a cleaner solution.