Skip to content

CLI - building interfaces uses a hidden, "empty" build strategy#1356

Merged
dOrgJelli merged 2 commits intoorigin-0.9-devfrom
pileks/bugfix/cli-building-interfaces-no-docker
Oct 25, 2022
Merged

CLI - building interfaces uses a hidden, "empty" build strategy#1356
dOrgJelli merged 2 commits intoorigin-0.9-devfrom
pileks/bugfix/cli-building-interfaces-no-docker

Conversation

@pileks
Copy link
Contributor

@pileks pileks commented Oct 20, 2022

Closes #1355

Targetting 0.9 as it seems like a non-breaking change.

The idea behind this fix is that building Interface projects doesn't require a BuildStrategy at all.
The -s argument is essentially ignored for Interface projects, and a hidden EmptyBuildStrategy is used, which does nothing.
Additionally, an info message is rendered to the user.

Users can't explicitly use this strategy.

@pileks pileks linked an issue Oct 20, 2022 that may be closed by this pull request
Copy link
Contributor

@dOrgJelli dOrgJelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've spoken with @pileks privately about a more future proof solution to this. The TL;DR is that we need to improve the semantics around how we treat interface projects, and all projects at large, when it comes to build & codegen commands. This work will be done in v0.10, and this PR will be merged as it's a quick-fix that doesn't touch too much code.

@dOrgJelli
Copy link
Contributor

Actually, going to paste my full thoughts here:

  • I think the "Empty Strategy" is a clever solution, and we can merge into v0.9 as it fixes the issue while changing the least amount of code.
  • There are 2 major architectural changes I think are needed to better support interfaces:
    1. We should split PolywrapProject into WasmProject & InterfaceProject.
    2. We should decouple the concept of generating the wrap.info from the Compiler, and use it for the build and codegen commands (see the comment in codegen.ts that says // HACK: ....

The logic for build should be something like this:

const project = createProject(manifest);
const supportedProject =
  project.isWasm() ||
  project.isPlugin() ||
  project.isInterface();

if (!supportedProject) {
  throw Error("Unsupported project type...");
}

// 1. Parse schema & build wrap.info
const schemaComposer = new SchemaComposer(...);
const infoCompiler = new WrapInfoCompiler(...);
infoCompiler.compile();

// 2, Compile wrap.wasm
if (project.isWasm()) {
  const wasmCompiler = new WrapWasmCompiler(...);
  wasmCompiler.compile();
}

And we can refine the semantics around the polywrap build and polywrap codegen commands. Basically, we shouldn't run codegen automatically when you run polywrap build, and we shouldn't generated the wrap.info file when you run polywrap codegen.

  • Plugin devs will need to run polywrap codegen for types & polywrap build for wrap.info the file.
  • Wasm devs will need to run polywrap codegen for types & polywrap build for wrap.info & wrap.wasm files.
  • Interface devs will need to run polywrap build for wrap.info file.
  • App devs will need to run polywrap codegen for types.

^^^ I guess this is the 3rd major architectural change :P

@dOrgJelli dOrgJelli merged commit 6d05f61 into origin-0.9-dev Oct 25, 2022
@dOrgJelli dOrgJelli deleted the pileks/bugfix/cli-building-interfaces-no-docker branch April 10, 2023 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI: Building Interfaces Shouldn't Require Docker

2 participants