-
Notifications
You must be signed in to change notification settings - Fork 291
Enable running wkg oci packages as bare Wasm
#3361
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
Conversation
| .with_context(|| format!("OCI config {} is not JSON", quoted_path(&lockfile_path)))?; | ||
|
|
||
| if locked_json.get("spin_lock_version").is_some() { | ||
| let mut locked_app = LockedApp::from_json(&locked_content).with_context(|| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chunk (to line 89) has not changed from line 51ff in the existing code: it has just moved down and been indented because of going inside the "is it a LockedApp" check. If I've made this function too long and/or intricate (as I strongly suspect I have), yell at me and I'll split the arms of the if into separate helpers.
kate-goldenring
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but it would be great to add an integration test for this.
651813b to
0e0a3c9
Compare
972ce24 to
263b318
Compare
Signed-off-by: itowlson <[email protected]>
263b318 to
fd6022f
Compare
|
@kate-goldenring integration test added (although right now it is going about as well as you'd expect, fie) |
Fixes #3360.
I tested this against
wkg oci push:Note that I've only tested to the "hello world" level: if we believe we need to try this on a wider range of binaries then let's chat about desirable test coverage. I'm also not sure if there's a good way to automate testing - open to advice!
It would be nice to also do it for proper packages (
wkg publish) but that's probably going to need a bit more investigation and thinky-designy-sort-of-work.