Improve GLIBC compatibility and fix build issue

rob-odwyer commented 1 year ago

This is a follow-up to #49 that fixes an issue with those changes, plus improves compatibility further using a few tricks borrowed from the core bytecodealliance/wasmtime build.

The publish step was replacing the compatible shared lib before building a universal JAR - this avoids rebuilding it by skipping those gradle tasks manually
To maximize glibc compatibility, we run the gradle build for linux in a centos:7 docker container, similar to https://github.com/bytecodealliance/wasmtime/tree/main/.github/actions/binary-compatible-builds
The macOS and Windows build tweaks were copied from the wasmtime build as well - I don't know much about the consequences of these options though so if they're not 100% safe I'm happy to drop them.

I've tested this on my own fork, and it appears to work for all platforms and not slow down the build too significantly. The bugfix in the publish step also speeds up the build by avoiding rebuilding of the shared lib.

The result of this change is that glibc >= 2.14 is supported (was 2.34 previously).

chris-hoefgen commented 1 year ago

@kawamuray this will dramatically improve the compatibility of wasmtime-java on more systems

kawamuray commented 1 year ago

@rob-odwyer very weird, but after merging this PR the master branch build has failed by the error for building rust lib.

https://github.com/kawamuray/wasmtime-java/actions/runs/5740063436/job/15557049239

error[E0433]: failed to resolve: could not find `FsConfigCmd` in `types`
   --> /root/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.38.5/src/backend/linux_raw/mount/syscalls.rs:214:27
    |
214 |             super::types::FsConfigCmd::Create,
    |                           ^^^^^^^^^^^ could not find `FsConfigCmd` in `types`

error[E0433]: failed to resolve: could not find `FsConfigCmd` in `types`
   --> /root/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.38.5/src/backend/linux_raw/mount/syscalls.rs:228:27
    |
228 |             super::types::FsConfigCmd::Reconfigure,
    |                           ^^^^^^^^^^^ could not find `FsConfigCmd` in `types`

error[E0277]: the trait bound `reg::ArgReg<'_, A3>: From<MountFlagsArg>` is not satisfied
   --> /root/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.38.5/src/backend/linux_raw/mount/syscalls.rs:28:13
    |
28  |             flags,
    |             ^^^^^ the trait `From<MountFlagsArg>` is not implemented for `reg::ArgReg<'_, A3>`
    |
   ::: /root/.cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.38.5/src/backend/linux_raw/arch/mod.rs:258:17
    |
258 |             $a3.into(),
    |                 ---- required by a bound introduced by this call
    |
    = help: the following other types implement trait `From<T>`:
              <reg::ArgReg<'a, Num> as From<&'a CStr>>
              <reg::ArgReg<'a, Num> as From<&'a mut [std::mem::MaybeUninit<T>]>>
              <reg::ArgReg<'a, Num> as From<&'a mut std::mem::MaybeUninit<T>>>
              <reg::ArgReg<'a, Num> as From<(backend::fs::types::Mode, backend::fs::types::FileType)>>
              <reg::ArgReg<'a, Num> as From<(net::types::SocketType, net::types::SocketFlags)>>
              <reg::ArgReg<'a, Num> as From<*const T>>
              <reg::ArgReg<'a, Num> as From<*mut T>>
              <reg::ArgReg<'a, Num> as From<Advice>>
            and 29 others
    = note: required for `MountFlagsArg` to implement `Into<reg::ArgReg<'_, A3>>

Still have no idea why it doesn't fail for this PR build...

rob-odwyer commented 1 year ago

Thanks for merging!

That is very weird - I'll look into this today and see if I can figure it out. I didn't run into this at all on my fork when doing my own v0.16.0 build on the main branch, so I'm guessing it's either a temporary blip or some dependency has changed.

rob-odwyer commented 1 year ago

I've been testing this on my own branch, triggering releases against the same commits and can't seem to reproduce the issue. Successful build against 2807a48: https://github.com/DevCycleHQ/wasmtime-java/actions/runs/5741642619/job/15562167700

However, it looks to me like the build triggered by the revert commit actually succeeded, and that does not revert the changes in this PR, just the version bump: https://github.com/kawamuray/wasmtime-java/actions/runs/5740201090

Based on that, I think these failures might have been some kind of temporary Github Actions error? It's concerning that it happened on both the master build and on the tag build, but I'm optimistic that if you make another v0.16.0 tag it'll work this time.

rob-odwyer commented 1 year ago

I think I figured out what happened here by looking more closely at the build logs and compile errors. The error occurs when trying to compile rustix-0.38.5. It looks like this was a broken patch release that was fixed shortly afterwards in rustix-0.38.6, which my builds have been pulling in. https://github.com/bytecodealliance/rustix/commit/f2d0ea2f0ddc5441f08d5ec27005e557648c4b03

rob-odwyer commented 1 year ago

I'm not very familiar with cargo, but shouldn't the entries in Cargo.lock have prevented this? Does it need to be updated maybe?

kawamuray commented 1 year ago

Thanks for the quick investigation Rob. I think that makes sense. 0.16.0 successfully built :) https://github.com/kawamuray/wasmtime-java/actions/runs/5745093520

shouldn't the entries in Cargo.lock have prevented this?

Good point. wasmtime-jni can be considered as an "end product" so I think it makes sense to check Cargo.lock in to git. https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html I've added Cargo.lock into git.

kawamuray / wasmtime-java

Improve GLIBC compatibility and fix build issue #50