Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packaging up js-quic for integration into Polykey #19

Merged
merged 1 commit into from
May 17, 2023

Conversation

CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented May 12, 2023

Description

This introduces the CI/CD flows we have for native packages, and produces Linux, Mac and Windows binaries.

Issues Fixed

Tasks

  • 1. Remove scaffolding
  • 2. Update scripts/prebuild.js
  • 3. Implement the .gitlab-ci.yaml
  • 4. Integrate the CI/CD structure of feature to staging to master
  • 5. Figure out the prebuild distribution system
  • 6. Apply branch and tag protection

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai
Copy link
Member Author

The shell.nix had jetbrains in it, removing.

@CMCDragonkai
Copy link
Member Author

Also according to napi-rs/napi-rs#1412 (comment), in order to actually stop the generation of index.d.ts in the project directory, we have to modify the features of napi-derive.

I've done this with:

napi-derive = { version = "2", default-features = false, features = ["strict", "compat-mode"] }

This disables the default features and then re-enables strict and compat-mode.

This is because in the napi-derive Cargo.toml:

[features]
compat-mode = []
default = ["compat-mode", "full"]
full = ["type-def", "strict"]
noop = ["napi-derive-backend/noop"]
strict = ["napi-derive-backend/strict"]
type-def = ["napi-derive-backend/type-def"]

There's no way to only disable a single feature. So I have to disable all default features and then re-activate what I want. So in this case I'm re-enabling compat-mode and strict.

After re-running the build, I can see there's no more index.d.ts file.

@CMCDragonkai
Copy link
Member Author

The --no-dts-header is not required anymore since the dts file is no longer generated anyway.

@CMCDragonkai CMCDragonkai self-assigned this May 12, 2023
@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented May 12, 2023

I think I will use /prebuilds/quic.node as the preferred native module location. The name quic.node is used because the napi uses the name of Cargo.toml, it seems difficult to change this so we will keep it as is.

If this file exists, it will be used.

If it doesn't exist, it will look for it in the dependencies.

The model for this is lmdb. It has optional dependencies.

Then in each optional dependency we have:

.
├── index.js
├── node.abi108.glibc.node
├── node.abi115.glibc.node
├── node.abi93.glibc.node
├── node.napi.glibc.node
├── node.napi.musl.node
├── package.json
└── README.md

Unlike lmdb, we will not have so many different variants. This is because we are using rust, and I don't think we will bother to release so many kinds. A single file could be quic.node.

Then it's just a matter of loading it either from /prebuilds/quic.node or from the optional dependencies.

That actually means if you just do npm install, you'll get the optional dependencies. But if you npm run prebuild, you'll get the development build which will override the optional dependency. This makes it similar to the behaviour you get in other projects like js-db which still uses node-gyp.

The optional dependency packages will need to be automatically created during the releasing procedure.

Note that if you run npm run prebuild, you'll get the release version. But if you run node ./scripts/prebuild you'll get the dev version. I'm not entirely sure if I'll maintain the behaviour... we'll see. You'll want to compile the dev version when developing cause that will be faster and might have debug symbols.

Now during the actual npm publish... this is meant to be done by the CI/CD.

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented May 12, 2023

Also as with MatrixAI/TypeScript-Demo-Lib-Native#3, cross compiling is whole another thing. So I'll leave this out for now.

In fact there's actually no cross-compilation in node-gyp anyway. I think the question is whether you can cross-compile across operating systems vs cross-compiling across CPU architectures.

@CMCDragonkai
Copy link
Member Author

Ok I've made npm run prebuild just build the dev version. No need to always build the release. Instead the CI/CD can specify --release to ensure that it's the release version when it runs it explicitly.

@CMCDragonkai
Copy link
Member Author

So the prepublishOnly script ONLY runs when the user runs npm publish.

This can be used as the system that builds the subpackages and publishes them first before publishing the main package.

In that case, there's an expectation that a prebuild has already occurred.

We shall go into the prebuilds/ directory, and for each subdirectory there create a separate package directory like @matrixai/quic-platform-arch to then publish.

So it should be:

  • @matrixai/quic-linux-x64
  • @matrixai/quic-win32-x64
  • @matrixai/quic-darwin-x64+arm64

I know that universal binary for macos is built from node-gyp, I'm not entirely sure how rust does it though.

@CMCDragonkai
Copy link
Member Author

Seems like for darwin it might be more complicated: rust-lang/cargo#8875.

For node-gyp, it was just a matter of adding an extra -arch option and setting npm_config_arch env variable to x64+arm64. This is the setting passed to the node-gyp build to do this.

@CMCDragonkai
Copy link
Member Author

Looks like this was made available for napi command: napi-rs/napi-rs#1397.

Will need to investigate that to figure out how to build the macos one.

@CMCDragonkai
Copy link
Member Author

I have 3 new scripts:

scripts/prebuild.js
scripts/postversion.js
scripts/prepublishOnly.js

These 3 scripts produce a new workflow to building the native objects and bumping up the version in sync, and publishing the optional dependencies.

The next step is to integrate the CI/CD code into js-quic as it is a bit different too...

I'm also testing a new thing where the native object is the "main" field of the optional package package.json to simplify the loading code.

I need to rewrite the loading code to prefer native objects under the prebuild/ directory (due to development builds), then if not finding these, to then load the appropriate native optional package based on os.platform() and os.arch().

Testing all of this will also depend some what on all tests being done for this repo @tegefaulkes.

@CMCDragonkai
Copy link
Member Author

Time to squash and rebase on master, then to test the CI/CD.

@CMCDragonkai CMCDragonkai force-pushed the feature-packaging branch 2 times, most recently from 393dae1 to bbbd1e5 Compare May 16, 2023 10:32
@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented May 17, 2023

I've tested requiring the dependency @matrixai/quic-linux-x64 using require. It works fine.

It actually works in 2 cases:

import x from '@matrixai/quic-linux-x64';
const y = require('@matrixai/quic-linux-x64');

I think import() will also work but it ends up being asynchronous. This means the native object can only be used asynchronously unless we have top-level await. And we don't have top-level await until we get MatrixAI/TypeScript-Demo-Lib#32.

So we will stick with require() for now.

@CMCDragonkai
Copy link
Member Author

I've updated the src/bin/native/quiche.ts to load from the prebuild/ directory first, then attempt to load from npm node_modules.

@CMCDragonkai
Copy link
Member Author

Need to flesh out the stub benches to generate some empty metrics for these to be filled out later.

@CMCDragonkai
Copy link
Member Author

I'm going to start the process of manual pushing this to gitlab now. Will be testing on gitlab's CI/CD.

I'm going to merge this into master first, and change the default branch to staging @tegefaulkes.

fix: removed jetbrains from `shell.nix`
fix: removed socket.ts scaffolding
feat: added benches stub
fix: `npm bin` has been deprecated, replaced with `$(npm root)/.bin`
chore: all scripts seem to be working
fix: updated `src/native/quiche.ts` to load from `prebuild/` then npm dependencies
@CMCDragonkai CMCDragonkai merged commit 7193edf into master May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants