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

Semantic/Custom version confusion #318

Open
tedinski opened this issue Jul 12, 2022 · 4 comments
Open

Semantic/Custom version confusion #318

tedinski opened this issue Jul 12, 2022 · 4 comments

Comments

@tedinski
Copy link

I haven't fully verified this is reproducible yet.

I think what I'm seeing is that:

  • On Ubuntu 18.04 Docker container, I get Semantic(18, 4, 0) as the detected version
  • On Ubuntu 18.04 EC2 AMI instances, I get Custom("18.04")

Could this be the presence/absence of the lsb_release mechanism working? Looks like that might reliably return Custom: https://github.com/stanislav-tkach/os_info/blob/master/os_info/src/linux/lsb_release.rs#L14 but then the file_release mechanism will try to parse: https://github.com/stanislav-tkach/os_info/blob/master/os_info/src/version.rs#L40

Should os_info include a "version comparison" function with appropriate hacks to make this more reliable? Or at least... have I missed documentation about how to reliably detect a version? Or is this just a bug?

@stanislav-tkach
Copy link
Owner

stanislav-tkach commented Jul 16, 2022

Thank you for the report! I suppose this is a bug. Unfortunately, I won't be able to dig into this in the nearest future - I have recently changed a job, so now it takes most of my time. Though I will still be able to review and merge a fix if you would like to try to deal with this issue.

By the way, now I think that the whole semantic/custom approach is unnecessary and only adds complications. I feel like storing a version string as it is would be enough, but changing it is a breaking change.

@tedinski
Copy link
Author

By the way, now I think that the whole semantic/custom approach is unnecessary and only adds complications

I possibly agree, but there might still be value in providing some functions that help inspect OS versions, where just reverting to Option<String> or the like kinda punts that task onto users. (OTOH, maybe that version comparison stuff should be from some other existing crate? IDK)

As a bonus, if we add some of these now, people can switch over to them and then be insulated from a future breaking change to Version.

For my own purposes, I mostly just need to test for specific versions because our goal is "On this old LTS? Apply this workaround" so I was thinking of adding something like:

impl Version {
  fn looks_like(&self, version_str: &str) -> bool { ... }
}

The goal here would be that I could write os.version().looks_like("18.04") and I get what I want without worry about the future breaking change, since (with the breaking change) it'd just becomes a simple string comparison. (Whereas for now, we'd have to try applying the version parsing function on version_str if self were Semantic.)

The drawback of this particular API though is that if people give it "18.4" they'd get the same flaky behavior I'm seeing. :/ I suppose the below would be more difficult to mis-use, but then you'd definitely be committed to doing some kind of parsing, even if just internally:

impl Version {
  fn looks_like(&self, major: u64, minor: u64) { ... }
}

Would you be interested in one of these as a PR?

@stanislav-tkach
Copy link
Owner

Sorry for the late reply and thanks for such a detailed explanation of your use case!

I don't see any harm in extending the api in such way. I only have one consideration about additional dependencies. It makes sense to hide the new api under a feature if some "heavy" crates are needed for the implementation.

@ydirson
Copy link
Contributor

ydirson commented Oct 11, 2023

There is similar issue with Debian versions as VMs under XCP-ng / Xen. I will need to dig this one.

ydirson added a commit to xcp-ng/rust-os_info that referenced this issue Oct 23, 2023
When the lsb_release executable is not found the version is taken from
/etc/os_release and properly parsed.  When it is found, however, the
string was essentially not parsed, resulting in only Version::Custom
objects to be returned.  Calling the proper parsing function does the
trick, and Version::Semantic objects get created as expected.

The nearly manual handling of "rolling" for lsb_release, however,
looks very suspicous, it should likely be folded to
Version::from_string() instead.

Signed-off-by: Yann Dirson <[email protected]>
stanislav-tkach added a commit that referenced this issue Nov 24, 2023
Linux/lsb_release: parse version string (#318)
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

No branches or pull requests

3 participants