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

feat(plugins): provide additional build info to plugins #1492

Merged
merged 1 commit into from May 17, 2023

Conversation

cardoso
Copy link
Contributor

@cardoso cardoso commented May 15, 2023

I'm building a plugin that manages spin versions. It would be useful to understand where the build comes from, so it can be upgraded/downgraded/recovered safely.

I had to update vergen to version 8 and did some cleaning up on the way.

The additional info is sent to the plugin child process envs:

SPIN_BIN_PATH=/Users/cardoso/.cargo/bin/spin
SPIN_BRANCH=feat/more_build_info
SPIN_BUILD_DATE=2023-05-15
SPIN_COMMIT_DATE=2023-05-15
SPIN_COMMIT_SHA=49fb11b
SPIN_DEBUG=false
SPIN_TARGET_TRIPLE=aarch64-apple-darwin
SPIN_VERSION=1.2.0-pre0
SPIN_VERSION_MAJOR=1
SPIN_VERSION_MINOR=2
SPIN_VERSION_PRE=pre0

@cardoso cardoso changed the title feat(plugins/cli): provide build info on CLI and to plugins feat(plugins/cli): provide additional build info to plugins and the CLI May 15, 2023
Copy link
Contributor

@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.

This looks great (and I'm excited to see your plugin - sounds super useful). My only reservation is the change to the existing behaviour of spin --version. Our docs and learning materials do currently guide people to --version and I'm wary of confronting new users with this much detail.

My vote on this would be to (as you mention) follow Cargo and have -V/--version show the short form and -V -v show the long form. But other folks may be less change-averse - happy to wait for others to weigh in.

And thanks again - I apologise for getting hung up on what may seem like a niggle - the content of the PR looks great.

src/build_info.rs Show resolved Hide resolved
src/bin/spin.rs Outdated Show resolved Hide resolved
@itowlson
Copy link
Contributor

@cardoso cardoso changed the title feat(plugins/cli): provide additional build info to plugins and the CLI feat(plugins): provide additional build info to plugins May 17, 2023
Signed-off-by: Matheus Cardoso <matheus@cardo.so>
@cardoso
Copy link
Contributor Author

cardoso commented May 17, 2023

@itowlson I opted to reduce the scope here to just sending the env vars to the plugin and make a separate PR for the CLI eventually

Copy link
Contributor

@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.

This looks great! 100% agree on reducing scope: as you say, this unblocks the plugin scenario, and we can always iterate on the UI. Thanks!

Copy link
Contributor

@kate-goldenring kate-goldenring left a comment

Choose a reason for hiding this comment

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

@cardoso this is great providing plugins with more information about Spin. I created an issue to also update developer docs to call out what env vars are passed to plugins fermyon/developer#632.

Copy link
Contributor

@karthik2804 karthik2804 left a comment

Choose a reason for hiding this comment

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

This is great! Thanks!

@itowlson itowlson merged commit c4fbd08 into fermyon:main May 17, 2023
9 checks passed
@itowlson
Copy link
Contributor

@cardoso This just missed the cutoff for Spin 1.2, so for now you will need to use canary or HEAD for your plugin development. I can't commit on when the next release will be; we are tracking a roughly monthly schedule at the moment, but absolutely no promises!

@cardoso
Copy link
Contributor Author

cardoso commented May 18, 2023

No problem @itowlson . I still need to submit more PRs to have enough features for the plugin to be solid. Hopefully 1.3 will suffice.

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.

None yet

4 participants