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

fix: system message returned from GetState #56

Merged
merged 3 commits into from
Feb 12, 2024

Conversation

kodumbeats
Copy link
Contributor

@kodumbeats kodumbeats commented Feb 10, 2024

Between Erlang/OTP 26.0 and 26.1, the interface for system messages changed from a literal type to result type, breaking our implementation. See OTP-18633 for more information.

This patch updates the library to 26.1, but sets a minimum version requirement. Since this is such a niche API (debugging only), we agreed that documentation is sufficient to enforce versions.

Open Questions

  • How descriptive do we need to be with the error type? Is Nil sufficient since we always respond with Ok(state)?

Tests pass locally, though some files need a reformat. Will address separately.

# Erlang/OTP 26.1

kdmb@nas ~/g/k/gleam_otp (fix/erlang_system_message_interface)> asdf current erlang
erlang          26.1.2          /home/kdmb/.tool-versions
kdmb@nas ~/g/k/gleam_otp (fix/erlang_system_message_interface)> gleam test
  Compiling gleam_otp
   Compiled in 0.34s
    Running gleam_otp_test.main
...............
Finished in 0.281 seconds
15 tests, 0 failures

# Erlang/OTP 26.2

kdmb@nas ~/g/k/gleam_otp (fix/erlang_system_message_interface)> asdf current erlang
erlang          26.2.1          /home/kdmb/.tool-versions
kdmb@nas ~/g/k/gleam_otp (fix/erlang_system_message_interface)> gleam test
   Compiled in 0.01s
    Running gleam_otp_test.main
...............
Finished in 0.282 seconds
15 tests, 0 failures

Between erlang v26.0 and v26.1, the interface for system messages
changed from a literal type to result type, breaking our implementation.

This patch updates actor message handling accordingly.
This way, the actor does not need to reply with Ok(state)
@kodumbeats kodumbeats force-pushed the fix/erlang_system_message_interface branch from 03d9b30 to 5e61fa0 Compare February 10, 2024 18:54
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you

@lpil lpil merged commit e1ec6d0 into gleam-lang:main Feb 12, 2024
1 check failed
@tankorsmash
Copy link

tankorsmash commented Mar 11, 2024

I'll mention this for posterity in case it helps anyone running into the same issue until this is live:

You can use kerl to install different versions of erlang, so for now, you need to do something like

# install older version of erlang
kerl build 26.0.1
kerl install 26.0.1 ~/erlang/v26.0.1 # or whichever path you prefer

# activate the installation
. ~/erlang/v26.0.1/activate

# to check the version
erl -eval '{ok, Version} = file:read_file(filename:join([code:root_dir(), "releases", erlang:system_info(otp_release), "OTP_VERSION"])), io:fwrite(Version), halt().' -noshell
>> 26.0.1

# once you're done editing, or want to install a new one
kerl_deactivate

I can confirm 26.2.2 is broken with gleam_otp v0.9.0, and that activating 26.0.1 allows for calling system.get_state without failing.

Not sure what other implications going to a different version of erlang might have though.

@lpil
Copy link
Member

lpil commented Mar 11, 2024

Sorry, I should have released this. I'll do that now.

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.

3 participants