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

Use lib_elixir instead of vendoring elixir modules manually #162

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

doorgan
Copy link
Owner

@doorgan doorgan commented Aug 8, 2024

Some notes:

  • We need to exclude exception modules from lib_elixir
  • We need to wait until lib_elixir uses httpc or other http module that doesn't require elixir >= 1.13

@zachallaun
Copy link
Contributor

@doorgan I've replaced Req with httpc on main; if you could give that a shot when you have a moment, I'd appreciate it! (You may have to rm -r _build as well.)

@doorgan
Copy link
Owner Author

doorgan commented Aug 14, 2024

@zachallaun is there a chance we can replace the usage of Keyword.validate! here by some manual validation? https://github.com/zachallaun/lib_elixir/blob/dda978a0ac1dc48d075aaec97c3639fc16ab5f13/lib/mix/tasks/compile.lib_elixir.ex#L71-L72

I think it's the only missing bit right now to keep supporting elixir 1.12 in Sourceror, otherwise it seems to be working great!

@zachallaun
Copy link
Contributor

zachallaun commented Aug 15, 2024

@doorgan Done! Also introduced some... hackery... to handle a change in the debug_info beam chunk that happened in Elixir 1.15. This is necessary to compile a lib_elixir >= 1.15 on a runtime < 1.15.

I'm sure there are more bugs, but there's one limitation I know about: lib_elixir main requires OTP 23+. This is because elixir.erl uses :erlang.atom_to_binary(atom) which wasn't introduced until OTP 23. I should be able to replace those calls with :erlang.atom_to_binary(atom, :utf8), which is the equivalent.

@zachallaun
Copy link
Contributor

This is proving a good resource for ironing out kinks! I'll pull this PR later and try to work through the remaining errors.

Re: Mix.Tasks.Format failing, right now namespacing doesn't include the mix app at all, but I think it could!

@zachallaun
Copy link
Contributor

Tests passing on 1.12.3/24.3 locally with latest lib_elixir and these changes:

diff --git a/lib/sourceror.ex b/lib/sourceror.ex
index 531bf1c..35c7f03 100644
--- a/lib/sourceror.ex
+++ b/lib/sourceror.ex
@@ -202,7 +202,7 @@ defmodule Sourceror do

     quoted
     |> to_algebra(opts)
-    |> Inspect.Algebra.format(line_length)
+    |> LibElixir.Inspect.Algebra.format(line_length)
     |> IO.iodata_to_binary()
     |> indent(indent_str, indent)
     |> splice(quoted, opts[:format])
diff --git a/lib/sourceror/zipper/inspect.ex b/lib/sourceror/zipper/inspect.ex
index ef43bd4..a9832e2 100644
--- a/lib/sourceror/zipper/inspect.ex
+++ b/lib/sourceror/zipper/inspect.ex
@@ -69,7 +69,7 @@ defmodule Sourceror.Zipper.Inspect do

   """

-  import Inspect.Algebra
+  import Sourceror.LibElixir.Inspect.Algebra

   alias Sourceror.Zipper, as: Z

diff --git a/mix.exs b/mix.exs
index 8139118..210542e 100644
--- a/mix.exs
+++ b/mix.exs
@@ -28,7 +28,8 @@ defmodule Sourceror.MixProject do
       lib_elixir: [
         namespace: Sourceror.LibElixir,
         ref: "v1.17.2",
-        modules: [Code, :elixir_tokenizer, Inspect.Algebra, Mix.Tasks.Format]
+        include: [Code, :elixir_tokenizer, Inspect.Algebra],
+        exclude: [Inspect.Opts]
       ]
     ]
   end
@@ -76,10 +77,7 @@ defmodule Sourceror.MixProject do
       {:ex_check, "~> 0.15.0", only: [:dev], runtime: false},
       {:ex_doc, ">= 0.0.0", only: :dev, runtime: false},
       {:excoveralls, "~> 0.15", only: [:test]},
-      {:lib_elixir,
-       github: "zachallaun/lib_elixir",
-       ref: "1a23de6275922b575f32d524064e95c153bb6d5a",
-       runtime: false},
+      {:lib_elixir, github: "zachallaun/lib_elixir", ref: "d0144f9", runtime: false},
       {:sobelow, "~> 0.11", only: :dev}
     ]
   end
diff --git a/mix.lock b/mix.lock
index 413121d..f83c16a 100644
--- a/mix.lock
+++ b/mix.lock
@@ -15,7 +15,7 @@
   "hpax": {:hex, :hpax, "1.0.0", "28dcf54509fe2152a3d040e4e3df5b265dcb6cb532029ecbacf4ce52caea3fd2", [:mix], [], "hexpm", "7f1314731d711e2ca5fdc7fd361296593fc2542570b3105595bb0bc6d0fad601"},
   "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},
   "jason": {:hex, :jason, "1.4.1", "af1504e35f629ddcdd6addb3513c3853991f694921b1b9368b0bd32beb9f1b63", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fbb01ecdfd565b56261302f7e1fcc27c4fb8f32d56eab74db621fc154604a7a1"},
-  "lib_elixir": {:git, "https://github.com/zachallaun/lib_elixir.git", "1a23de6275922b575f32d524064e95c153bb6d5a", [ref: "1a23de6275922b575f32d524064e95c153bb6d5a"]},
+  "lib_elixir": {:git, "https://github.com/zachallaun/lib_elixir.git", "d0144f92aa37dcced78d37148e1eb2bb48e36e1c", [ref: "d0144f9"]},
   "makeup": {:hex, :makeup, "1.1.1", "fa0bc768698053b2b3869fa8a62616501ff9d11a562f3ce39580d60860c3a55e", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "5dc62fbdd0de44de194898b6710692490be74baa02d9d108bc29f007783b0b48"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
   "makeup_erlang": {:hex, :makeup_erlang, "0.1.5", "e0ff5a7c708dda34311f7522a8758e23bfcd7d8d8068dc312b5eb41c6fd76eba", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "94d2e986428585a21516d7d7149781480013c56e30c6a233534bedf38867a59a"},

Can now add additional module exclusions using the :exclude config!

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.

2 participants