Add erlang-module support #2094

fnchooft · 2023-07-19T18:27:00Z

The code is deemed a module definition if the first line start with "-module(".

In this case the entire code-block is interpreted as erlang-module and if there are no errors the module is compiled and loaded.

An example for a working livebook (livemd-file):

Untitled notebook

Erlang - on the fly ( inline_module ) via Livebook

-module(inline_module2).

-export([go/1]).

go(jose) -> {ok,<<"thanks Jose! :)">>};
go(fabian) -> {ok,<<"kkkk - this Elixir-code is heavily Erlang-flavoured heim??? :)">>};
go(_) -> {ok, <<"thanks contributers!">>}.

inline_module2:go(jose).

CLAassistant · 2023-07-19T18:27:05Z

All committers have signed the CLA.

josevalim · 2023-07-19T19:01:30Z

lib/livebook/runtime/evaluator.ex

+      Enum.map(
+        statements,
+        fn str ->
+          {:ok, result, _} = :erl_scan.string(String.to_charlist(str))


We need to handle errors accordingly. It will be similar to the eval_statements branch, there is probably a lot we can share between the two. :)

Thanks for the feedback - will adapt based on the original-code.

josevalim · 2023-07-19T19:02:07Z

lib/livebook/runtime/evaluator.ex

@@ -675,6 +675,47 @@ defmodule Livebook.Runtime.Evaluator do
  end

  defp eval(:erlang, code, binding, env) do
+    is_module = String.starts_with?(code, "-module(")


Instead of detecting based on a string, we can do the first pass with :erl_scan.string(String.to_charlist(str)) and then traverse all entries and see if any defines a module attribute.

Your opinion: Analyze the forms to only allow one module? Throw an error to instruct the user?
Seems more inline with how Erlang defines modules.

If you defined multiple modules we would have to have a lot more checks, and we would have to seperate the statements per module.

Yeah, for now, I think the best is to:

Check if it has attributes

If it has attributes, one of them (no more, no less) has to be a module attribute

For visibility @josevalim @fnchooft Did a PR for this here!
fnchooft#1

josevalim · 2023-07-19T19:02:32Z

This is very exciting @fnchooft, I have added some comments. We need error handling and tests. :)

fnchooft · 2023-07-20T14:21:55Z

Hi Jose, still W.I.P...

Changes:

Basic try-catch for error-handling
Use the scanner for the whole code-block
Create forms from tokens - Erlang-code-conversion - I am not Elixir apt-yet.... :)
TODO: Refactor the common parts
TODO: Add tests

josevalim · 2023-07-20T16:49:11Z

lib/livebook/runtime/evaluator.ex

+        Enum.map(
+          form_statements,
+          fn {form_statement} ->
+            {:ok, form} = :erl_parse.parse_form(form_statement)


Instead of try/catch below, we should pattern match and handle those forms similar to the other branch. You can look into Elixir's with (which is similar to Erlang's maybe).

This has been cleaned up in the manner you proposed.

josevalim · 2023-07-20T16:49:31Z

lib/livebook/runtime/evaluator.ex

+    catch
+      kind, error ->
+        stacktrace = prune_stacktrace(:erl_eval, __STACKTRACE__)
+        {{:error, kind, error, stacktrace}, []}


I think this outer try-catch is not necessary. :)

Morning - this has been cleaned up in latest update.

jonatanklosko · 2023-07-20T16:52:26Z

Also note that we need to recognise which modules are defined and used by each evaluation and return this information, similarly to used_vars:

livebook/lib/livebook/runtime/evaluator.ex

Lines 766 to 774 in 9296c48

    
           # Simple heuristic to detect the used variables. We look at 
        
           # the tokens and assume all var tokens are used variables. 
        
           # This will not handle shadowing of variables in fun definitions 
        
           # and will only work well enough for expressions, not for modules. 
        
           used_vars = 
        
             for {:var, _anno, name} <- tokens, 
        
                 do: {erlang_to_elixir_var(name), nil}, 
        
                 into: MapSet.new(), 
        
                 uniq: true

