wapm.io integration #43

jkbecker · 2023-03-09T14:59:24Z

Since I was planning on looking into executing arbitrary existing software packages via Serval Mesh, I had the sudden urge of making wapm.io integration happen. My goal for this change is to make it as easy as

❯ cargo run --bin serval -- pull author/package/1.0.0
Found package wapm.io/author/package/1.0.0
Downloading binary https://registry-cdn.wapm.io/contents/author/package/1.0.0/target/wasm32-wasi/release/package.wasm... Done

 WASM task name:      io.wasm.author.package
 Version:             1.0
 Manifest integrity:  sha256-p7XKW+adaldkmlakmsdlmalsdkml=
 WASM integrity:      sha256-a68Q3adlasldfkmalsdfkaslkdfjlsE8uYwVTafrg=
 To run:              cargo run -p serval -- run io.wasm.author.package

Where serval pull would

pull the appropriate package from wapm.io
generate a manifest for it
run serval store on the manifest.

First actual code submission, please be gentle 😸

This is a very rough work in progress - just wanted to share it already because my Rust is, well, rusty, and you might have conceptual opinions as well...

Please let me know what you think!

ceejbot

I love this. This is a great idea. We need this.

Let's think about feature design a little bit!

I know of two WASM package manager thingies:

WAPM is real. WARG is a proposal with a reference implementation that's under active development that might become real. If we generalize, we should support both registries. This leads us to a cli invocation like this:

serval pull wapm author/package@version
serval pull wapm ceejbot/loudbot
serval pull warg bytecodealliance.org/[email protected]

Note that I'm going with @version optionally because that's what all the other package manager clis seem to do.

We can pass the registry string into the pull() function in the cli implementation, and then it can be parsed by the specific registry implementation.

I have some specific suggestions about how we handle the specific registry implementations. Right now you're passing a string into a parse() function, which goes on to fill out some struct fields with registry-specific format strings at runtime. I suggest we restructure to make Rust's type system do this work for us.

Here's a possible implementation:

enum PackageRegistry {
    Wapm,
    Warg
}

impl PackageRegistry {
    fn download_url(&self, pkg: &PackageSpec) -> String {
        match self {
            PackageRegistry::Wapm => {
                format!("https://registry-cdn.wapm.io/contents/{}/{}/{}/target/wasm32-wasi/release/{}.wasm", pkg.author, pkg.name, pkg.version, pkg.name)
            },
            PackageRegistry::Warg => todo!(),
        }
    }

    // even cooler....
    fn download(&self, pkg: &PackageSpec) -> Result<Bytes, ServalError> {
        // do the work of downloading from this kind of registry
    }
}

pub struct PackageSpec {
    author: String,
    name: String,
    version: String,
    registry: PackageRegistry,
    // any other useful fields
}

impl PackageSpec {
    // other useful functions here

    pub fn download_url(&self) -> String {
        self.registry.download_url(self)
    }
}

impl TryFrom<&str> for PackageSpec {
    // put your parsing code here
}

The advantage of this approach is that different registries might have different ways to download, e.g., different ways to authenticate. We can hide that inside a specific registry download() implementation if we need to and get rid of the download_url() function. You can probably think of a bunch of different ways to achieve the goal of hiding the registry-specific stuff as much as possible.

The interesting Rust lesson here is to notice that I swapped out your match on a string approach for matching on an enum.

utils/src/registry.rs

cli/src/main.rs

utils/src/registry.rs

ceejbot · 2023-03-09T20:49:34Z

YAY for thinking of this feature, btw. Smrt. 🏆

jkbecker · 2023-03-10T09:59:56Z

Thanks, these are awesome suggestions. I'm so not used to having an "implementation" for an Enum - I initially had a similar setup to yours with a Registry enum (but different / less idiomatic implementation downstream from it) but then discarded it because I got stuck somewhere. Definitely rewriting this based on your suggestion! :)

…ration

cli/src/main.rs

utils/src/registry.rs

ceejbot

As before, some optional comments aimed at being rusty!

ceejbot · 2023-03-16T22:46:25Z

cli/src/main.rs

