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

Update Rustler version 0.21 -> 0.22 #8

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

ndac-todoroki
Copy link
Owner

fixes #7.

This should make OTP24 to work.
Rustler 0.22 has many changes, and migrating from deprecated macros may be necessary.

@ndac-todoroki
Copy link
Owner Author

Current progress:

I'm quite stuck and confused with the lifetime of Rust code.

~/Re/gi/nd/elixir-id3 on  update-deps [!?] is 📦 v1.0.2 via 💧 1.12.2 (OTP 24) on ☁️  ap-northeast-1 
23:48:11 ❯❯ iex -S mix
Erlang/OTP 24 [erts-12.0.3] [source] [64-bit] [smp:24:24] [ds:24:24:10] [async-threads:1] [jit]

Compiling 4 files (.ex)
Compiling crate id3 in debug mode (native/id3)
   Compiling proc-macro2 v1.0.29
   Compiling unicode-xid v0.2.2
   Compiling autocfg v1.0.1
   Compiling crc32fast v1.2.1
   Compiling syn v1.0.75
   Compiling libc v0.2.101
   Compiling rustler_sys v2.1.1
   Compiling void v1.0.2
   Compiling cfg-if v1.0.0
   Compiling unicode-segmentation v1.8.0
   Compiling adler v1.0.2
   Compiling rustler v0.22.0
   Compiling bitflags v1.3.2
   Compiling lazy_static v1.4.0
   Compiling byteorder v1.4.3
   Compiling unreachable v1.0.0
   Compiling miniz_oxide v0.4.4
   Compiling heck v0.3.3
   Compiling quote v1.0.9
   Compiling flate2 v1.0.20
   Compiling id3 v0.5.3
   Compiling rustler_codegen v0.22.0
   Compiling id3 v1.0.2 (/home/todoroki/Repositories/github.com/ndac-todoroki/elixir-id3/native/id3)
error[E0496]: lifetime name `'a` shadows a lifetime name that is already in scope
  --> src/lib.rs:60:15
   |
