-
Notifications
You must be signed in to change notification settings - Fork 608
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
Store Lima version in the instance directory #2107
Conversation
a6e17e9
to
d0bf1bf
Compare
@AkihiroSuda Thank you for your feedback; I believe I've addressed all your comments. I switched to defining a
This code would use the new defaults with any current dev build because This should also make it easier to test the new functionality in CI without having to bump the version first. It does mean though that all instances created by any dev build of Lima after |
Current failure happens because the CI version has no tags, so the version cannot be parsed from the SHA1:
Not sure if we should pull tags in CI, so we can then test version-specific changes? |
We may pull the tags, but anyway the commit hash string shouldn't cause a fatal error. |
The problem is that we won't know the version, so all the version check will return We can make this configurable, but it feels like overkill: why do we need to support building outside of a git checkout? |
I think the version without semver prefix can be just assumed to be "latest", with a warning message.
Because somebody might be using a package built in that way? |
d0bf1bf
to
4b3b93b
Compare
That way future Lima releases can make decisions about default settings based on the Lima version that created the instance instead of the current instance, and settings for existing instances will not change just because the user updated Lima. Signed-off-by: Jan Dubois <jan.dubois@suse.com>
4b3b93b
to
10cbcd1
Compare
Ok, I think that would work for all practical scenarios; updated! |
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.
Thanks, is this still draft?
It is only still draft because of
I guess we can also consider merging as-is, and then modifying |
That way future Lima releases can make decisions about default settings based on the Lima version that created the instance instead of the current instance, and settings for existing instances will not change just because the user updated Lima.
This implements the mechanism I described in #1953 (comment). I'm leaving it in draft mode for now because I'm not sure if the
store.MinVersion
function is the right way to use the data; I originally wanted to make it an instance method, but then noticed that most location that would need to call it don't have a copy of the instance.So this PR probably needs to wait until we have a companion PR that makes use of this new mechanism. Or I could drop the
MinVersion
function for now and leave it for the first PR that will take advantage of the data.Either way, feedback welcome!