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

docs(sips/006-spin-plugins): Fix typo in package.os field definition: osx -> macos #3011

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

PetarKirov
Copy link
Contributor

At the time of writing (Spin v3.1.2), the Os enum is defined as follows:

enum Os {
    Linux,
    Macos,
    Windows,
}

This commit adjust the SIP to match the implementation by renaming osx to macos. Note that the SIP later on uses macos as part of the URL in one of the examples.

Background

Our team is using the Nix to package (build from source and "install") a Spin plugin that we developed. In process we created a Nix function to generate the spin plugin manifest file by compiling the source code, archiving and hashing it and filling the rest of manifest fields, following the SIP006 spec. Specifically, we were mapping Nix kernel name to spin plugin package os like so:

{
  linux = "linux";
  darwin = "osx";
}.${hostPlatform.parsed.kernel.name};

Testing on macOS immediately highlighted the problem:

invalid manifest for plugin '' at /nix/store/ycqy6f0pqg8c7fm894hbgjg5cfss4sw3-trigger-oracle.json: unknown variant `osx`, expected one of `linux`, `macos`, `windows` at line 9 column 17

…n (`osx` -> `macos`)

At the time of writing (Spin v3.1.2), the `Os` enum is [defined][0] as follows:

```rs
enum Os {
    Linux,
    Macos,
    Windows,
}
```

This commit adjust the SIP to match the implementation by renaming `osx` to `macos`. Note that the SIP later on uses `macos` as part of the URL in one of the examples.

[0]: https://github.com/fermyon/spin/blob/v3.1.2/crates/plugins/src/manifest.rs#L113

Signed-off-by: Petar Kirov <[email protected]>
Copy link
Collaborator

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

Thanks for spotting this!

@itowlson itowlson enabled auto-merge February 12, 2025 21:16
@itowlson itowlson merged commit 0f8b7ad into spinframework:main Feb 12, 2025
17 checks passed
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.

2 participants