Skip to content

update require_kernel_version string to handle "_" in version string #1316

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

Merged
merged 1 commit into from
Oct 16, 2020

Conversation

ritzk
Copy link
Contributor

@ritzk ritzk commented Oct 16, 2020

test code breaks on fc333

$ cargo test
failures:

---- sys::test_socket::recvfrom::udp_offload::gro stdout ----
thread 'sys::test_socket::recvfrom::udp_offload::gro' panicked at 'called `Result::unwrap()` on an `Err` value: ParseError("Extra junk after valid version: _64")', test/sys/test_socket.rs:292:13

this is due underscore in release string( arch/x86_64), which is not supported by semver.

$ uname -r
5.8.14-300.fc33.x86_64

@ritzk ritzk changed the title update version string to handle "_" in version string update require_kernel_version string to handle "_" in version string Oct 16, 2020
@asomers
Copy link
Member

asomers commented Oct 16, 2020

So the problem here is that we're parsing the Linux kernel version as a Semver string, even though it isn't. And _ isn't allowed anywhere in a Semver string. Your solution is reasonable. But could you please add a comment explaining why?

test code breaks on fedora 33
```
$ cargo test
failures:

---- sys::test_socket::recvfrom::udp_offload::gro stdout ----
thread 'sys::test_socket::recvfrom::udp_offload::gro' panicked at 'called `Result::unwrap()` on an `Err` value: ParseError("Extra junk after valid version: _64")', test/sys/test_socket.rs:292:13
```

this is due underscore in release string( arch/x86_64), which is not supported by semver.
```
$ uname -r
5.8.14-300.fc33.x86_64
```

Replace the underscore with hypen to provide a consistent sematic.
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot added a commit that referenced this pull request Oct 16, 2020
1316: update require_kernel_version string to handle "_" in version string r=asomers a=ritzk

 test code breaks on fc333

```
$ cargo test
failures:

---- sys::test_socket::recvfrom::udp_offload::gro stdout ----
thread 'sys::test_socket::recvfrom::udp_offload::gro' panicked at 'called `Result::unwrap()` on an `Err` value: ParseError("Extra junk after valid version: _64")', test/sys/test_socket.rs:292:13
```

this is due underscore in release string( arch/x86_64), which is not supported by semver.
```
$ uname -r
5.8.14-300.fc33.x86_64

```

Co-authored-by: Ritesh Khadgaray <[email protected]>
@bors
Copy link
Contributor

bors bot commented Oct 16, 2020

Build failed:

@asomers
Copy link
Member

asomers commented Oct 16, 2020

bors retry

@bors
Copy link
Contributor

bors bot commented Oct 16, 2020

Build succeeded:

@bors bors bot merged commit c51f63a into nix-rust:master Oct 16, 2020
@ritzk ritzk deleted the version branch October 18, 2020 02:37
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