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

CLI extracted from Polykey and migrated to Polykey-CLI #2

Merged
merged 17 commits into from
Aug 7, 2023

Conversation

tegefaulkes
Copy link
Contributor

@tegefaulkes tegefaulkes commented Jul 28, 2023

Description

In this PR we are pulling out all of the bin related code from Polykey and transplanting it here. This enables our Polykey repo to be simpler and also closer to a library that can be used in downstream applications. The Polykey's CI/CD should work faster, and not be blocked on integration problems. This repo can be more focused on CLI workflows and integration problems and cross-platform packaging.

Tasks

  • 1. New repo needs to be created, use the ts-demo-lib as a base
  • 2. All CLI code needs to be transplanted.
  • 3. All Bin related tests need to be transplanted.
  • 4. All CLI and integration CI jobs need to be transplanted.

Final checklist

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

@tegefaulkes tegefaulkes self-assigned this Jul 28, 2023
@tegefaulkes
Copy link
Contributor Author

I don't know if we want to explore bundling the source code just yet. I'll create/move an issue here for that. It should be done in a separate PR.

@CMCDragonkai
Copy link
Member

You mean bundling before we feed it all into pkg?

We can do that later. Because we have to upgrade to ES modules first, and then figure out which bundler can support all we want properly.

@tegefaulkes
Copy link
Contributor Author

Then we'll need to explore using ES modules in the next stage.

@tegefaulkes
Copy link
Contributor Author

I think we're ready to start testing CI now.

@tegefaulkes
Copy link
Contributor Author

We'll need to set up the Gitlab repo and mirroring.

@CMCDragonkai
Copy link
Member

Is MatrixAI/Polykey#534 also completed? I mean it's pending the first PR, but that would have to extract everything so PK can be used as a library right?

@tegefaulkes
Copy link
Contributor Author

The Polykey side changes haven't been done yet. It can be used as a library already except for two points.

  1. We need to merge and publish the agent migration changes if we want to run CI testing here.
  2. We need to review how Polykey does exports. Right now I'm doing a lot of Polykey/dist/whatever paths when importing and it's not ideal.

I'm blocked here at this stage. I need to move onto the Polykey side of CLI migration an merge the agent migration PR before moving forward here.

@CMCDragonkai
Copy link
Member

I've setup the CI/CD configuration for this repo and also in gitlab.

Note that branches are now protected, so before merging, make sure to squash properly.

Furthermore, I've changed the default branches to be staging. Make sure that this PR is targeting staging not master.

Also remember to replicate all the scaffolding files like .github/workflows/codesee-arch-diagram.yml and all the other relevant files at the project root and testing architecture.

@CMCDragonkai
Copy link
Member

  • github
  • .env
  • .env.example
  • .eslintignore
  • .eslintrc
  • .gitignore
  • .gitlab-ci.yml
  • .npmignore
  • nodemon.json
  • .npmrc
  • .prettierrc
  • tsconfig.json

Make sure tests structure and benches structure are the same, and also generated docs too.

@CMCDragonkai
Copy link
Member

Oh and the images/cli_demo.gif should be moved over too!

The README.md inside Polykey should have a link to Polykey-CLI. And the demo gif can be moved here.

@CMCDragonkai
Copy link
Member

Because Polykey is no longer an application, there's no need to have default.nix, nor utils.nix, nor release.nix. The only thing necessary is pkg.nix and shell.nix.

@CMCDragonkai
Copy link
Member

The .github will bring in codesee.

Note that one thing is the container path needs to change too. We are now uploading containers to the polykey-cli repo in Gitlab:

image

@CMCDragonkai
Copy link
Member

Regarding benchmarks, make sure that you have a dummy structure there for later, as can create benchmarks for our CLI too later.

@CMCDragonkai CMCDragonkai self-requested a review August 4, 2023 03:05
@tegefaulkes tegefaulkes changed the base branch from master to staging August 4, 2023 03:10
@CMCDragonkai
Copy link
Member

Synchronised issue labels too now.

@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management label Aug 4, 2023
@CMCDragonkai CMCDragonkai changed the title CLI migration CLI extracted from Polykey and migrated to Polykey-CLI Aug 4, 2023
@CMCDragonkai
Copy link
Member

ETA on merge?

Make sure to test out the CI jobs locally too inside your own shell.nix.

@CMCDragonkai
Copy link
Member

Some problems I had worked on earlier that will be relevant here:

Pretty much all the issues in TS demo lib is relevant here!

@tegefaulkes
Copy link
Contributor Author

Tests are passing locally now. We're importing a pre-release version of polykey for this so that's all working.

The Container stuff still needs to be updated.

CI job is failing, namely nix-dry and the tests. Tests are acting odd, they're failing to import polykey and I'm not sure what's happening there.

The dry run nix-build is failing with the following error.

...
...
trace: warning: in docker image polykey-cli-0.0.1: The contents parameter is deprecated. Change to copyToRoot if the contents are designed to be copied to the root filesystem, such as when you use `buildEnv` or similar between contents and your packages. Use copyToRoot = buildEnv { ... }; or similar if you intend to add packages to /bin.
error (ignored): error: end of string reached
error: getting status of '/nix/store/Polykey': No such file or directory

I still need to look into this.

@tegefaulkes
Copy link
Contributor Author

tegefaulkes commented Aug 7, 2023

ETA on merge is hard to say. All that is left is to get CI passing but there are some odd bugs i'm dealing with.

The main one is that the peculiar crypto stuff is not working when using polykey as a library. I'm pretty sure it's a mismatch in dependency versions. I see two different versions for some peculiar modules so I'm trying to resolve that.

I still need to get the nix build working, there are some minor bugs there i'm investigating still.

The CI is failing with imports missing. Everything was working locally. I'm still looking into this as well.

Hopefully I can get it done today. But there are a lot of oddities with using Polykey as a library that is cropping up at this stage.

@tegefaulkes
Copy link
Contributor Author

I think all of the above is resolved now. Fixed mostly by re-generating the package-lock and fixing some dependencies in Polykey.

@tegefaulkes
Copy link
Contributor Author

I think this step can be merged. the next step is testing the staging and master step CI jobs. I may need to work from a staging PR for that. I think I can merge this PR for now and work in the merge to master PR that gets auto-generated.

@tegefaulkes
Copy link
Contributor Author

Testing if pkg works right now.

Looks like pkg is trying to download the prebuilt node file and failing. Looking into this, these are meant to be provided by nix-shell.

It's trying to download version 18.5.0 while nix-shell is providing 18.15.0. I tried to update pkg/pkg-fetch to the version that would fetch 18.15.0 instead, but there is no released version that does this. So I'll have to modify the env to provide 18.5.0.

Note that our version of node is 18.15.0, not sure if this will cause problems with building but we'll see.

@tegefaulkes
Copy link
Contributor Author

pkg is working now. There may still be runtime dependencies that are missing, should be seen in tests.

@tegefaulkes tegefaulkes force-pushed the feature-CLI_migration branch from ab26682 to 71a9df8 Compare August 7, 2023 05:48
@tegefaulkes
Copy link
Contributor Author

I'm merging this, further fixes will be done in the staging branch.

@tegefaulkes tegefaulkes marked this pull request as ready for review August 7, 2023 06:20
@tegefaulkes tegefaulkes merged commit 15c755f into staging Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management
Development

Successfully merging this pull request may close these issues.

2 participants