For Elixir we use tracer, but as a reference that's how the entries look like:

livebook/lib/livebook/runtime/evaluator.ex

Lines 938 to 949 in 9296c48

    
           # Modules 
        
           identifiers_used = 
        
             for module <- tracer_info.modules_used, 
        
                 do: {:module, module}, 
        
                 into: identifiers_used 
        
           identifiers_defined = 
        
             for {module, _vars} <- tracer_info.modules_defined, 
        
                 version = module.__info__(:md5), 
        
                 do: {{:module, module}, version}, 
        
                 into: identifiers_defined

fnchooft · 2023-07-20T18:58:38Z

@jonatanklosko - Thanks for the insight! Question: In the case we would limit the Code-erlang-block to the definition of only one Erlang-module, not allowing other expressions for instance, would it be enough to only add the module_name to the used_identifiers?

jonatanklosko · 2023-07-21T10:53:14Z

If we enforce just a single module we can compute md5 of the source as the module version or phash of the forms, because we would know all the code is about the module.

josevalim

Looking good! Can you please add some tests that exercise the error branches too? Some that make the tokenizer fail after a module is defined. And another that makes the parser file :) thanks!

The code is deemed a module definition if the first line start with "-module(". In this case the entire code-block is interpreted as erlang-module and if there are no errors the module is compiled and loaded. Basic error handling has been added Basic tests have been added

fnchooft · 2023-07-24T19:21:29Z

Afternoon - had some time to add:

More tests ( expression after module / test for duplicate function definitions etc )

@jonatanklosko - I tried adding only the modulename + md5 to the logic - as you suggested.
However it still is not 100% ok.

For instance - it is not detecting correctly if a call to a function in the module is stale or not.
If the function is not defined the error-handling is ok, however it would be nice to get the "stale"-indication on the erlang-code-cell.

jonatanklosko · 2023-07-25T15:29:41Z

For instance - it is not detecting correctly if a call to a function in the module is stale or not.

Yeah, and the same is going to be the case if you define a module with go/0, have another cell call go()., and then change the module implementation. We need to analyse the AST to find function calls and from that get a list of modules used by the given evaluation (cell).

For elixir we use compiler tracer to get the referenced modules and we then we use this information here:

livebook/lib/livebook/runtime/evaluator.ex

Lines 861 to 864 in efeebe9

    
           identifiers_used = 
        
             for module <- tracer_info.modules_used, 
        
                 do: {:module, module}, 
        
                 into: identifiers_used

@josevalim I'm thinking that ideally we would emit the tracer events programmatically (defining a module, referencing functions) and then the rest is handled using the same code path. Does that make sense, or is there any reason not to do that?

josevalim · 2023-07-25T16:48:41Z

@jonatanklosko emulating the tracer or the tracer_info is perfectly fine for me. The only concern is that there is no env.

jonatanklosko · 2023-07-25T17:10:26Z

Ah true, actually it should be simpler to use %Evaluator.Tracer{} together with Evaluator.Tracer.apply_updates/2 directly. If there are issues, we can have something more separate.

Other notes: we need to persist the module bytecode with Evaluator.write_module!(module, bytecode) and also add it to env.context_modules.

fnchooft · 2024-02-16T13:21:25Z

@jonatanklosko - Apologies for letting this simmer for some months.

I have not been following changes in livebook - but is there still value in this approach? If I have some time I will rebase and re-read the comments.

Kind regards,
Fabian

jonatanklosko · 2024-02-16T15:56:30Z

@fnchooft no worries, nothing changed in this matter, so sounds good :)

github-actions · 2024-09-24T20:25:42Z

Uffizzi Preview deployment-56598 was deleted.

guisaez · 2024-09-25T19:09:11Z

Hello! @fnchooft @josevalim @jonatanklosko Is there an update on this feature?