+    Pull {
+        /// The name of the software package, formatted as
+        /// [[protocol://]registry.tld/]author/packagename[@version][:module]
+        identifer: String,


We can eventually make the identifier a type more specific than a string and have clap parse it for us. Some examples here. There is no need to do that in this PR, however.

ceejbot · 2023-03-17T18:47:19Z

cli/src/main.rs

@@ -289,6 +295,100 @@ fn blocking_maybe_discover_service_url(
    Ok(format!("http://{addr}:{port}"))
 }

+/// Pull a Wasm package from a package manager, generate its manifest, and store it.
+fn pull(identifer: String) -> Result<()> {
+    let pkg_spec = PackageSpec::try_from(identifer).unwrap();


I will teach you a new trick with Rust! Or maybe you already know this one, but here is let-else:

let pkg_spec = Ok(PackageSpec::try_from(identifer)) else { // tell the user what they got wrong // then exit with a non-zero code }

This is nicer than panicking here if the format is wrong.

ahhhh... nice. I was theoretically aware of let else but did not grok the specific usage. Will hunt down a bunch of unwrap()s and clean them up.

ceejbot · 2023-03-17T18:48:44Z

cli/src/main.rs

+/// Pull a Wasm package from a package manager, generate its manifest, and store it.
+fn pull(identifer: String) -> Result<()> {
+    let pkg_spec = PackageSpec::try_from(identifer).unwrap();
+    log::debug!("{:#?}", pkg_spec);


There is absolutely zero reason to change this, but I note that the dbg!() macro is another option for this kind of fast printf debugging.

cli/src/main.rs

ceejbot · 2023-03-17T19:04:33Z

utils/src/registry.rs

+            PackageRegistry::Warg => todo!(),
+        }
+    }
+    // even cooler....


hey wait I recognize that ellipsis with an extra dot

blatant plagiarism. painstakingly embellished for hours and hours, though, I claim fair use 😹

shinypb

Posting a comment solely to get GitHub to stop showing me this PR under my Review Requests 😂 Hope you're well!

initial incomplete sketch of wapm.io integration

7840960

jkbecker added the enhancement New feature or request label Mar 9, 2023

jkbecker requested review from shinypb and ceejbot March 9, 2023 14:59

got some 'splaining to do

28bc11c

ceejbot reviewed Mar 9, 2023

View reviewed changes

utils/src/registry.rs Outdated Show resolved Hide resolved

cli/src/main.rs Outdated Show resolved Hide resolved

cli/src/main.rs Outdated Show resolved Hide resolved

cli/src/main.rs Outdated Show resolved Hide resolved

utils/src/registry.rs Outdated Show resolved Hide resolved

jkbecker added 6 commits March 10, 2023 13:04

significant registry module and interface cleanup

0d6e80e

trying to please clippy

8c1cff4

c'mon, man

37427df

Merge branch 'main' of github.com:servals/serval-mesh into wapm-integ…

d8c0b49

…ration

documentation and tests for identifier parsing

33af993

downloading executables, error handling

c9c7345

jkbecker marked this pull request as ready for review March 16, 2023 20:27

jkbecker commented Mar 16, 2023

View reviewed changes

cli/src/main.rs Show resolved Hide resolved

jkbecker commented Mar 16, 2023

View reviewed changes

utils/src/registry.rs Show resolved Hide resolved

jkbecker added 7 commits March 16, 2023 16:32

once again trying to please cargo fmt

c43b2bb

actually storing the manifest & binary

d58d3bf

omfg clippy

08589ab

Some cleanup - pulling, storing, running works now

ce757ca

yes clippy I know

1bfc363

cleanup

d6fc005

improve hint description

7f4f2c3

jkbecker requested a review from ceejbot March 17, 2023 14:34

ceejbot reviewed Mar 17, 2023

View reviewed changes

jkbecker added 2 commits March 17, 2023 15:26

module names can have underscores

007d98e

better error handling

82dc7dd

jkbecker mentioned this pull request Mar 24, 2023

Switch from mdns to using the kaboodle peer mesh #48

Merged

shinypb reviewed Jan 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wapm.io integration #43

wapm.io integration #43

jkbecker commented Mar 9, 2023

ceejbot left a comment

ceejbot commented Mar 9, 2023

jkbecker commented Mar 10, 2023

ceejbot left a comment

ceejbot Mar 16, 2023

ceejbot Mar 17, 2023

jkbecker Mar 17, 2023

ceejbot Mar 17, 2023

ceejbot Mar 17, 2023

jkbecker Mar 17, 2023

shinypb left a comment

wapm.io integration #43

Are you sure you want to change the base?

wapm.io integration #43

Conversation

jkbecker commented Mar 9, 2023

ceejbot left a comment

Choose a reason for hiding this comment

ceejbot commented Mar 9, 2023

jkbecker commented Mar 10, 2023

ceejbot left a comment

Choose a reason for hiding this comment

ceejbot Mar 16, 2023

Choose a reason for hiding this comment

ceejbot Mar 17, 2023

Choose a reason for hiding this comment

jkbecker Mar 17, 2023

Choose a reason for hiding this comment

ceejbot Mar 17, 2023

Choose a reason for hiding this comment

ceejbot Mar 17, 2023

Choose a reason for hiding this comment

jkbecker Mar 17, 2023

Choose a reason for hiding this comment

shinypb left a comment

Choose a reason for hiding this comment