firecracker-microvm / firecracker

Secure and fast microVMs for serverless computing.
http://firecracker-microvm.io
Apache License 2.0
25.39k stars 1.77k forks source link

[Packaging] Overwrite Firecracker version string at compile time #3057

Closed folliehiyuki closed 1 year ago

folliehiyuki commented 2 years ago

Describe the bug

FIRECRACKER_VERSION env is set at compile time by the build script build.rs (with git describe --dirty command and fallbacks to CARGO_PKG_VERSION).

This is troublesome for 3rd-party packaging systems (eg. Linux distros) as lots of them store build scripts and build applications from their release tarballs inside a git repo.

To Reproduce

Expected behaviour

A way to overwrite the version string at build time instead of relying on build.rs. Ideally something like this:

FIRECRACKER_VERSION=v10.0.0 cargo build --release

would produce the version string v10.0.0.

Additional context

Here is the output from firecracker package on AlpineLinux testing repo:

$ firecracker --version
Firecracker v3.16.0-2407-g5586fce819

Supported snapshot data format versions: v0.23.0, v0.24.0, v0.25.0, v1.0.0, v1.1.0

ArchLinux is using a hacky way (pulling the git repo at specific commit instead of downloading the release tarball) to bypass the build script.

I'd suggest setting the env FIRECRACKER_VERSION only inside devtool script and remove build.rs.

Checks

luminitavoicu commented 2 years ago

A lot of features depend on the output of Firecracker version and we do not recommend to override the version. Could you please offer us more details about why you need to do this? There is a distinction between the firecracker version and the version of the package of the Linux distribution.

folliehiyuki commented 2 years ago

A lot of features depend on the output of Firecracker version and we do not recommend to override the version.

I'm not trying to overwrite the version to a random number. What I want is to make the version string the same as it is presented in the latest released binary on Github.

Could you please offer us more details about why you need to do this?

I'll use AlpineLinux as an example here.

This behavior cannot be changed as build.rs is invoked on every cargo build command. We can't have Firecracker output the correct version string (1.1.0) even though we are building it from 1.1.0 source code.

There is a distinction between the firecracker version and the version of the package of the Linux distribution.

Yes I agree. I'm solely concerning about the Firecracker version here. Though, normally they should be the same.

alsrdn commented 2 years ago

@FollieHiyuki thanks for the feedback. We'll look into how to make this work.

alsrdn commented 2 years ago

@FollieHiyuki we discussed internally and we would like to also keep the git --describe versioning because it helps us trace back logs to a specific commit when doing testing. However, having a FIRECRACKER_VERSION_OVERRIDE variable which can be used to force the version number is very reasonable. Would this work for you? Do you build using devtool? I'm guessing a version override parameter can be added to devtool as well.

folliehiyuki commented 2 years ago

Would this work for you?

I'm totally okay with keeping the current git --describe behavior and I agree with your suggestion.

Do you build using devtool?

I mainly use AlpineLinux and as you can see in the build template (referenced above), firecracker is built directly with cargo commands.

I'm guessing a version override parameter can be added to devtool as well.

Yes, extending devtool to support both versioning methods would be nice. Although I don't use the script, it can act as a good reference for distro packagers like me.

we discussed internally and we would like to also keep the git --describe versioning because it helps us trace back logs to a specific commit when doing testing. However, having a FIRECRACKER_VERSION_OVERRIDE variable which can be used to force the version number is very reasonable.

So to sum it up, is this what we are going to have?

pb8o commented 1 year ago

Now Firecracker uses the version in Cargo.toml. Resolving.