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

lang/rust: Migrate to avro-rs #3199

Merged
merged 3 commits into from
Oct 8, 2024
Merged

lang/rust: Migrate to avro-rs #3199

merged 3 commits into from
Oct 8, 2024

Conversation

Xuanwo
Copy link
Member

@Xuanwo Xuanwo commented Oct 6, 2024

What is the purpose of the change

This PR migrates the rust codebase to avro-rs

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

@Xuanwo Xuanwo marked this pull request as ready for review October 6, 2024 09:07
<exclude>lang/rust/Cargo.lock</exclude>
<exclude>lang/rust/README.tpl</exclude>
<exclude>lang/rust/.requirements-precommit.txt</exclude>
<exclude>lang/rust/README.md</exclude>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Xuanwo What do you use instead of RAT for license checking in your projects ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I'm using https://github.com/korandoru/hawkeye which written in rust and provides github actions natively:

licenses:
  runs-on: ubuntu-latest
  timeout-minutes: 10
  env:
    FORCE_COLOR: 1
  steps:
    - uses: actions/checkout@v4
    - name: Check license headers
      uses: korandoru/hawkeye@v5

## WebAssembly demo application

See [wasm-demo/README.md](./wasm-demo/README.md)
We apologize for the inconvenience and greatly appreciate your contributions!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious, why only rust version moved to its own repo?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an experiment, if it ends up being better for sustained development, the other languages will likely be split out as well.

@martin-g
Copy link
Member

martin-g commented Oct 7, 2024

pypy3.9: install_deps> python -I -m pip install coverage python-snappy zstandard
  error: subprocess-exited-with-error
  
  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [6 lines of output]
      Checking for Rust toolchain....
      
      Cargo, the Rust package manager, is not installed or is not on PATH.
      This package requires Rust and Cargo to compile extensions. Install it through
      the system's package manager or via https://rustup.rs/
      
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

It seems the Python tests depend on the Rust toolchain.

@KalleOlaviNiemitalo
Copy link
Contributor

Linking to the dev mailing list thread where the rationale was explained:
[VOTE] Extract the Rust SDK into its own Git repository

@Xuanwo
Copy link
Member Author

Xuanwo commented Oct 8, 2024

Linking to the dev mailing list thread where the rationale was explained: [VOTE] Extract the Rust SDK into its own Git repository

Thank you for the suggestion; it's been added to the Rust README now.

@martin-g martin-g merged commit 2ac5a46 into apache:main Oct 8, 2024
7 checks passed
@martin-g
Copy link
Member

martin-g commented Oct 8, 2024

Thank you, @Xuanwo !

@Xuanwo Xuanwo deleted the migrate-rust branch October 8, 2024 06:58
@Xuanwo
Copy link
Member Author

Xuanwo commented Oct 8, 2024

Some python tools are written in rust:

        --- stderr
        configure.ac:22: installing 'build-aux/compile'
        configure.ac:24: installing 'build-aux/config.guess'
        configure.ac:24: installing 'build-aux/config.sub'
        configure.ac:11: installing 'build-aux/install-sh'
        configure.ac:11: installing 'build-aux/missing'
        Makefile.am: installing 'build-aux/depcomp'
        parallel-tests: installing 'build-aux/test-driver'
        configure: error: No modern nasm or yasm found as required. Nasm should be v2.11.01 or later (v2.13 for AVX512) and yasm should be 1.2.0 or later.
        thread 'main' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/isal-sys-0.3.1+496255c/build.rs:72:17:
        configure failed
        note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
      warning: build failed, waiting for other jobs to finish...
      💥 maturin failed
        Caused by: Failed to build a native library through cargo
        Caused by: Cargo build finished with "exit status: 101": `env -u CARGO PYO3_ENVIRONMENT_SIGNATURE="pypy-3.9-64bit" PYO3_PYTHON="/home/runner/work/avro/avro/lang/py/.tox/py/bin/python" PYTHON_SYS_EXECUTABLE="/home/runner/work/avro/avro/lang/py/.tox/py/bin/python" "cargo" "rustc" "--message-format" "json-render-diagnostics" "--manifest-path" "/tmp/pip-install-qqb1t7fw/cramjam_87d55c093e714efabcee81485a095f12/Cargo.toml" "--release" "--lib" "--" "-C" "strip=symbols"`
      Error: command ['maturin', 'pep517', 'build-wheel', '-i', '/home/runner/work/avro/avro/lang/py/.tox/py/bin/python', '--compatibility', 'off'] returned non-zero exit status 1
      [end of output]

I'm working on this now.

@Xuanwo
Copy link
Member Author

Xuanwo commented Oct 8, 2024

Some python tools are written in rust:

        --- stderr
        configure.ac:22: installing 'build-aux/compile'
        configure.ac:24: installing 'build-aux/config.guess'
        configure.ac:24: installing 'build-aux/config.sub'
        configure.ac:11: installing 'build-aux/install-sh'
        configure.ac:11: installing 'build-aux/missing'
        Makefile.am: installing 'build-aux/depcomp'
        parallel-tests: installing 'build-aux/test-driver'
        configure: error: No modern nasm or yasm found as required. Nasm should be v2.11.01 or later (v2.13 for AVX512) and yasm should be 1.2.0 or later.
        thread 'main' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/isal-sys-0.3.1+496255c/build.rs:72:17:
        configure failed
        note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
      warning: build failed, waiting for other jobs to finish...
      💥 maturin failed
        Caused by: Failed to build a native library through cargo
        Caused by: Cargo build finished with "exit status: 101": `env -u CARGO PYO3_ENVIRONMENT_SIGNATURE="pypy-3.9-64bit" PYO3_PYTHON="/home/runner/work/avro/avro/lang/py/.tox/py/bin/python" PYTHON_SYS_EXECUTABLE="/home/runner/work/avro/avro/lang/py/.tox/py/bin/python" "cargo" "rustc" "--message-format" "json-render-diagnostics" "--manifest-path" "/tmp/pip-install-qqb1t7fw/cramjam_87d55c093e714efabcee81485a095f12/Cargo.toml" "--release" "--lib" "--" "-C" "strip=symbols"`
      Error: command ['maturin', 'pep517', 'build-wheel', '-i', '/home/runner/work/avro/avro/lang/py/.tox/py/bin/python', '--compatibility', 'off'] returned non-zero exit status 1
      [end of output]

I'm working on this now.

Oh, they have been failing for a long time, even before this PR.

@Xuanwo
Copy link
Member Author

Xuanwo commented Oct 8, 2024

I see that cramjam depends on python-snappy, which is written in Rust. cramjam doesn't have a pre-built binary for pypy 3.9: https://pypi.org/project/cramjam/#files, so it needs to be built from source. However, our environment lacks nasm or yasm, causing the build of cramjam to fail.

https://github.com/intake/python-snappy/blob/60754617af99ff97a9b2b6fcbc2901d611c3f658/setup.py#L42-L43


Tracked at milesgranger/cramjam#185

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants