Conversation
jaybosamiya-ms
left a comment
There was a problem hiding this comment.
Thanks Weiteng, this is similar to a (broader) refactoring I was planning do but haven't had a chance to finish it up. There are some minor cleanups possible here, but I don't want to block you especially since I plan to do some of that cleanup later on anyways. Please feel free to merge, thanks!
| /// A trait required for file systems to be used in the shim. | ||
| pub trait ShimFS: litebox::fs::FileSystem + Send + Sync + 'static {} | ||
| impl<T: litebox::fs::FileSystem + Send + Sync + 'static> ShimFS for T {} |
There was a problem hiding this comment.
This portion is the key change, as I understand it, right?
This is similar to a (broader) refactoring I was planning do but haven't had a chance to finish it up. That refactoring can build on top of this one and can clean it up, so please feel free to merge this as is.
There was a problem hiding this comment.
I assume you've already tested this, but any particular reason it needs to be Send + Sync + 'static? Can we relax any/all of them? It'd be nice to have it be the weakest requirements needed. If it cannot be weakened by removing those traits, that is fine, but just checking.
There was a problem hiding this comment.
'static is required by Box
litebox/litebox_shim_linux/src/syscalls/process.rs
Lines 550 to 568 in 1e6a790
Send and Sync are required by Arc in multiple places for sharing across threads, e.g.,
|
🤖 SemverChecks 🤖 Click for details |
I was trying to add 9p fs and use it for SNP runner, then I realized our Linux shim decides the fs to use not runner. This PR makes Linux shim generic on fs so that runner can pick whatever fs.