fnchooft · 2024-09-25T19:16:29Z

Hi, yesterday night I rebased on master, hence the trigger. I want to try and reserve some time to complete this; it seems we are almost there. Kind regards, Fabian

…

On Wed, Sep 25, 2024 at 4:09 PM Guillermo Saez Santana < ***@***.***> wrote: Hello! @fnchooft <https://github.com/fnchooft> @josevalim <https://github.com/josevalim> @jonatanklosko <https://github.com/jonatanklosko> Is there an update on this feature? — Reply to this email directly, view it on GitHub <#2094 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABD6LCJBNPOKGPIQSGCT3IDZYMC65AVCNFSM6AAAAABOZCPPSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZUHE3TCNZUHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

guisaez · 2024-09-27T02:21:49Z

test/livebook/runtime/evaluator_test.exs

+        []
+      )
+
+      assert_receive {:runtime_evaluation_response, :code_4,


@fnchooft Again, I'm not very faimiliar with the test logic in Elixir, but I noticed that for most tests it actually returns a map, not a tuple. i changed this lines to

assert_receive {:runtime_evaluation_response, :code_4, terminal_text("\"erlang module successfully compiled\""), metadata()}

guisaez · 2024-09-27T02:22:33Z

test/livebook/runtime/evaluator_test.exs

+        []
+      )
+
+      assert_receive {:runtime_evaluation_output, :code_4,


assert_receive {:runtime_evaluation_output, :code_4, terminal_text(":1:52: function go/0 already defined\n", true)}

guisaez · 2024-09-27T02:23:49Z

test/livebook/runtime/evaluator_test.exs

+      assert_receive {
+        :runtime_evaluation_response,
+        :code_4,
+        {


assert_receive { :runtime_evaluation_response, :code_4, error(_), %{code_markers: _List} }

fnchooft · 2024-09-27T10:15:38Z

Hi yesterday i spent some time cleaning those up. I will try to share today. Kind regards, Fabian

…

On Thu, Sep 26, 2024, 23:22 Guillermo Saez Santana ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In test/livebook/runtime/evaluator_test.exs <#2094 (comment)> : > @@ -1300,6 +1300,53 @@ defmodule Livebook.Runtime.EvaluatorTest do assert_receive {:runtime_evaluation_response, :code_1, terminal_text("6"), metadata()} end + test "evaluate erlang-module code", %{evaluator: evaluator} do + Evaluator.evaluate_code( + evaluator, + :erlang, + "-module(tryme). -export([go/0]). go() ->{ok,went}.", + :code_4, + [] + ) + + assert_receive {:runtime_evaluation_response, :code_4, @fnchooft <https://github.com/fnchooft> Again, I'm not very faimiliar with the test logic in Elixir, but I noticed that for most tests it actually returns a map, not a tuple. i changed this lines to assert_receive {:runtime_evaluation_response, :code_4, %{:text => "\"erlang module successfully compiled\""}, metadata()} — Reply to this email directly, view it on GitHub <#2094 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABD6LCJC37DQWU7NNCPBGLTZYS6NHAVCNFSM6AAAAABOZCPPSGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMZSGYZTAMJQGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

fnchooft · 2024-09-27T13:59:43Z

@guisaez: I started a fresh branch v2.
This branch has your fixes in them as well. Thanks!

The following snipplet shows a use-case:

I need to ask for some assistance to add correctly the module-name/version since Stale-indicator should trigger on the second cell if the first one is updated.

fnchooft · 2024-10-01T00:30:46Z

Hi all, I started a new pull-request based on latest main.
It is now working in the case you delete the cell with the module - subsequent cells will not work anymore.

Thanks all for the the discussions/contributions here - lets pick up the remaining issues in #2806?

josevalim reviewed Jul 19, 2023

View reviewed changes

fnchooft force-pushed the erlang_module_support branch from 3af8aa1 to 9296c48 Compare July 20, 2023 14:17

josevalim reviewed Jul 20, 2023

View reviewed changes

fnchooft force-pushed the erlang_module_support branch from 9296c48 to 1447859 Compare July 21, 2023 11:26

josevalim reviewed Jul 21, 2023

View reviewed changes

fnchooft force-pushed the erlang_module_support branch from 1447859 to 13e17f6 Compare July 24, 2023 19:17

Merge branch 'livebook-dev:main' into erlang_module_support

e64aa97

guisaez reviewed Sep 27, 2024

View reviewed changes

josevalim closed this Oct 1, 2024

fnchooft deleted the erlang_module_support branch October 17, 2024 12:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add erlang-module support #2094

Add erlang-module support #2094

fnchooft commented Jul 19, 2023 •

edited

Loading

CLAassistant commented Jul 19, 2023 •

edited

Loading

josevalim Jul 19, 2023

fnchooft Jul 19, 2023

josevalim Jul 19, 2023

fnchooft Jul 19, 2023

josevalim Jul 19, 2023

guisaez Sep 26, 2024

josevalim commented Jul 19, 2023

fnchooft commented Jul 20, 2023

josevalim Jul 20, 2023

fnchooft Jul 21, 2023

josevalim Jul 20, 2023

fnchooft Jul 21, 2023

jonatanklosko commented Jul 20, 2023 •

edited

Loading

fnchooft commented Jul 20, 2023

jonatanklosko commented Jul 21, 2023

josevalim left a comment

fnchooft commented Jul 24, 2023

jonatanklosko commented Jul 25, 2023 •

edited

Loading

josevalim commented Jul 25, 2023

jonatanklosko commented Jul 25, 2023

fnchooft commented Feb 16, 2024

jonatanklosko commented Feb 16, 2024

github-actions bot commented Sep 24, 2024 •

edited

Loading

guisaez commented Sep 25, 2024

fnchooft commented Sep 25, 2024 via email

guisaez Sep 27, 2024 •

edited

Loading

guisaez Sep 27, 2024 •

edited

Loading

guisaez Sep 27, 2024 •

edited

Loading

fnchooft commented Sep 27, 2024 via email

fnchooft commented Sep 27, 2024 •

edited

Loading

fnchooft commented Oct 1, 2024

Add erlang-module support #2094

Add erlang-module support #2094

Conversation

fnchooft commented Jul 19, 2023 • edited Loading

Untitled notebook

Erlang - on the fly ( inline_module ) via Livebook

CLAassistant commented Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josevalim commented Jul 19, 2023

fnchooft commented Jul 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonatanklosko commented Jul 20, 2023 • edited Loading

fnchooft commented Jul 20, 2023

jonatanklosko commented Jul 21, 2023

josevalim left a comment

Choose a reason for hiding this comment

fnchooft commented Jul 24, 2023

jonatanklosko commented Jul 25, 2023 • edited Loading

josevalim commented Jul 25, 2023

jonatanklosko commented Jul 25, 2023

fnchooft commented Feb 16, 2024

jonatanklosko commented Feb 16, 2024

github-actions bot commented Sep 24, 2024 • edited Loading

guisaez commented Sep 25, 2024

fnchooft commented Sep 25, 2024 via email

guisaez Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

guisaez Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

guisaez Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

fnchooft commented Sep 27, 2024 via email

fnchooft commented Sep 27, 2024 • edited Loading

fnchooft commented Oct 1, 2024

fnchooft commented Jul 19, 2023 •

edited

Loading

CLAassistant commented Jul 19, 2023 •

edited

Loading

jonatanklosko commented Jul 20, 2023 •

edited

Loading

jonatanklosko commented Jul 25, 2023 •

edited

Loading

github-actions bot commented Sep 24, 2024 •

edited

Loading

guisaez Sep 27, 2024 •

edited

Loading

guisaez Sep 27, 2024 •

edited

Loading

guisaez Sep 27, 2024 •

edited

Loading

fnchooft commented Sep 27, 2024 •

edited

Loading