You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/12/07 21:15:34 UTC
[GitHub] [arrow] jorgecarleitao opened a new pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
jorgecarleitao opened a new pull request #8864:
URL: https://github.com/apache/arrow/pull/8864
This is a requirement fielded to merge #7767, and so I though about adding it to our CI.
I am not sure whether this is exactly what we need to test #7767, and so I would like to request help from @paddyhoran and @rj-atw here.
NOTE: this is built on top of #8821 . Only the last commit is relevant. Basically, this runs
```
rustup add target wasm32-unknown-unknown
cd rust/arrow
cargo build --target wasm32-unknown-unknown
```
Note that this does not actually run the tests yet. I think that that is #7767 's goal.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on a change in pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#discussion_r537896948
##########
File path: .github/workflows/rust.yml
##########
@@ -350,3 +350,45 @@ jobs:
pip install maturin==0.8.2 toml==0.10.1 pyarrow==1.0.0
maturin develop
python -m unittest discover tests
+
+ # test the arrow crate builds against wasm32 in stable rust
+ wasm32-build:
+ name: AMD64 Debian 10 Rust ${{ matrix.rust }} test arrow wasm32
+ runs-on: ubuntu-latest
+ strategy:
+ matrix:
+ arch: [amd64]
+ rust: [nightly-2020-11-24]
+ container:
+ image: ${{ matrix.arch }}/rust
+ env:
+ ARROW_TEST_DATA: /__w/arrow/arrow/testing/data
+ PARQUET_TEST_DATA: /__w/arrow/arrow/cpp/submodules/parquet-testing/data
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Cache Cargo
+ uses: actions/cache@v2
+ with:
+ path: /github/home/.cargo
+ # this key equals the ones on `linux-build-lib` for re-use
+ key: cargo-cache-
+ - name: Cache Rust dependencies
+ uses: actions/cache@v2
+ with:
+ path: /github/home/target
+ key: ${{ runner.os }}-${{ matrix.arch }}-target-wasm32-cache-${{ matrix.rust }}
+ - uses: actions-rs/toolchain@v1
+ with:
+ toolchain: ${{ matrix.rust }}
+ default: true
+ override: true
+ components: rustfmt
+ target: wasm32-unknown-unknown
+ - name: Build arrow crate
Review comment:
I wonder if we could also run some tests? Or maybe that is a follow on step?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#issuecomment-747090607
@nevi-me -- thank you. You are right -- I was not building with `-p arrow` (so presumably the build errors were from some other crate).
In that case, I agree that this would be a good PR to merge to make sure people don't accidentally introduce new code in arrow that breaks the wasm build .
What do you think @jorgecarleitao ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nevi-me commented on a change in pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
nevi-me commented on a change in pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#discussion_r543343517
##########
File path: .github/workflows/rust.yml
##########
@@ -350,3 +350,45 @@ jobs:
pip install maturin==0.8.2 toml==0.10.1 pyarrow==1.0.0
maturin develop
python -m unittest discover tests
+
+ # test the arrow crate builds against wasm32 in stable rust
+ wasm32-build:
+ name: AMD64 Debian 10 Rust ${{ matrix.rust }} test arrow wasm32
+ runs-on: ubuntu-latest
+ strategy:
+ matrix:
+ arch: [amd64]
+ rust: [nightly-2020-11-24]
+ container:
+ image: ${{ matrix.arch }}/rust
+ env:
+ ARROW_TEST_DATA: /__w/arrow/arrow/testing/data
+ PARQUET_TEST_DATA: /__w/arrow/arrow/cpp/submodules/parquet-testing/data
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Cache Cargo
+ uses: actions/cache@v2
+ with:
+ path: /github/home/.cargo
+ # this key equals the ones on `linux-build-lib` for re-use
+ key: cargo-cache-
+ - name: Cache Rust dependencies
+ uses: actions/cache@v2
+ with:
+ path: /github/home/target
+ key: ${{ runner.os }}-${{ matrix.arch }}-target-wasm32-cache-${{ matrix.rust }}
+ - uses: actions-rs/toolchain@v1
+ with:
+ toolchain: ${{ matrix.rust }}
+ default: true
+ override: true
+ components: rustfmt
+ target: wasm32-unknown-unknown
+ - name: Build arrow crate
Review comment:
Follow-up, as some tests currently fail
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb closed pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
alamb closed pull request #8864:
URL: https://github.com/apache/arrow/pull/8864
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#issuecomment-746329723
FWIW, I do not think we should merge this if there are no plans to merge #7767.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#issuecomment-747058180
I don't fully understand what this check is doing
I see it did something which claims to have run `cargo build --target wasm32-unknown-unknown`:
<img width="929" alt="Screen Shot 2020-12-16 at 4 34 47 PM" src="https://user-images.githubusercontent.com/490673/102409205-a6d04280-3fbc-11eb-928c-1c2213e8107e.png">
But it takes a suspiciously short amount of time (8s) and when I do (seemingly) the same thing locally I get a bunch of compilation errors
```
Compiling encode_unicode v0.3.6
error[E0433]: failed to resolve: could not find `unix` in `os`
--> /Users/alamb/.cargo/registry/src/github.com-1ecc6299db9ec823/dirs-1.0.5/src/lin.rs:41:18
|
41 | use std::os::unix::ffi::OsStringExt;
| ^^^^ could not find `unix` in `os`
error[E0432]: unresolved import `unix`
--> /Users/alamb/.cargo/registry/src/github.com-1ecc6299db9ec823/dirs-1.0.5/src/lin.rs:6:5
|
6 | use unix;
| ^^^^ no `unix` in the root
Compiling hex v0.4.2
Compiling tower-service v0.3.0
Compiling alloc-no-stdlib v2.0.1
Compiling adler32 v1.0.4
error[E0599]: no function or associated item named `from_vec` found for struct `OsString` in the current scope
--> /Users/alamb/.cargo/registry/src/github.com-1ecc6299db9ec823/dirs-1.0.5/src/lin.rs:48:34
|
48 | Some(PathBuf::from(OsString::from_vec(out)))
| ^^^^^^^^ function or associated item not found in `OsString`
|
= help: items from traits can only be used if the trait is in scope
= note: the following trait is implemented but not in scope; perhaps add a `use` for it:
`use std::sys_common::os_str_bytes::OsStringExt;`
error: aborting due to 3 previous errors
Some errors have detailed explanations: E0432, E0433, E0599.
For more information about an error, try `rustc --explain E0432`.
error: could not compile `dirs`
To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed
```
I probably don't have something installed right
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nevi-me commented on pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#issuecomment-747065423
Are you building from within the `arrow` directory, or using `-p arrow`?
The purpose of adding just the `cargo build` (or even `cargo check`) to CI is to prevent making changes (normally feature-gates in SIMD) that break `wasm32-unknown-unknown` targets.
We have partial wasm32 support in `arrow`, in the sense that the crate can be built, but not all tests pass, and it's likely that `std::fs`-related code + some other stuff don't work.
So we'd want to at least test that we don't break support, see ARROW-9088.
I've just tested compiling with the wasm32 target in both Windows and Linux via WSL, and the `arrow` crate builds. I'm assuming that the 8 seconds taken is due to the build artefacts being cached from some previous build, but @jorgecarleitao would be authoritative in answering that.
If you currently try to run tests without the magic sauce that's in #7767, you get:
```rust
$ cargo test --target wasm32-unknown-unknown -p arrow
arrow/rust/target/wasm32-unknown-unknown/debug/deps/arrow-2495d35fed1d3bb9.wasm: 1: Syntax error: end of file unexpected
error: test failed, to rerun pass '--lib'
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#issuecomment-747209052
I agree with @nevi-me , that this is useful to avoid regressions in being able to compile wasm32. Let's merge this then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#issuecomment-740194219
https://issues.apache.org/jira/browse/ARROW-10838
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nevi-me commented on pull request #8864: ARROW-10838: [Rust] [CI] Add arrow build targeting wasm32
Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #8864:
URL: https://github.com/apache/arrow/pull/8864#issuecomment-746332585
I think we should merge this given that we don't know when the PR with tests will be ready.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org