LibertyOS-Development / kernel

The kernel for LibertyOS.
Apache License 2.0
277 stars 12 forks source link

Rust 1.59 support #15

Closed Javier-varez closed 2 years ago

Javier-varez commented 2 years ago

There have been some changes in Rust 1.59 to stabilize the asm feature that now require action from crate users. This PR resolves them

Javier-varez commented 2 years ago

I believe this fixes #12.

danielteberian commented 2 years ago

Hello! Thank you for your interest in LibertyOS, and thank you for your PR! I have an update that I plan on releasing tonight, but I plan on merging your PR when 0.14.1 is published.

Thanks, again!

danielteberian commented 2 years ago

I have a few questions:

  1. Why did you commit the Cargo.lock file?
  2. Why did you delete the toolchain file?
  3. Are you sure that the dependencies you updated are compatible with the kernel?
Javier-varez commented 2 years ago
  1. The Cargo.lock file should ideally be kept up to date with the new versions of dependencies. Since you're building a kernel (a binary and not just a library crate) keeping track of the up-to-date Cargo.lock file helps with reproducibility (to ensure everyone uses the same dependencies). If we don't commit this change, cargo will anyway update the lock file for every user and keep that as a local change, since the version in the Cargo.toml for the x86 crate is not compatible (because of semver) with the one in Cargo.lock.
  2. I renamed it to rust-toolchain.toml, which is the new format as you can see here. This allows you to specify in more detail the actual toolchain, components and targets.
  3. I built it, run the kernel and run the tests, didn't notice any obvious problems with the new dependencies. In theory for both dependencies it is a minor number bump and a revision bump, so it should not cause any incompatibilities (according to semver). In practice, the change for the x86_64 version is not really making a difference, because this version was already in the Cargo.lock file.

So only the x86 crate has been actually updated. Looking at the git log differences between both releases the changes are pretty minimal, so should cause no problem at all:

danielteberian commented 2 years ago

Excellent work! Alright, I have another version that should release tonight, which will include a bunch of cool stuff (a completely rewritten vgabuff module, work on syscalls, some new modules for mathematics, etc.). When said version is published, I can resolve whatever conflicts there are in the PR.

I'm new to using git, so bear with me. :)

Thank you, and God bless!

Javier-varez commented 2 years ago

I just rebased the changes on top of the main branch, so you don't need to resolve the merge conflict anymore :) Thanks!

danielteberian commented 2 years ago

I'll need to rebase once more soon. I am getting ready to release the biggest version yet. I'm almost finished writing a basic shell. :)

danielteberian commented 2 years ago

Please rebase this PR.