Page MenuHomePhabricator

[keyserver] Make opaque-ke-napi build on Windows
ClosedPublic

Authored by michal on Jan 3 2023, 5:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jun 21, 4:03 AM
Unknown Object (File)
Fri, Jun 21, 4:03 AM
Unknown Object (File)
Tue, Jun 18, 7:04 AM
Unknown Object (File)
Sun, Jun 16, 4:24 PM
Unknown Object (File)
May 24 2024, 7:16 PM
Unknown Object (File)
May 24 2024, 8:31 AM
Unknown Object (File)
May 24 2024, 7:21 AM
Unknown Object (File)
May 24 2024, 7:21 AM
Subscribers

Details

Summary

We want to be able to build the desktop app on Windows but currently opaque-ke-napi fails.
As far as I've investigated the issue happens because when you run napi build it searches for node_modules/@napi-rs/cli/index.js. It can't find it on windows because of some symlink weirdness
(we are using yarn workspaces so we are running this in node_modules/native folder that's symlinked to the real folder).

This diff uses the nohoist option to keep @napi-rs/cli inside native/node-modules (otherwise it's hoisted to the root node_modules). The biggest problem with this approach is that if we ever use @napi-rs/cli in some other package it wouldn't be shared.

Test Plan
  • yarn cleaninstall on macOS laptop works
  • CI on phab works
  • yarn cleaninstall on Windows laptop and on GitHub Windows Runner (in a forked repo) works

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Alternatively, we could do something similar to D6142 (make a bash script and skip napi build step if it's Windows).

keyserver/addons/opaque-ke-napi/package.json
7

We need to make this package private to use the workspaces option.

michal requested review of this revision.Jan 3 2023, 5:50 AM

Doesn't seem great but I'm not sure of a better option. The nohoist shouldn't have any additional performance cost since this is the only place we use it

This revision is now accepted and ready to land.Jan 3 2023, 10:43 AM