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

Add support for non-windows/unix targets #10

Merged
merged 5 commits into from
Jan 18, 2018

Conversation

dflemstr
Copy link
Contributor

@dflemstr dflemstr commented Dec 16, 2017

This is mostly for wasm support and I'm not sure if this approach is great for other targets.

I could also add a continuous integration job for this target if necessary.

@carllerche
Copy link
Owner

Thanks.

Could you provide more context about how this would be used on wasm? Also, a CI branch would be appreciated.

I also think this is based on the 0.1.x branch. I switched master to represent 0.2.x. This can be handled in a separate PR though.

@dflemstr
Copy link
Contributor Author

@carllerche There is no iovec concept in wasm AFAIK since that is a UNIX (POSIX?) specific concept. Since the heap isn't segmented I don't see an advantage in tracking iovec memory at all on that architecture, since I can't think of anything reasonable a readv(2) implementation could use it for.

This PR is of course a bit generic since it assumes no iovec support by default on unknown platforms. Maybe it's better to cause a compilation error and whitelist specific architectures?

@dflemstr dflemstr force-pushed the wasm-support branch 2 times, most recently from 6006e03 to a1634ba Compare December 18, 2017 15:54
@dflemstr
Copy link
Contributor Author

Not sure why the build is failing; it builds locally...

@dflemstr dflemstr closed this Jan 8, 2018
@dflemstr dflemstr reopened this Jan 8, 2018
@carllerche
Copy link
Owner

Is there a way to run wasm targets in CI?

I guess at the very least, CI compiles but doesn't run?

@dflemstr
Copy link
Contributor Author

@carllerche it's possible to run them using a significant amount of node.js scaffolding. What would you like to do? Create a program that exercises this code path?

@koute
Copy link

koute commented Jan 15, 2018

Is there a way to run wasm targets in CI?

At least on Travis you can run all of the Rust's web targets pretty easily with cargo-web if that's what you want; for example:

# Install nodejs
nvm install 9

# Unfortunately we don't have binary packages for cargo-web yet
cargo install cargo-web

rustup target add asmjs-unknown-emscripten
cargo web test --nodejs --target-asmjs-emscripten

rustup target add wasm32-unknown-emscripten
cargo web test --nodejs --target-webasm-emscripten

set +e
echo "$(rustc --version)" | grep -q "nightly"
if [ "$?" = "0" ]; then
    export IS_NIGHTLY=1
else
    export IS_NIGHTLY=0
fi
set -e

if [ "$IS_NIGHTLY" = "1" ]; then
    rustup target add wasm32-unknown-unknown
    cargo web test --nodejs --target-webasm
fi

That will run the tests for all three web targets using nodejs. Running the tests in a headless chromium is also supported for the *-emscripten targets (if you don't pass the --nodejs flag), but you wouldn't really need that.

Just please note that right now a lot of stuff on the wasm32-unknown-unknown target (like, for example, println! calls) do nothing, so if the tests on that target fail you won't know what went wrong. Once rust-lang/rust#47102 lands I'll add support for it in cargo-web so that println! and such will work.

@carllerche
Copy link
Owner

Interesting. I'm less interested in getting full test parity, mostly ensure things "work" at some level.

@carllerche
Copy link
Owner

Ok, I think this is good now. Thanks!

@carllerche carllerche merged commit e777234 into carllerche:master Jan 18, 2018
ctjhoa pushed a commit to ctjhoa/iovec that referenced this pull request Jan 19, 2018
ctjhoa pushed a commit to ctjhoa/iovec that referenced this pull request Jan 25, 2018
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.

3 participants