kubkon edited PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
pchickey submitted PR Review.
pchickey created PR Review Comment:
can you just use the
Into<u32>
implementation?
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
I agree it'd be good to have one way to do this, rather than two. Could I suggest preferring
.inner()
over.into()
? Most of the code in wasi-common should treat these as opaque, so it'd be nice for the places where we do need to peek inside to stand out.
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
fd_pool
andentries
need to stay in sync with each other. How awkward would it be to have a singleRefCell
that holds a struct containing both?
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
And similar here? If so, we may not even need the
contains_key
method, which would be nice because it can be tricky to use without look-before-you-leap hazards.
sunfishcode created PR Review Comment:
Instead of
from
, what would you think of naming this methodfrom_raw
, to emphasize that this call is not something one should do often.
sunfishcode created PR Review Comment:
Implementing this as a trait for
WasiCtx
is an interesting idea, and it is pretty fun to doctx.fd_close
as the code does above, but does this make it harder to share aWasiCtx
between snapshot versions in the same wasm module?
sunfishcode created PR Review Comment:
This isn't strictly incorrect (at least while we only have one thread), but it feels awkward. Would it work to do something like this? A little verbose still, but it only does one borrow and one entries lookup.
let maybe = Ref::map(self.entries.borrow(), |entries| { entries.get(&fd) }); if maybe.is_none() { return Err(Errno::BadF); } Ref::map(maybe, |maybe_entry| maybe_entry.unwrap()) }?
sunfishcode created PR Review Comment:
What does
bc
stand for?
sunfishcode created PR Review Comment:
Is there a path to where these
trace!
calls could be auto-generated by wiggle?
sunfishcode created PR Review Comment:
Can you say more about the
unsafe
here? I assume wiggle has done the bounds checking before we get here, is that right?
sunfishcode created PR Review Comment:
Is it temporary to add this hook in
wig
here?
kubkon submitted PR Review.
kubkon created PR Review Comment:
Ah, excellent observation!
kubkon submitted PR Review.
kubkon created PR Review Comment:
Awesome, this is exactly what I wanted to do and couldn't figure out how. Thanks! :-D
kubkon submitted PR Review.
kubkon created PR Review Comment:
Right, I've actually tweaked this only so that it builds. I was going to cc you about the
fs
module and how we'd want to handle things here :-)
kubkon submitted PR Review.
kubkon created PR Review Comment:
Excellent question! Thanks for bringing it up! @pchickey and I had some thoughts about this, and as far as I can tell, he would rather avoid adding logging to
wiggle
(which is where we'd want this to end up actually). I haven't put that much thought to it just yet, hence why I left the traces inwasi-common
, but I'm open to suggestions here!
sunfishcode created PR Review Comment:
Ah, I missed the part of your earlier comment where you explained this ^_^.
sunfishcode submitted PR Review.
kubkon submitted PR Review.
kubkon created PR Review Comment:
Excellent question! So I think it won't be that difficult. However, I suggest to leave that for a subsequent PR. I'm purposely migrated all impls into one module and directly into the trait impl so that we can have a better idea how to tackle the problem of multiple snapshots/polyfill. Does it make sense?
kubkon submitted PR Review.
kubkon created PR Review Comment:
You got me, I was just following @alexcrichton's example in
wiggle
's tests :-} I'm happy to change it to something more meaningful, sayborrows
?
kubkon submitted PR Review.
kubkon created PR Review Comment:
That's correct,
as_raw
does the bounds checking. We needunsafe
here since we're dereferencing a raw*mut _
pointer, and in this particular case, we're obtaining&str
. The signature of this function here is:fn as_raw(...) -> Result<*mut _, GuestError>This is unsafe since we cannot really track the references here other than by using
GuestBorrows
. More on that here.
kubkon created PR Review Comment:
Yep, until we figure out where to put the glue macro for
wasmtime-wasi
/wasmtime
, i.e., the one that's autogenerating theFunc
wrappers, and whatnot.
kubkon submitted PR Review.
kubkon submitted PR Review.
kubkon created PR Review Comment:
I'll try to beat everyone to the question here. I'm not sure how to handle this better with the current state of
wiggle
(which is not to say we can't modifywiggle
to accommodate this case). So, I was forced to create two instances ofGuestBorrows
since it is possible for the twopath
s that are handed to us from guest to point at the same memory location (one use case being for instance symlink loops, etc.).I was thinking, we could potentially check for that by comparing the pointers (if they implemented
PartialEq
), but it still feels clunky. Perhaps we should revisit the concept of immutability inwiggle
? That is, differentiate immutable vs mutable borrows? In this case, I reckon it should be fine to borrow the same location immutably multiple times. Thoughts? @alexcrichton what do you reckon?
kubkon edited PR Review Comment.
kubkon submitted PR Review.
kubkon created PR Review Comment:
@sunfishcode Note that we don't have
unsafe
's in methods' signatures. I was somewhat worried about this since we're still dealing with file descriptors and other OS handles, but perhaps the entry points to WASI syscalls are OK being safe especially given that WASI fd is now a handle type? What do you think?
pchickey created PR Review Comment:
This is a great question. One alternative to creating two different borrowed path strings is to clone two path strings from the wasm memory into Rust owned
String
s. I expect, since paths shouldn't be too big, that is the right design choice here, except for the possibility of a DoS attack by creating two 4gb Strings here.
pchickey submitted PR Review.
pchickey edited PR Review Comment.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
Differentiating between mutable and immutable borrows sounds like a good way to go.
For example, POSIX doesn't say anything about
link
's arguments aliasing or not, but most functions with output buffers, such as readlink are defined withrestrict
qualifiers, meaning the output buffer is expected to not overlap with any of the input buffers. This also echos the Rust's borrow checker: you can have many aliasing immutable references, but a mutable reference must be unaliased.
pchickey submitted PR Review.
pchickey created PR Review Comment:
I've thought about this and I'm fairly convinced the right thing, for now, is to leave a nice comment that outlines the design decisions you just did in this thread.
pchickey edited PR Review Comment.
pchickey submitted PR Review.
pchickey created PR Review Comment:
its short for "borrow checker".
alexcrichton created PR Review Comment:
Could there be a
log
feature ofwiggle
which generates log statements? That way wasmtime could turn it on but if it's not needed in lucet it could be disabled?
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I'd actually imagine that it makes it easier to share a
WasiCtx
between versions because we could implement both traits for one struct!
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
Crazy idea that I haven't thought all the way through yet: Would it work if we made
Fd
work likestd::fs::File
?
- Make
Fd::from_raw
unsafe? , similar tofrom_raw_fd
being unsafe.- Make
Fd
not implementCopy
. ChangeFd
arguments to&Fd
.- Make
Fd
implementDrop
and have it automatically close. This might be hard though. Is there a way in Rust to makedrop
private, so that you have to pass it back to the context?
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
FWIW stylistically I'd group sets of statements that are all "participating in the unsafe" together rather than trying to minimize the
unsafe
block to just one expression.
alexcrichton created PR Review Comment:
I'm not sure if this is how it was implemented before, but I'm actually pretty surprised by this. I thought that one of the purposes of
GuestBorrows
and stuff was to pass wasm buffers directly to syscalls, no?
alexcrichton created PR Review Comment:
One of the downsides of this returning
RefMut
directly is that it runs the risk of making it easy to panic in wasi-common code by accident I think. For example if while you have aRefMut
you call back into wasm any reentrancy back into wasi would cause a panic due to a failed borrow.It may just be worth documenting that for now, but wanted to point out that this may be a panic waiting to happen in the future.
alexcrichton created PR Review Comment:
Also, I don't think this function needs to be
unsafe
?
alexcrichton created PR Review Comment:
I think the usage of
GuestBorrows
here technically may not be correct. There's no real need to do any borrow checking here because there's no long-lived borrows, this is effectively just a "given this guest slice pointer write in these bytes' which is in general a safe operation (likeGuestPtr::write
is)
alexcrichton created PR Review Comment:
As a side note, this is not a safe function in general because the virtual implementation could reenter wasm. I think we'll want to make the virtual traits
unsafe
since you need to assert you don't reenter wasm or try to reborrow/read/etc from wasm memory in the underlying implementations.
alexcrichton created PR Review Comment:
I'd have the same question as before with
pread
here about passing raw wasm buffers to the underlying implementation.
alexcrichton created PR Review Comment:
There's two
slices
vecs here, but I think there only needs to be one?
alexcrichton created PR Review Comment:
I don't actually think that
GuestBorrows
is really needed here at all because there are no long-lived pointers, I think it's fine to do what this is currently doing, but I might recommend making it a bit more concise with the comment I made above
alexcrichton created PR Review Comment:
This idiom is showing up in a few places ("given this slice write this slice"), so it might be good to encapsulate this in a safe function perhaps?
alexcrichton created PR Review Comment:
This
read
is unsafe in combination with the long-lived borrows here, you'll need to add theGuestPtr<Iovec>
to theGuestBorrows
to make sure this pointer doesn't overlap with previous buffers.
alexcrichton created PR Review Comment:
Hm does
as_raw
oras_array
not work with zero-length arrays? I would naively expect this condition to not be necessary.
alexcrichton created PR Review Comment:
Could the idiom of unsafely getting a
&str
and then callingpath::get
be wrapped up in a safe function tounsafe
doesn't have to be used each time a path is resolved?
alexcrichton created PR Review Comment:
Technically the FIXME items elsewhere are equally applicable here, there's not really any fundamental reason why these two guest buffers shouldn't be allowed to overlap.
pchickey submitted PR Review.
pchickey created PR Review Comment:
Unfortunately, this is how it was implemented before. I think we should correct it in a follow-up PR.
pchickey submitted PR Review.
pchickey created PR Review Comment:
Agreed, this could be a method on GuestPtr<[T]>
pchickey edited PR Review Comment.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Ah ok, in that case I think we can probably avoid the
GuestBorrows
and collecting slices, since all the writes/reads only happen in one go and there's no need for long-lived borrows.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
The
unsafe
is there for the raw file descriptor. This is a convention we picked up fromstd::fs::File::from_raw_fd
, which isn't necessarily memory-unsafe, but extends the concept of safety to include I/O safety. Or an alternative interpretation is that file descriptors are pointers into a different address space (they can dangle, alias, be out of bounds, be bogus, etc.), and memory safety rules should apply to that address space too.That said, I think we're still working out exactly how we want to approach this. See https://github.com/bytecodealliance/wasmtime/pull/1202/files#r395300196 for an example.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
On further consideration, the
Drop
part makes this unworkable. I'm going to investigate other approaches.
pchickey submitted PR Review.
pchickey created PR Review Comment:
Hmm, I don't think I understand what you mean here by
GuestBorrows
not needed here at all. I'm probably missing something but let me outline the things I do believe:I agree that in this case, the borrow checking is vacuous. But there isn't presently another method besides
GuestPtr::as_raw
that allows us to get a&str
out of thisGuestPtr<str>
. We could add a method to GuestPtr that returns an ownedString
, but I fear that is a DoS vector because the guest will control the size of that String, and therefore gets to allocate in Rust's heap. We haven't yet rigorously audited this codebase for DoS vectors, of course, but I'm wary of adding something I know I'll have to forbid actually using in production.I do not think we should create a shortcut variant of
GuestPtr::as_raw
to omit borrow checking which is only safe if the user ensures borrows are always short-lived, because I could see that leading to bugs when someone less familiar with the safety properties updates the code. Maybe that is too paranoid?
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
Good point. Do you think we could do it in a subsequent PR and just leave reminder "TODO" comment now?
kubkon submitted PR Review.
kubkon created PR Review Comment:
Yep, @pchickey is right, I've just adapted the "prior art" to
wiggle-runtime
types. I'm happy to look at this though, but @pchickey might be right that we should perhaps increment in-tree?
kubkon submitted PR Review.
kubkon created PR Review Comment:
Cool, I'm fine with that. I have to admit when porting the syscalls to
wiggle-runtime
I was following thefd_pread
example fromwiggle
's WASI testcase :-)
kubkon submitted PR Review.
kubkon created PR Review Comment:
I guess you may be right, however, without the
GuestBorrows
I cannot obtain a mutable ref to[u8]
, and there's not other simple way of copying the contents ofarg_bytes
into theargv_buf
pointer currently. The only other way I can see is manually incrementingargv_buf
pointer and copying byte-by-byte fromarg_bytes
. Unless I'm perhaps missing something here?
kubkon submitted PR Review.
kubkon created PR Review Comment:
Cool, I'll leave a comment in for now, and we can figure out a fix later. As far as
unsafe
is concerned, as @sunfishcode pointed out, this is "prior art" and was there to signal the unsafety of using I/O. FWIW I'd leave that in for now and we can figure out how to deal with all the unsafety in a future PR?
kubkon submitted PR Review.
kubkon created PR Review Comment:
I like the idea but yeah,
Drop
will not work like this, unless we made the context object static/global. Having said that your suggestion aligns with my mental model ofFd
100%, so perhaps we could come up with something close to it but not necessarily one-to-one.
kubkon submitted PR Review.
kubkon created PR Review Comment:
Exactly as @pchickey explained, obtaining a reference to a slice-type object in Rust from
wiggle
is currently only possible throughas_raw
method which requires aGuestBorrows
object in scope.
kubkon submitted PR Review.
kubkon created PR Review Comment:
When you say "encapsulate in a safe function" do you mean as a helper in
wasi-common
or a method ofGuestPtr<'_, [T]>
? I'm partial to the latter approach, and agreed that a method like this would be most useful!
kubkon submitted PR Review.
kubkon created PR Review Comment:
Excellent observation, and as I explained above, I've only made enough to make
wiggle
work withwasi-common
. I'm happy to investigate this further though!
kubkon submitted PR Review.
kubkon created PR Review Comment:
Hmm, out of curiosity, is the example here wrong then?
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
Oh, this must be a remnant of my hack to not stifle progress with
wasi-common
while some required changes towiggle
were in progress. This change is no longer part of this PR, so please ignore it.
kubkon submitted PR Review.
kubkon created PR Review Comment:
Oh, I believe what @alexcrichton suggests sounds like _the_ solution to this problem. @pchickey @sunfishcode what do you guys reckon? Oh, and until that lands in
wiggle
, my suggestion would be to leave thetrace
s as-is inwasi-common
. What do you think?
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
Oh, and I think we should do the same for
wasi-common
as well. We've got a couplelog
calls here and there for debugging and what not, and I believe that @pchickey would like to have a version stripped off all altogether for lucet/xqd.
kubkon submitted PR Review.
kubkon created PR Review Comment:
Not suggesting we do it immediately in this PR, but we should have it in mind. :-)
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
You're right, I'd expect this to work as well. It turns out
wiggle_runtime::Region
didn't accommodate for zero-lengthRegion
s. I've now fixed that in #1366. I'll leave a comment in to remove the check once it lands inwiggle-runtime
.
kubkon created PR Review Comment:
I've simplified this now, so have a look and lemme know what you reckon!
kubkon submitted PR Review.
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
BTW @alexcrichton, lemme know what's your plan for this, i.e., what to use instead of
RefMut
! :-)
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
sunfishcode created PR Review Comment:
Ok, thinking this through more carefully, maybe we can do this without the
Drop
part. So makehandle
types not implementCopy
, passhandle
arguments by reference rather than by value, and make the raw constructorunsafe
. That's not the same asstd::fs::File
, but it still has some nice properties. We just need to be careful when calling the raw constructor, but there aren't many places where we need to do that outside of the generated wrappers. Does that sound feasible?If you'd like to defer this to a later PR, that's fine too.
sunfishcode submitted PR Review.
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon created PR Review Comment:
OK, I've refactored both
pread
andpwrite
so that we no longer need temp storage buffer. Instead, we're using slices bound to guest's memory directly (asIoSliceMut
andIoSlice
respectively), and both ops now essentially consists of two stepsSeekFrom::Start(offset)
, followed byread_vectored(...)
andwrite_vectored(...)
respectively.
kubkon submitted PR Review.
kubkon submitted PR Review.
kubkon created PR Review Comment:
Sounds good, and yeah, if we could defer to a subsequent PR, that would be perfect IMHO. There's already a substantial amount of tweaks I have to apply and I'm worried more and more will slip away. Is that OK?
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
I've done just that. Now
path::get
takes&GuestPtr<'_, str>
directly and does borrowing internally. This actually, at the surface at least, gets rid of the double-immutable-borrow problem.
kubkon created PR Review Comment:
Hmm, but wouldn't
borrow_slice
take care of it for us here?
kubkon submitted PR Review.
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
@pchickey What do you think of having a
from_raw
method for handles inwiggle
?
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon created PR Review Comment:
Ok, I've now added an aux struct called
EntryTable
which holds bothfd_pool
andentries
, and insideWasiCtx
it's behind a singleRefCell
thus both working in sync.
kubkon submitted PR Review.
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
Ok, since #1366 landed, I've now removed the check and everything works as expected :+1:
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
@pchickey er sorry I don't think I was being very clear! I don't think we should remove the
GuestBorrows
argument or use owned strings here, I mean that each of these is resolved independently and there's never any overlapping borrows so documenting fixmes and/or hacks isn't necessary because each path is getting sucked into an owned object anyway so borrows never last long enough for runtime borrow-checking to be that relevant.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Hm ok so I would strongly recommend removing
unsafe
here. There's already enoughunsafe
as-is with dealing with raw wasm memory, and this feels like it's overkill.The unsafety around libstd's I/O was specifically about providing a guarantee that each entity holding a file descriptor owns that file descriptor. That allows safe abstractions around mmap and stuff (in theory). I really think we should not be adding
unsafe
here lightly, and we may just not have that guarantee for wasi-common.The unsafety here also doesn't really seem to stop anything in practice, every callsite just wraps a
unsafe { ... }
block which just makes it more verbose to call these methods rather than providing the "oh I should think about this" speedbump it seems.
kubkon submitted PR Review.
kubkon created PR Review Comment:
Ok, this proves to be more difficult than I originally envisioned. If I follow your suggestion, than we end up with a lifetime mismatch since the first
maybe
will contain weirdness of the like&Option<&Entry>
, and the compiler cannot assign approriate lifetimes to the finalRef<'_, Entry>
. Perhaps when we revisit the use ofRefCell
inwasi-common
altogether, this will no longer be an issue?
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Oh no nevermind my interpretation is wrong, I think this should be fine.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Oh I should also mention, I think the only real alternative to returning
RefMut<Entry>
is to returnRc<Entry>
. That will likely be required anyway if we want to make this multithreaded at some point (probably don't want to hold locks for entireties of syscalls). I don't know of a great way to otherwise handle this. For now it seems fine to document how you need to be careful about holding a borrow for too long.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Oh sorry my point is that persisting the same
GuestBorrows
for the entirety of this method I believe is not necessary. Each write can use an independent instance ofGuestBorrows
since there's no need to prevent overlap in this method.
kubkon submitted PR Review.
kubkon created PR Review Comment:
Hmm, right, I think I get it.
Rc
should still allow for obtaining&mut
of the wrapped type provided there is only one at play at any one time, right? And then in the multithreaded case, we'd just swap it forArc
?
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I've posted https://github.com/bytecodealliance/wasmtime/pull/1367, and my point here is that if that's used then there's no need for
GuestBorrows
in this method and no need for this method to be unsafe.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
FWIW I've posted https://github.com/bytecodealliance/wasmtime/pull/1367 to cover this.
alexcrichton created PR Review Comment:
In terms of semantics, I'm not sure if
pread
is specified to change the file cursor or not, but I believe this implementation does? Might be good to double check...
kubkon submitted PR Review.
kubkon created PR Review Comment:
That's perfect! It'll clean up the code nicely! :sunglasses:
kubkon submitted PR Review.
kubkon created PR Review Comment:
Oh crap, you're right, it shouldn't! I forgot to rewind the cursor! Thanks for bringing it up!
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I don't believe that
&mut
is needed anywhere actually? For example all methods on I/O files are implemented for&T
so you have to do something like(&entry.file).read(...)
which is a bit wonky, but there shouldn't be a fundamental need to ever mutate anEntry
.If there is though then a
RefCell
can always be used, and the borrows would need to be scoped in duration.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Yes that should be fine
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
Fixed! Thanks for spotting that one!
kubkon created PR Review Comment:
I left the comment in
virtfs.rs
module.
kubkon submitted PR Review.
kubkon submitted PR Review.
kubkon created PR Review Comment:
You're absolutely right, we could easily operate on refs since we have impl on
&T
for I/O in Rust. That's cool. I'll make this change here already to clean up the code as much as possible (i.e., removing themut
s). In terms ofRc
, I'm guessing we'd still want to wrap the map holding theEntry
s in anRc
?
kubkon submitted PR Review.
kubkon created PR Review Comment:
Right, we do since we still need to able to mutate
EntryTable
and notEntry
!
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
The map will want to be in a
RefCell
for sure (wasRc
a typo?) because we're modifying it behind a shared reference, and then each entry would be in its ownRc
to get handed out without holding a long-lived borrow.
kubkon submitted PR Review.
kubkon created PR Review Comment:
Sorry, yeah, meant
RefCell
.Right, makes sense!
kubkon submitted PR Review.
kubkon created PR Review Comment:
I was wrong actually. We do need to be able to mutate
Entry
in two places:fd_fdstat_set_rights
andfd_fdstat_set_flags
.
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
@sunfishcode would you agree at removing the
unsafe
here?
sunfishcode created PR Review Comment:
Yes. I've been exploring the "pass Fd by reference" idea more, and I'm sufficiently confident that that will provide a way for
get_entry
andget_entry_mut
to be safe functions, even considering I/O safety, so I think we can leave the unsafe off for now because that's where they'll end up anyway.
sunfishcode submitted PR Review.
kubkon submitted PR Review.
kubkon created PR Review Comment:
@alexcrichton Would you be OK if we parked the
&
vs&mut
cleanup inwasi-common
? Changing to&
in most cases while absolutely correct will require some more work withvirtfs
module (at quick glance, not everything is behind aRefCell
or alike to be able to mutate the interior behind&
.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Oh sorry yeah, I think cleaning up later is fine, but it'd be nice to remove the
unsafe
before landing
kubkon submitted PR Review.
kubkon created PR Review Comment:
Absolutely, I'm on it :+1:
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon requested alexcrichton for a review on PR #1202.
kubkon requested alexcrichton, and sunfishcode for a review on PR #1202.
kubkon requested alexcrichton, pchickey, and sunfishcode for a review on PR #1202.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
This looks like it may be buggy? I think
environ_buf_offset
may not be needed here because we're already offsetingenviron_buf
on each iteration of the loop?
sunfishcode submitted PR Review.
sunfishcode submitted PR Review.
sunfishcode created PR Review Comment:
Ok, if it gets awkward, then I agree it makes sense to leave the code as-is for now.
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
kubkon submitted PR Review.
kubkon created PR Review Comment:
@alexcrichton Right on, thanks for spotting that one! I believe I've fixed it in 5e75830, but if you could double check, that'd be awesome!
kubkon updated PR #1202 from wiggle-time
to master
:
It's [
wiggle
] time!This PR is the beginning of a journey which aims at replacing
wig
crate withwiggle
. @pchickey and myself have been working onwiggle
for over a month now, and we feel we got to the point where it could successfully be used inwasi-common
,wasmtime
andlucet
. We also agreed that if any functionality is still missing, we should iterate in-tree if possible. Anyhow, this is a draft PR so things will invariably change (a lot!), but I wanted to get it out there sooner rather than later so that I can get some meaningful feedback early on and work on addressing that when things are still plastic.[
wiggle
]: https://github.com/kubkon/wiggle
pchickey submitted PR Review.
kubkon merged PR #1202.
Last updated: Dec 23 2024 at 12:05 UTC