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

build(native): switch to wasmcloud-otp #644

Merged
merged 3 commits into from
Jul 21, 2023

Conversation

rvolosatovs
Copy link
Member

@rvolosatovs rvolosatovs commented Jul 21, 2023

Feature or Problem

Related Issues

closes #640

Release Information

Consumer Impact

Testing

Built on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • x86_64-windows

Tested on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • x86_64-windows

Unit Test(s)

Acceptance or Integration

Manual Verification

@rvolosatovs rvolosatovs force-pushed the update/deps branch 2 times, most recently from 13d016d to ee0fac0 Compare July 21, 2023 12:43
@rvolosatovs rvolosatovs marked this pull request as ready for review July 21, 2023 12:43
@rvolosatovs rvolosatovs enabled auto-merge (rebase) July 21, 2023 12:44
autodidaddict
autodidaddict previously approved these changes Jul 21, 2023
Copy link
Member

@autodidaddict autodidaddict left a comment

Choose a reason for hiding this comment

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

I'd made similar changes to my own PR, but I didn't think about the nix flake. Good catch

@autodidaddict
Copy link
Member

LGTM so long as the build passes

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@rvolosatovs
Copy link
Member Author

I'd made similar changes to my own PR, but I didn't think about the nix flake. Good catch

The CI in this repo was broken for a while apparently, as I mentioned earlier (at least for now) updates to mix dependencies require updating the hash in the flake

@autodidaddict
Copy link
Member

Can this get fixed?

thread 'main' panicked at 'The NIF version given from RUSTLER_NIF_VERSION is not supported: 2.17', /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustler_sys-2.2.0/build.rs:925:

rusterlium/rustler#544

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@rvolosatovs
Copy link
Member Author

Can this get fixed?

thread 'main' panicked at 'The NIF version given from RUSTLER_NIF_VERSION is not supported: 2.17', /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustler_sys-2.2.0/build.rs:925:

See rusterlium/rustler#544

The issue is that OTP version used by CI is non-deterministic. Note, how it works just fine within Nix sandbox (because OTP version is locked)

@connorsmith256
Copy link
Contributor

I don't follow. @rvolosatovs are you saying you expect CI to fail with this change? Do we need to/can we upgrade rustler to fix it?

@rvolosatovs
Copy link
Member Author

I don't follow. @rvolosatovs are you saying you expect CI to fail with this change? Do we need to/can we upgrade rustler to fix it?

No. CI running outside Nix sandbox failed before a8df2c5, see https://github.com/wasmCloud/wasmcloud-otp/actions/runs/5622887410/job/15236382057 for example.

@autodidaddict
Copy link
Member

So I am still a bit fuzzy... @rvolosatovs is this something that you're comfortable merging, and saying that the 2 failing items (so far) are things we can safely ignore?

@rvolosatovs
Copy link
Member Author

So I am still a bit fuzzy... @rvolosatovs is this something that you're comfortable merging, and saying that the 2 failing items (so far) are things we can safely ignore?

I do not see any failures, CI is still running

@autodidaddict
Copy link
Member

So I am still a bit fuzzy... @rvolosatovs is this something that you're comfortable merging, and saying that the 2 failing items (so far) are things we can safely ignore?

I do not see any failures, CI is still running

Yay for eventually consistent GUI. Github was showing me 2 failed runs but when I refreshed the status changed to "expected"

@rvolosatovs rvolosatovs merged commit c72d3a8 into wasmCloud:main Jul 21, 2023
@rvolosatovs rvolosatovs deleted the update/deps branch July 21, 2023 15:46
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.

4 participants