59 | impl<'a> Encoder for ReadResult<'a> {
   |      -- first declared here
60 |     fn encode<'a>(&'a self, env: Env<'a>) -> Term<'a> {
   |               ^^ lifetime `'a` already in scope

error[E0261]: use of undeclared lifetime name `'a`
  --> src/lib.rs:23:10
   |
23 | #[derive(NifStruct)]
   |          -^^^^^^^^
   |          |
   |          undeclared lifetime
   |          lifetime `'a` is missing in item created through this procedural macro
   |
   = note: this error originates in the derive macro `NifStruct` (in Nightly builds, run with -Z macro-backtrace for more info)

error: `impl` item signature doesn't match `trait` item signature
  --> src/lib.rs:60:5
   |
60 |     fn encode<'a>(&'a self, env: Env<'a>) -> Term<'a> {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&ReadResult<'a>, Env<'_>) -> rustler::Term<'_>`
   | 
  ::: /home/todoroki/.cargo/registry/src/github.com-1ecc6299db9ec823/rustler-0.22.0/src/types/mod.rs:41:5
   |
41 |     fn encode<'a>(&self, env: Env<'a>) -> Term<'a>;
   |     ----------------------------------------------- expected `fn(&ReadResult<'a>, Env<'a>) -> rustler::Term<'a>`
   |
   = note: expected `fn(&ReadResult<'a>, Env<'a>) -> rustler::Term<'a>`
              found `fn(&ReadResult<'a>, Env<'_>) -> rustler::Term<'_>`
   = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait`
   = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output

error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
  --> src/lib.rs:61:15
   |
61 |         match self {
   |               ^^^^
   |
note: first, the lifetime cannot outlive the lifetime `'a` as defined on the impl at 59:6...
  --> src/lib.rs:59:6
   |
59 | impl<'a> Encoder for ReadResult<'a> {
   |      ^^
note: ...so that the types are compatible
  --> src/lib.rs:61:15
   |
61 |         match self {
   |               ^^^^
   = note: expected `&ReadResult<'_>`
              found `&'a ReadResult<'a>`
   = note: but, the lifetime must be valid for the static lifetime...
note: ...so that the types are compatible
  --> src/lib.rs:81:15
   |
81 |             ).encode(env),
   |               ^^^^^^
   = note: expected `rustler::Encoder`
              found `rustler::Encoder`

error[E0277]: the trait bound `&[rustler::Term<'a>]: Decoder<'_>` is not satisfied
   --> src/lib.rs:114:1
    |
114 | #[rustler::nif]
    | ^^^^^^^^^^^^^^^ the trait `Decoder<'_>` is not implemented for `&[rustler::Term<'a>]`
    |
    = note: this error originates in the attribute macro `rustler::nif` (in Nightly builds, run with -Z macro-backtrace for more info)

Some errors have detailed explanations: E0261, E0277, E0495, E0496.
For more information about an error, try `rustc --explain E0261`.
error: could not compile `id3` due to 5 previous errors

== Compilation error in file lib/id3/native.ex ==
** (RuntimeError) Rust NIF compile error (rustc exit code 101)
    (rustler 0.22.0) lib/rustler/compiler.ex:36: Rustler.Compiler.compile_crate/2
    lib/id3/native.ex:8: (module)
    (stdlib 3.15.2) erl_eval.erl:685: :erl_eval.do_apply/6

@cmush
Copy link

cmush commented Sep 26, 2021

Greetings @ndac-todoroki!

I am new to rust so forgive me if my proposed solution is off the mark.

Regarding the issue

error[E0496]: lifetime name `'a` shadows a lifetime name that is already in scope --> src/lib.rs:60:15

See PR #365 of rustler

According to the changelog detailing the v0.22 rustler updates:

"NIFs can be derived from regular functions, if the arguments implement Decoder and the return type implements Encoder". It includes the following example:

//
// Before
//
fn add<'a>(env: Env<'a>, args: &[Term<'a>]) -> Result<Term<'a>, Error> {
    let num1: i64 = args[0].decode()?;
    let num2: i64 = args[1].decode()?;

    Ok((atoms::ok(), num1 + num2).encode(env))
}

//
// After
//
#[rustler::nif]
fn add(a: i64, b: i64) -> i64 {
  a + b
}

By continuing to manually declare the lifetimes (v0.21.1's format), it appears to introduce some form of duplication that causes the compiler to freak out.

@ndac-todoroki
Copy link
Owner Author

ndac-todoroki commented Sep 27, 2021

Thanks for helping @cmush !

The lifetime problem seems a bit more complex to me, because

  1. The add function in the example never fails (it's always :ok)
  2. The function in the example don't return strings within structs (which requires declared lifetimes?)

fn major_frames returns a ReadResult enum, which is either an Ok<MajorFrames> or an Error, which corresponds to {:ok, _} | :error.
MajorFrames is a struct with some Option<&str> fields (e.g. title, genre).
&str inside a struct requires a declared lifetime, because it may not exclude the lifetime of the struct itself,
then the MajorFrames struct it self needs to have its lifetime declared,
then the ReadResult enum needs a lifetime declared too,
thus the major_frames function also needs a declared lifetime.

The returned ReadResult<'a> needs to be encoded via the Encoder trait to a rustler::Term, which corresponds to an Erlang term.
But, the function signature fn encode<'a>(&'a self, env) -> Term<'a> does not exist in the rustler::Term trait, so it cannot be implemented...


I think something must be wrong with my implementation flow, but currently not available to solve it.

BTW,

error[E0496]: lifetime name 'a shadows a lifetime name that is already in scope --> src/lib.rs:60:15

this error is because I've placed the same lifetime parameter a here:

impl<'a> Encoder for ReadResult<'a> {
fn encode<'a>(&'a self, env: Env<'a>) -> Term<'a> {

the function and the impl both has the same parameter and the outer one is shadowed. This should be changed to another name.

@ndac-todoroki
Copy link
Owner Author

I've changed from borrowing strs inside structs to owning Strings inside structs, and the lifetime problem seems to be avoided.
I think it would be better if I copy it just before the encode happens, but I'm not sure. It'll be a WIP anyways.

The write function are now changed too, in the same way as the read function.
Since it is more simple where it only returns tuples of atoms, the implementations are far simple than read.

These functions are not yet tested much yet.... I should write some tests :(

@cmush
Copy link

cmush commented Sep 29, 2021

Thanks for helping @cmush !

The lifetime problem seems a bit more complex to me, because

  1. The add function in the example never fails (it's always :ok)
  2. The function in the example don't return strings within structs (which requires declared lifetimes?)

fn major_frames returns a ReadResult enum, which is either an Ok<MajorFrames> or an Error, which corresponds to {:ok, _} | :error. MajorFrames is a struct with some Option<&str> fields (e.g. title, genre). &str inside a struct requires a declared lifetime, because it may not exclude the lifetime of the struct itself, then the MajorFrames struct it self needs to have its lifetime declared, then the ReadResult enum needs a lifetime declared too, thus the major_frames function also needs a declared lifetime.

The returned ReadResult<'a> needs to be encoded via the Encoder trait to a rustler::Term, which corresponds to an Erlang term. But, the function signature fn encode<'a>(&'a self, env) -> Term<'a> does not exist in the rustler::Term trait, so it cannot be implemented...

I think something must be wrong with my implementation flow, but currently not available to solve it.

BTW,

error[E0496]: lifetime name 'a shadows a lifetime name that is already in scope --> src/lib.rs:60:15

this error is because I've placed the same lifetime parameter a here:

impl<'a> Encoder for ReadResult<'a> {
fn encode<'a>(&'a self, env: Env<'a>) -> Term<'a> {

the function and the impl both has the same parameter and the outer one is shadowed. This should be changed to another name.

This got me thinking I should pick up a rust book and some tuts. So that's what I've been up to :)

@cmush
Copy link

cmush commented Sep 29, 2021

I've changed from borrowing strs inside structs to owning Strings inside structs, and the lifetime problem seems to be avoided. I think it would be better if I copy it just before the encode happens, but I'm not sure. It'll be a WIP anyways.

The write function are now changed too, in the same way as the read function. Since it is more simple where it only returns tuples of atoms, the implementations are far simple than read.

These functions are not yet tested much yet.... I should write some tests :(

@ndac-todoroki I can confirm that with the latest commits you've made, the library is able to successfully compile the crate as well as open an iex -S mix session; run ID3.get_tag!("path_to.mp3") and get the expected response :)

Sidenote: Earlier, I had issues compiling on an m1 device but the following setup fixed it:

  1. create native/id3/.cargo/config in your project root
  2. paste the following content:
[target.aarch64-apple-darwin]
rustflags = [
    "-C", "link-arg=-undefined",
    "-C", "link-arg=dynamic_lookup",
]

asdf config (.tool-versions):

erlang 24.1
elixir 1.12.3-otp-24%

@cmush
Copy link

cmush commented Sep 29, 2021

These functions are not yet tested much yet.... I should write some tests :(

I started on something: #9
Let me know what you think.

@ndac-todoroki ndac-todoroki marked this pull request as ready for review October 5, 2021 06:44
@ndac-todoroki
Copy link
Owner Author

@cmush So much thanks for #9, the tests would be merged first, then this (or maybe after setting up an CI)!

@w0rd-driven
Copy link

If other people get here, I was able to use this library in Livebook by installing the dependency via:

Mix.install([
  {:id3, git: "https://github.com/ndac-todoroki/elixir-id3.git", branch: "update-deps"}
])

I am in absolutely no hurry so I didn't want to add any urgency as I understand having tests and CI would help move this forward in the future.

I also wanted to thank you because out of trying most of the public packages, this one just works(tm) when I used this branch.

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.

Compilation fails with Erlang 24
3 participants