Skip to content

Add WASI support for the 0.1 branch#271

Merged
pfmooney merged 1 commit into
time-rs:v0.1from
jedisct1:0.1-wasi
Aug 26, 2020
Merged

Add WASI support for the 0.1 branch#271
pfmooney merged 1 commit into
time-rs:v0.1from
jedisct1:0.1-wasi

Conversation

@jedisct1

Copy link
Copy Markdown

This adds WASI support to the v0.1 branch, since some crates still depend on 0.1.x.

@jhpratt

jhpratt commented Aug 25, 2020

Copy link
Copy Markdown
Member

I'll leave the decision up to others, but I feel obliged to mention that adding the field to SteadyTime is a breaking change.

@jhpratt jhpratt added the v0.1 label Aug 25, 2020
@jhpratt jhpratt requested a review from a team August 25, 2020 06:15
@KodrAus

KodrAus commented Aug 25, 2020

Copy link
Copy Markdown

Personally, I don’t think we should really backport or make non-trivial (especially breaking) changes to the 0.1 branch, otherwise you’re effectively having to carry two continually diverging codebases forward.

@jhpratt

jhpratt commented Aug 25, 2020

Copy link
Copy Markdown
Member

Agreed. Don't get me wrong, I have serious reservations about any further changes to 0.1. Obviously I am the only one with publishing permissions nowadays, but it is far from my decision alone. I need to be convinced that a further release is truly worth it.

Time 0.3 is in progress, which will solve the issues that came up in 0.2, especially regarding it being too heavy as a dependency. I know some crates (not naming any specifically) have decided not to update to 0.2 because of those concerns. That is their choice. While I understand and support the authors of these crates, everyone must understand that that decision has impacts and downstream implications.

Comment thread src/sys.rs
@jedisct1

Copy link
Copy Markdown
Author

As pointer out by Jacob, without naming them, widely used dependencies still depend on time 0.1.

In order be compiled to WebAssembly, the current practice is to use forks of these dependencies that embed modified versions of time 0.1 (with the changes from this PR). A new 0.1 version would prevent such hacks from proliferating.

@pfmooney

pfmooney commented Aug 25, 2020

Copy link
Copy Markdown

In order be compiled to WebAssembly, the current practice is to use forks of these dependencies that embed modified versions of time 0.1 (with the changes from this PR). A new 0.1 version would prevent such hacks from proliferating.

I'm in favor of adding the implementation for wasi. Leaving the wasm32 definition alone would maintain the status quo there.

@jhpratt

jhpratt commented Aug 25, 2020

Copy link
Copy Markdown
Member

@jedisct1 That's not quite what I said. My intent was to say that some crates have rejected a potential upgrade to 0.2. Yes, a number of crates haven't made a decision, but that's not what I was referring to. The fact that 0.1 doesn't compile to wasi is one of the implications I was referring to — 0.1 is not as portable, plain and simple.

@jedisct1

Copy link
Copy Markdown
Author

The stubs for wasm32 without wasi or emscripten have been restored.

@pfmooney

Copy link
Copy Markdown

The stubs for wasm32 without wasi or emscripten have been restored.

Thanks. Would you mind doing a squash and rebase?
I've fixed the CI tests so they should be able to run clean again.

@jedisct1

Copy link
Copy Markdown
Author

Rebased and squashed!

@pfmooney pfmooney merged commit d989b5f into time-rs:v0.1 Aug 26, 2020
@pfmooney

Copy link
Copy Markdown

Rebased and squashed!

And merged. Thanks

@jhpratt

jhpratt commented Aug 26, 2020

Copy link
Copy Markdown
Member

Just curious, why pin the wasi dependency?

@jedisct1

Copy link
Copy Markdown
Author

Because WASI is still a moving target, and 0.10 is a -snapshot version. But this is the version supported by all current runtimes.

@pfmooney

Copy link
Copy Markdown

Because WASI is still a moving target, and 0.10 is a -snapshot version. But this is the version supported by all current runtimes.

Are you looking for an immediate time-rs release, now that this is merged, or is just having it integrated fine while the wasi stuff remains a moving target?

@jedisct1

jedisct1 commented Aug 26, 2020

Copy link
Copy Markdown
Author

Having an immediate time-rs would be fantastic.

We still have a ton of work to do before WASI leaves the snapshot state, and it is unlikely to happen this year.

@jhpratt

jhpratt commented Aug 26, 2020

Copy link
Copy Markdown
Member

I'll have a new 0.1 release out later today.

@pfmooney

Copy link
Copy Markdown

I'll have a new 0.1 release out later today.

Tagged and ready to go @ 4235bd8, whenever you have time to publish to crates.io.

Thanks for taking a look.

@jhpratt

jhpratt commented Aug 26, 2020

Copy link
Copy Markdown
Member

Published.

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.

4 participants