Skip to content
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

Upgrading from 0.14.0 to 0.14.1 breaks compatibility with wasm32-unknown-unknown target #118

Closed
bnjbvr opened this issue Jan 5, 2023 · 2 comments · Fixed by #123
Closed
Assignees
Labels
bug Something isn't working

Comments

@bnjbvr
Copy link
Contributor

bnjbvr commented Jan 5, 2023

puffin 0.14.1 introduces a new required wasm import env::now, that's apparently imported by instant. I suspect this is coming from the js-sys ecosystem, or it might even be a WASI standard. There should be a way to opt out of this, so one can use puffin in wasm32-unknown-unknown environments too.

It seems that 0.14.0 didn't depend at all on instant, so making the dependency on instant optional + reuse whatever was used before to retrieve the time, should be sufficient to fix this.

@bnjbvr bnjbvr added the bug Something isn't working label Jan 5, 2023
@bnjbvr
Copy link
Contributor Author

bnjbvr commented Jan 5, 2023

Also I realize that it might be nice to have tests for that compile target, that just uses simple scopes etc.

@bnjbvr bnjbvr self-assigned this Jan 30, 2023
TimonPost pushed a commit that referenced this issue Jan 30, 2023
A wasm embedder that hasn't enabled the `web` feature will still get the
`Instant::now()` function generated, even if they end up using a custom
`now` function. After all, the compiler/link don't have runtime
knowledge about how the program runs, so it could be that
`ThreadProfiler::initialize` is never called, so the default value of
`now_ns` that makes use of `Instant::now` could be called at any point.
 
This changes the `now_ns` function so it's defined only using `Instant`
for native target and wasm when web is enabled. In the other case, we
assume this is never called and it's a programmer error because we don't
have any time reporter.

Fixes #118.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants